Skip to content

Conversation

@erwango
Copy link
Member

@erwango erwango commented Nov 9, 2021

Based on #39430 (Thanks @gmarull for the great initial work)

Following merge of pinctrl API (#37572), provide the STM32 implementation.
This takes place of the existing, similar but non generic STM32 implementation.

All in-tree drivers are converted, along with the peripheral configuration for each in tree boards.

Deprecation of the previous API is not addressed, to comply with deprecation timeline proposed in #39740

Replaces #39430

Important notice:
pinctrl-0 and pinctrl-names are now set as required properties (for pinctrl user devices).
Main reason is to avoid DT_API cryptic error messages when not provided.

Note: Set as DNM as it includes a BK temp patch to increase workload capacity

EDIT: Compliance check reports a typdef warning, which I suggest to discard.

-:9321: WARNING:NEW_TYPEDEFS: do not add new typedefs
#9321: FILE: soc/arm/st_stm32/common/pinctrl_soc.h:32:
+typedef struct pinctrl_soc_pin {

EDIT: 11/15: Removed PWM driver update, due to underlying issue in PWM driver that should be taken care first and separately. Submitted #40430 to handle PWM case.
EDIT: 11/23: Based on #40515 to remove need to use PINCTRL_STORE_REG on F1 series. As a consequence, PWM driver update is back.
EDIT: 11/24: Added a couple of commits to fix erroneous board descriptions that are now leading to build issues (now that pinctrl-0 and pinctrl-names are required)

Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

LGTM, a couple of minor comments. Another suggestion: drivers now depend on CONFIG_PINCTRL=y, so should we specify such dependency in Kconfig?

@erwango
Copy link
Member Author

erwango commented Nov 9, 2021

@gmarull Thanks for the review

Another suggestion: drivers now depend on CONFIG_PINCTRL=y, so should we specify such dependency in Kconfig?

Most drivers also depends on CLOCK_CONTROL and this dependency is not specified.
So, for coherency, if we start stating these dependencies, we should perform a full assessment and update.

@gmarull
Copy link
Member

gmarull commented Nov 9, 2021

Another suggestion: may be useful to add a test for DT parsing: 50bd669

Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Additionally, rename eth0_pins to eth0_pcfg to better fit
new pinctrl API.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the QSPI peripherals.

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
There's no QSPI peripheral device available on this board.
Remove node definition.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the I2C peripherals.
Changes performed based on the script proposed in
"boards: arm: stm32: add pinctrl state name for UART peripheral"

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the I2S peripherals.

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the FMC peripherals.

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the CAN peripherals.
Changes performed based on the script proposed in
"boards: arm: stm32: add pinctrl state name for UART peripheral"

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the USB peripherals.

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.

Additionally, rename usb_pinctrl to usb_pcfg to better fit
new pinctrl API.

Signed-off-by: Erwan Gouriou <[email protected]>
Add the pinctrl state name (default) for the PWM peripherals.
Changes performed based on the script proposed in
"boards: arm: stm32: add pinctrl state name for UART peripheral"

Signed-off-by: Erwan Gouriou <[email protected]>
Use the new pinctrl API to configure pins.
Since STM32F1 series require pinctrl option and required register
address is parent timer address in place of own node register address,
use PINCTRL_DT_INST_CUSTOM_REG_DEFINE in place of usual
PINCTRL_DT_INST_DEFINE for this specific series.

Additionally, remove the automatic selection of PINMUX API.

Signed-off-by: Erwan Gouriou <[email protected]>
Now that all drivers and all boards have been converted to the
use of PINCTRL, remove usage of PINMUX.

Signed-off-by: Erwan Gouriou <[email protected]>
Now that STM32 drivers are using pinctrl API, set pintrl-0 and
pintrl-names properties as required in order to report malformed
nodes description soon at build stage and avoid cryptic
DT api build error messages.

Signed-off-by: Erwan Gouriou <[email protected]>
Set SPI nodes status as disabled, as this should be the in .dtsi
soc description.

Signed-off-by: Erwan Gouriou <[email protected]>
On stm32l5 based boards, some serial devices descriptions were
scattered between common.dtsi and base or non secure dts files.

This leads to build issues now that pinctrl-0 and pinctrl-names
properties are required but this was anyway broken since the
beginning.

Group all serial devices definition in common.dtsi so complete devices
description is available to both non secure and base descriptions.

Signed-off-by: Erwan Gouriou <[email protected]>
Now that pinctrl-0 property is required in descriptions of nodes
that implies definition of pins, these incomplete nodes definitions
lead to build error.
Fix or remove depending of the board documentation.

Signed-off-by: Erwan Gouriou <[email protected]>
Copy link
Contributor

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

LGTM

@erwango erwango removed the DNM This PR should not be merged (Do Not Merge) label Nov 24, 2021
Pinctrl is missing in i2c1 device description on stm32373c_eval
ovderlay file, besides using I2C1 this board (as mentioned in readme)
requires to remove 2 resistors on the board.

Make it simple and instead use a board with arduino_i2c port
available.

Signed-off-by: Erwan Gouriou <[email protected]>
@erwango
Copy link
Member Author

erwango commented Nov 25, 2021

@carlescufi Aside from the compliance issue about the typdedef addition that I can't avoid, PR is good to merge.

@carlescufi carlescufi merged commit dd0b133 into zephyrproject-rtos:main Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants