Skip to content

plat-stm32mp1: bumping to pinctrl framework - #6082

Merged
jforissier merged 11 commits into
OP-TEE:masterfrom
etienne-lms:stm32-pinctrl-2
Jul 31, 2023
Merged

plat-stm32mp1: bumping to pinctrl framework #6082
jforissier merged 11 commits into
OP-TEE:masterfrom
etienne-lms:stm32-pinctrl-2

Conversation

@etienne-lms

Copy link
Copy Markdown
Contributor

Bump stm32_gpio driver to recently merged pinctrl device driver probing support.

For STM32MP13 variant to comply with pinctrl support (and not panic on driver probe errors), I needed to update STM32MP13 DTS and DTSI file and 2 stm32 drivers (stm32_etzpc and stm32_rstctrl). They are part of this series.

A patch fixing a pinctrl driver probe function in under review at #6081.

@etienne-lms

Copy link
Copy Markdown
Contributor Author

This series it too long. I'll shorten it and rebase it.

@etienne-lms

etienne-lms commented Jun 6, 2023

Copy link
Copy Markdown
Contributor Author

I've kept only the 5 first commits initially posted (15 patches) in the P-R, the one that introduce PINCTRL support in stm32_gpio.c, leaving out activation of CFG_DRIVERS_PINCTRL on plat-stm32mp1 as there are few other platform resources to update.

Some of these platform updates are proposed to review in P-R #6091.

@etienne-lms etienne-lms changed the title plat-stm32mp1: bump to pinctrl framwork + update DTS files and etzpc/rstctrl drivers plat-stm32mp1: prepare bump to pinctrl framwork Jun 7, 2023
@etienne-lms etienne-lms changed the title plat-stm32mp1: prepare bump to pinctrl framwork plat-stm32mp1: prepare bumping to pinctrl framework Jun 7, 2023
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c
Comment thread core/drivers/stm32_gpio.c
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c
Comment thread core/drivers/stm32_gpio.c
Comment thread core/drivers/stm32_gpio.c
Comment thread core/drivers/stm32_gpio.c
@etienne-lms
etienne-lms force-pushed the stm32-pinctrl-2 branch 3 times, most recently from 9a10c45 to 80081a0 Compare June 7, 2023 17:56
@etienne-lms

Copy link
Copy Markdown
Contributor Author

for info, CI checkpatch error reports are false positives (build error trace messages dumped in commit messages).

@etienne-lms

Copy link
Copy Markdown
Contributor Author

review comments addressed

@etienne-lms

Copy link
Copy Markdown
Contributor Author

may I squash the fixup commits?

Comment thread core/arch/arm/plat-stm32mp1/shared_resources.c Outdated
Comment thread core/arch/arm/plat-stm32mp1/shared_resources.c Outdated
@GseoC

GseoC commented Jun 9, 2023

Copy link
Copy Markdown
Contributor

may I squash the fixup commits?

For clearer view, yes please

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Fixup commits squashed + addressed comment on stm32mp_register_secure_pinctrl()/stm32mp_register_non_secure_pinctrl().

@GseoC

GseoC commented Jun 14, 2023

Copy link
Copy Markdown
Contributor

Thank you,

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>

@etienne-lms

Copy link
Copy Markdown
Contributor Author

tag applied
thanks

@etienne-lms

Copy link
Copy Markdown
Contributor Author

I need to rebase due to #6098 being recently merged.

@etienne-lms

Copy link
Copy Markdown
Contributor Author

@GseoC, I added a fixup commit to "drivers: stm32_gpio: support CFG_DRIVERS_PINCTRL", to address the rebase issue.
Could you please confirm (or infirm) your A-b tag with this change?

@jforissier

Copy link
Copy Markdown
Contributor

I have restarted IBART which failed on a socket test (rcu_sched stalls), looks unrelated to this PR.

@etienne-lms

Copy link
Copy Markdown
Contributor Author

thanks

@GseoC

GseoC commented Jun 14, 2023

Copy link
Copy Markdown
Contributor

Seems fine, tag applies

@etienne-lms

Copy link
Copy Markdown
Contributor Author

Some changes in the series, to finalize bumping stm32_gpio.c to GPIO and PINCTRL frameworks:

  • 1st commit is unmodified regarding previous review. @GseoC review tag is already applied to the commit.
    "drivers: stm32_gpio: support CFG_DRIVERS_PINCTRL"

  • The 5 next commit were review but I changed added functions prototypes to return void instead of a useless TEE_Result. @GseoC, I removed your review tag from those commits:
    "drivers: stm32_gpio: add helper function stm32_gpio_pinctrl_bank_pin()"
    "drivers: stm32_gpio: add helper function stm32_pinctrl_set_secure_cfg()"
    "plat-stm32mp1: shared_resources: support CFG_DRIVERS_PINCTRL"
    "drivers: stm32_uart: support CFG_DRIVERS_PINCTRL"
    "drivers: stm32_i2c: support CFG_DRIVERS_PINCTRL"

  • I added 5 new commits in the series, to finalize the evolution: enforce CFG_DRIVERS_PINCTRL=y and remove deprecated functions:
    "plat-stm32mp1: conf: enable CFG_DRIVERS_PINCTRL"
    "drivers: stm32_gpio: remove cases when CFG_DRIVERS_PINCTRL is disabled"
    "drivers: stm32_uart: remove cases when CFG_DRIVERS_PINCTRL is disabled"
    "drivers: stm32_i2c: remove cases when CFG_DRIVERS_PINCTRL is disabled"
    "drivers: stm32_gpio: move definitions to source file"

