Zephyr: Unify STM32 configuration files

[zephyrbot]

_Reported by Erwan Gouriou:_

Some differences exist today between configuration files of each stm32 series.

This story should help unifying configuration files and factorizing some parameters when possible.

(Imported from Jira ZEP-2042)

[erwango]

1 - Clean up driver generic flags from _defconfig files

Remove series specific CONFIG flags from various
For instance, remove following line in boards/arm/disco_l475_iot1/disco_l475_iot1_defconfig
CONFIG_I2C_STM32_V2=y
Replace in arch/arm/soc/st_stm32/stm32l4/Kconfig.defconfig.series, by:
if I2C
config I2C_STM32_V2
def_bool y
endif # I2C

Same for (in to be created arch/arm/soc/st_stm32/common/Kconfig.defconfig.series):
CONFIG_PWM_STM32
CONFIG_GPIO_STM32
CONFIG_PINMUX_STM32
CONFIG_SPI_STM32
CONFIG_IWDG_STM32

Move CONFIG_SOC_STM32F10X_CONNECTIVITY_LINE_DEVICE to arch/arm/soc/st_stm32/stm32f1/Kconfig.defconfig.stm32f107xc

2 - Create arch/arm/soc/st_stm32/common/Kconfig.series

to factorize
select CPU_CORTEX_M
select SOC_FAMILY_STM32
select SYS_POWER_LOW_POWER_STATE_SUPPORTED
select HAS_STM32CUBE
select CPU_HAS_SYSTICK
select CLOCK_CONTROL_STM32_CUBE if CLOCK_CONTROL

3 - Clean up _defconfig files from instance configs

Use 96b_carbon as example

[erwango]

@superna9999 , @ydamigos , @mbolivar, @lowlander
Some clean-up would be nice in STM32 configuration files.
Your inputs/ideas are welcome before proceeding

[superna9999]

Seems OK for me.

We still need to define the defconfig policy for the nucleo/discovery boards concerning the i2c/spi ports.
Since they can be mapped at multiple ports, we should no enforce a specific pinmux config, no ?

This should be done in an eventual HAT dt overlay/config fragment.

[ydamigos]

I started cleaning up stm32f4 family boards and I believe we should also move to boards' defconfig from series defconfig the following flags:

CONFIG_CONSOLE
CONFIG_UART_CONSOLE

What about GPIO_STM32_PORT* flags? Should we enable them per board? Are there boards which doesn't use all the enabled ports?

[ydamigos]

I pushed PR #5515 to address the first step.

[ydamigos]

Enabling the various controllers in Kconfig.defconfig.series, it causes tests (like tests/drivers/build_all) to fail on some boards (e.g. nucleo_f103rb, stm3210c_eval, etc.) because SPI and I2C are not configured on these boards. We could either enable SPI and I2C on them or disable the tests in boards yaml files. I prefer the first option but I don't have the hardware to test it.

[mbolivar]

Happy new year, everyone!

@erwango:

Remove series specific CONFIG flags from various

+1.

I guess this also means checking samples/ (and maybe tests/) for cases where the SOC-specific flags are enabled in prj.conf when no longer needed.

Create arch/arm/soc/st_stm32/common/Kconfig.series

+1.

Clean up _defconfig files from instance configs

I'm not sure what "instance configs" means, sorry. If this means using the Kconfig.defconfig pattern:

if FOO

config STM32_FOO
    default y

endif # FOO

as is used in 96b_carbon instead of just putting STM32_FOO=y in the _defconfig, I'm +1.

@superna9999:

We still need to define the defconfig policy for the nucleo/discovery boards concerning the i2c/spi ports.
Since they can be mapped at multiple ports, we should no enforce a specific pinmux config, no ?

I assume restricting to nucleo/discovery is to limit scope to ST-designed boards, and that makes sense to me.

We should consider that other pinmux (USARTx, TIMx_CHy, for example) can also be remapped, and make sure the policy is generic. (I guess this type of policy is useful for all SoCs, not just STM32).

@ydamigos:

I started cleaning up stm32f4 family boards and I believe we should also move to boards' defconfig from series defconfig the following flags:

CONFIG_SERIAL
CONFIG_CONSOLE
CONFIG_UART_CONSOLE

+1. I just looked at PR5515; nice stuff.

What about GPIO_STM32_PORT* flags? Should we enable them per board? Are there boards which doesn't use all the enabled ports?

Looks like you chose to put these in the board files. Users may want to override that (e.g. in extreme situations, to save a little space), but that should be possible with extra .conf file lines.

Enabling the various controllers in Kconfig.defconfig.series, it causes tests (like tests/drivers/build_all) to fail on some boards (e.g. nucleo_f103rb, stm3210c_eval, etc.) because SPI and I2C are not configured on these boards. We could either enable SPI and I2C on them or disable the tests in boards yaml files. I prefer the first option but I don't have the hardware to test it.

Does this mean something like enabling the SPI driver in the STM32F4 Kconfig.def.config.series? If so I don't like this for code bloat reasons. The driver initialization functions can't be optimized out by the gc-sections option to the linker.

[ydamigos]

@mbolivar:

Looks like you chose to put these in the board files. Users may want to override that (e.g. in extreme situations, to save a little space), but that should be possible with extra .conf file lines.

I chose to put it in the board files, but it is under discussion, if someone has a different opinion.

Does this mean something like enabling the SPI driver in the STM32F4 Kconfig.def.config.series? If so I don't like this for code bloat reasons. The driver initialization functions can't be optimized out by the gc-sections option to the linker.

Yes and no. I will try to make my comment more clear. Enabling the SPI driver in common Kconfig.defconfig.series broke same tests for some boards (e.g. 96b_carbon in the STM32F4 family). So, for 96b_carbon I added:

if SPI

config SPI_2
    default y

config SPI_2_IRQ_PRI
    default 5

endif # SPI

in boards Kconfig.defconfig file to fix it. But, other boards does not enable support for I2C or SPI driver (e.g. pinmux configuration is missing for I2C or SPI, i2c is not enabled in board's dts file).

[mbolivar]

Enabling the SPI driver in common Kconfig.defconfig.series broke same tests for some boards (e.g. 96b_carbon in the STM32F4 family).

I thought you might have meant that, but I can compile tests/drivers/build_all for nucleo_f103rb (which was one of your examples) with the code in your PR #5515, so I don't see the breakage. I'm assuming by test "failure" you mean a build break since you say you don't have HW to test. What am I missing?

[ydamigos]

I thought you might have meant that, but I can compile tests/drivers/build_all for nucleo_f103rb (which was one of your examples) with the code in your PR #5515, so I don't see the breakage. I'm assuming by test "failure" you mean a build break since you say you don't have HW to test. What am I missing?

Yes, I mean a build break. Sample "samples/sensor/sx9500" doesn't compile on nucleo_f103b. You could also check the shippable logs for PR #5515 to find out which sample or test create a build failure on which board.

[mbolivar]

I think there's a way in the test case YAML to filter out based on whether a board supports a peripheral. There's got to be some way to make sure the relevant boards say they don't (i.e. default is "no" at SoC level, boards can turn it on if appropriate), right?

[ydamigos]

I think there's a way in the test case YAML to filter out based on whether a board supports a peripheral. There's got to be some way to make sure the relevant boards say they don't (i.e. default is "no" at SoC level, boards can turn it on if appropriate), right?

According to the documentation the option depends on for the testcases:

depends_on: <list of features>
    A board or platform can announce what features it supports, this option will enable the test only those platforms that provide this feature.

and the option supported in the boards yaml file should do the trick.

supported:
    A list of features this board supports. This can be specified as a single word feature or as a variant of a feature class. 

I will give it a try.

[erwango]

Best wishes to you all!
Thanks for your interest on that issue and thanks @ydamigos for your PR. I'll try to provide a review today, but this is definitely an interesting work.
Here are some replies to the discussion above. Let's see if we can define some generic and coherent rules, keeping in mind that some of these CONFIG flags will be moved to dts in medium/long term.

@superna9999,

We still need to define the defconfig policy for the nucleo/discovery boards concerning the i2c/spi ports. Since they can be mapped at multiple ports, we should no enforce a specific pinmux config, no ?

About pin config for nucleo boards and all disco boards featuring an arduino connector, I think it would make sense to configure by default the pinmux to match arduino connections: I2C (D14/D15), SPI(D10/11/12/13), UART (D1/D0) and PWM. Then we'd be in line with the slikscreen and it would allow direct use of arduino shields. What do you think?

@mbolivar ,

I'm not sure what "instance configs" means, sorry. If this means using the Kconfig.defconfig pattern [...] as is used in 96b_carbon instead of just putting STM32_FOO=y in the _defconfig, I'm +1.

This was the idea. I think @superna9999 also mentioned once that board/Kconfig.defconfig should hold BSP configuration (by default on the board, like BT stuff on carbon or console for nucleo boards) and board/_defconfig should be limited to HW ready with pinmux left to the user. I think this is a simple and good rule.

@ydamigos , @mbolivar

What about GPIO_STM32_PORT* flags? Should we enable them per board? Are there boards which doesn't use all the enabled ports?

Looks like you chose to put these in the board files. Users may want to override that (e.g. in extreme situations, to save a little space), but that should be possible with extra .conf file line

I was thinking about putting GPIO definitions in arch/arm/... So in _defconfig, only CONFIG_GPIO appears. Then, in generic case, they are all activated or all deactivated. For specific case and footprint optimisation, user can tune his use case and disable in board _defconfig.

[ydamigos]

@erwango Best wishes!!!

About pin config for nucleo boards and all disco boards featuring an arduino connector, I think it would make sense to configure by default the pinmux to match arduino connections: I2C (D14/D15), SPI(D10/11/12/13), UART (D1/D0) and PWM. Then we'd be in line with the slikscreen and it would allow direct use of arduino shields. What do you think?

+1, it would be good to follow the same approach on other boards (e.g. olimexino-stm32) which featuring an arduino connector. We could do the same for 96Boards expansion connectors.

I was thinking about putting GPIO definitions in arch/arm/... So in _defconfig, only CONFIG_GPIO appears. Then, in generic case, they are all activated or all deactivated. For specific case and footprint optimisation, user can tune his use case and disable in board _defconfig.

I will move GPIO definition for ports A, B, C, D in common Kconfig.defconfig.series and enable the rest ports per SOC in SOC's Kconfig.defconfig.series or Kconfig.defconfig.stm32*.

[mbolivar]

This was the idea. I think @superna9999 also mentioned once that board/Kconfig.defconfig should hold BSP configuration (by default on the board, like BT stuff on carbon or console for nucleo boards) and board/_defconfig should be limited to HW ready with pinmux left to the user. I think this is a simple and good rule.

I agree in principle, but there is a problem in practice as described in the commit log for ef022d39b4:

Similarly, provide BT_SPI_* configs to integrate with the BT HCI SPI
driver. The files these config values apply to only get built when
CONFIG_BT=y, but this configuration can't be handled in the "if BT"
section in 96b_carbon's Kconfig.defconfig.  This is because BT_SPI is
a choice value, and thus doesn't support a default setting.

So some CONFIG_BT_SPI values for Carbon need to be in the defconfig, even though by this rule they would go in the Kconfig.defconfig.

I was thinking about putting GPIO definitions in arch/arm/... So in _defconfig, only CONFIG_GPIO appears. Then, in generic case, they are all activated or all deactivated. For specific case and footprint optimisation, user can tune his use case and disable in board _defconfig.

That seems reasonable. Optimization should also be possible in prj.conf on an application-specific basis.

[erwango]

@superna9999, @mbolivar , @ydamigos

So it seems we're aligned. Let me sum it up:

GPIO port configuration (port activation) would be done at SoC level:

• in arch/arm/soc/st_stm32/common/Kconfig.defconfig.series for port ABCD
• in arch/arm/soc/st_stm32/stm32xx/Kconfig.defconfig.<_soc_> for other ports
  Then, when CONFIG_GPIO is selected, all ports will be activated by default. Optimisation should take place at board level or in prj.conf on an application-specific basis

Board configuration would be done as follows:

• in boards/arm/<_board_>/<_board_>_defconfig
  -- SoC configuration
  -- Kernel configuration
  -- Driver activation
  -- Specific driver configuration that cannot be done in Kconfig.defconfig due to use of 'choice' type
• in boards/*/<_board_>/Kconfig.defconfig
  --Driver configuration
  --Configuration of peripherals available directly on board (sensors, communication chip like BT, WiFi, ST Link, ..)

Finally, configuration would be done as follows:

• pin configuration for peripheral available on boards
• pin configuration matching silkscreen, focusing on arduino connectors

Is that ok for you?

[ydamigos]

+1. Just a small comment. Port D is not available for all SOCs so, I wouldn't configure it in common/Kconfig.defconfig.series.

[superna9999]

Ok for me ! :shipit:

[mbolivar]

+1, and thanks! This will be a great cleanup.

[erwango]

Fixed by #5515.
I'm closing this one and opening a new one for pin configuration