@etienne-lms etienne-lms changed the title plat-stm32mp1: prepare bumping to pinctrl framework plat-stm32mp1: bumping to pinctrl framework Jun 22, 2023
@etienne-lms

Copy link
Copy Markdown
Contributor Author

Changed title has the seire finalizes stm32mp1 upgrades to pinctrl framework.
Checkpatch failure on a typo in a commit. I'll the 2 fixed after the review.

Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c
size_t conf_index = 0;

if (!pinctrl)
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a message as we may think we secure a pin that will not be secure?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is no pinctrl, there is no pin to configure so returning straight is fine, IMO.

Comment thread core/drivers/stm32_i2c.c
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c Outdated
Comment thread core/drivers/stm32_gpio.c Outdated
@GseoC

GseoC commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

I do not catch why the code style check fails, I guess it did not restart

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>

@jforissier

jforissier commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

I do not catch why the code style check fails, I guess it did not restart

It did, but the way the code style check works might be a bit counter-intuitive 😉 It is actually run twice:

  • once for each commit in the PR, and
  • once on the global diff (all commits squashed together if you wish)

The rationale is, we want to check the code but also the commit descriptions, so each commit is checked (the checkpatch lines in the log). When fixes are done as fixup commits (as opposed to force-pushing with corrected commits), the initial failure remain visible. However thanks to the second phase (called checkdiff in the log), you can be confident that the code style issues will be gone when squashing the fixups.

@GseoC

GseoC commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

I do not catch why the code style check fails, I guess it did not restart

It did, but the way the code style check works might be a bit counter-intuitive wink It is actually run twice:

  • once for each commit in the PR, and
  • once on the global diff (all commits squashed together if you wish)

The rationale is, we want to check the code but also the commit descriptions, so each commit is checked (the checkpatch lines in the log). When fixes are done as fixup commits (as opposed to force-pushing with corrected commits), the initial failure remain visible. However thanks to the second phase (called checkdiff in the log), you can be confident that the code style issues will be gone when squashing the fixups.

Thanks for the explanation :)

@etienne-lms

Copy link
Copy Markdown
Contributor Author

I'll sqaush the fixup commits. Once done, checkpatch should be happy.

Changes stm32_gpio driver to support generic pin control framework
(CFG_DRIVERS_PINCTRL=y).

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Adds helper function stm32_gpio_pinctrl_bank_pin() to get an array
of bank and pin IDs related to a pin control state.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Adds helper function stm32_pinctrl_set_secure_cfg() to set the
GPIO pin secure state (secure or non-secure) for each pin referenced
by a pin control state.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Adds shared resources helper functions stm32mp_register_secure_pinctrl()
and stm32mp_register_non_secure_pinctrl() for when a platform driver
registers pins from a pin control state with secure or non-secure
attribute. These function are required when CFG_DRIVERS_PINCTRL is
enabled.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Updates stm32_uart driver for when CFG_DRIVERS_PINCTRL is enabled making
UART driver to get pin control configuration using the generic pin
control framework.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Updates stm32_i2c driver for when CFG_DRIVERS_PINCTRL is enabled making
I2C driver to get pin control configuration using the generic pin
control framework. When enabled, stm32_i2c driver get the active and
sleep pin control configuration from the device tree. Sleep pinctrl
configuration is optional.

SE050 and STM32MP1 PMIC drivers that use the stm32_i2c bus are both
updated accordingly.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Changes platform stm32mp1 configuration to always enable
CFG_DRIVERS_PINCTRL. The platform requires pinctrl_apply_state() to
be unpaged has it can be used during PM suspend and resume sequences.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Removes implementation when CFG_DRIVERS_PINCTRL is disables as stm32mp1
platform configuration enforces the switch is enabled.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Removes implementation when CFG_DRIVERS_PINCTRL is disables as stm32mp1
platform configuration enforces the switch is enabled.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Removes implementation when CFG_DRIVERS_PINCTRL is disables as stm32mp1
platform configuration enforces the switch is enabled.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
Moves macros and structures definitions from stm32_gpio.h header file
to the driver source file as these definition do not need to be
visible from other drivers thank to pin control abstraction.

Acked-by: Gatien Chevallier <gatien.chevallier@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@foss.st.com>
@etienne-lms

Copy link
Copy Markdown
Contributor Author

Thakns for the review.
Fixup commits squashed.
Review tag applied.

I also removed a suprious inline comment in comment "drivers: stm32_i2c: remove cases when CFG_DRIVERS_PINCTRL is disabled" (below). @GseoC, I preserved your review tag on that commit, I think it still applies :)

@@ -113,7 +113,6 @@ struct i2c_cfg {
  * @sec_cfg: I2C registers configuration storage
  * @pinctrl: Pin control configuration for the I2C bus in active state
  * @pinctrl_sleep: Pin control configuration for the I2C bus in standby state
- * @pinctrl_count: Number of PINCTRLs elements
  */
 struct i2c_handle_s {
        struct io_pa_va base;

@github-actions

Copy link
Copy Markdown

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions Bot added the Stale label Jul 31, 2023
@jforissier
jforissier merged commit 69715ce into OP-TEE:master Jul 31, 2023
@etienne-lms
etienne-lms deleted the stm32-pinctrl-2 branch October 12, 2023 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants