arch/stm32h7: add CM4 core support

[raiden00pl]

Summary

• armv7-m/mpu.h: add macro to configure shared memory region
• stm32h7: add HSEM support
• arch/stm32h7/rcc: default value for BOARD_FLASH_PROGDELAY
• arch/stm32h7: add an option to bypass clock configuration
• arch/stm32h7: use STM32_CPUCLK_FREQUENCY to initialize perf
• arch/stm32h7: add CM4 core support
• arch/stm32h7: add RPTUN support
• boards/stm32h7: add nucleo-h745zi
• boards: disable CM4 for stm32h745i-disco and stm32h747i-disco

Impact

Boards that use H7 dual core chips but they don't care about CM4 should set CONFIG_STM32H7_CORTEXM4_ENABLED=n.
I keep this option disabled at default, because using dual core with CM4 disabled probably makes no sense.

Should have no impact on single core H7 but configurations need refreshing

Testing

cmake -B build_h7m7 -DBOARD_CONFIG=nucleo-h745zi:nsh_cm7_rptun -GNinja
cmake -B build_h7m4 -DBOARD_CONFIG=nucleo-h745zi:nsh_cm4_rptun -GNinja

fix for not patched openamp issue: - issue solved #10241

rm openamp/.openamp_patch
cmake --build build_h7m4 -t menuconfig

cmake --build build_h7m7
cmake --build build_h7m4

flash CM4 and CM7 hex with JFlashLiteExe, device must be selected to STM32H745ZI_M7

[davids5]

@raiden00pl Nice work.

My comments are related to adding dual core code to non dual code CPUS and avoiding the stm32 issues of not bifurcating the code sooner.

I find CONFIG_ARCH_CHIP_STM32H7_CORTEXM7 confusing. I am wondering if something like CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 and
CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M4 would be better?

[raiden00pl]

I find CONFIG_ARCH_CHIP_STM32H7_CORTEXM7 confusing. I am wondering if something like CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 and
CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M4 would be better?

@davids5 I'm not sure about that. CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 can also be confusing as we need that flag in both dual core and single core scenario.
We can drop DUAL_CORE part and have CONFIG_ARCH_CHIP_STM32H7_IS_M7 but I don't know if it's better than what it is now. CORTEXM7/CORTEXM4 seems to be more obvious.
Maybe CONFIG_ARCH_CHIP_STM32H7_IS_CORTEXM7 or CONFIG_ARCH_CHIP_STM32H7_IS_CM7 ?

CONFIG_ARCH_CHIP_STM32H7_CORTEXM7 is an evolution of CONFIG_ARCH_CORTEXM7

[davids5]

I find CONFIG_ARCH_CHIP_STM32H7_CORTEXM7 confusing. I am wondering if something like CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 and
CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M4 would be better?

@davids5 I'm not sure about that. CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 can also be confusing as we need that flag in both dual core and single core scenario. We can drop DUAL_CORE part and have CONFIG_ARCH_CHIP_STM32H7_IS_M7 but I don't know if it's better than what it is now. CORTEXM7/CORTEXM4 seems to be more obvious. Maybe CONFIG_ARCH_CHIP_STM32H7_IS_CORTEXM7 or CONFIG_ARCH_CHIP_STM32H7_IS_CM7 ?

CONFIG_ARCH_CHIP_STM32H7_CORTEXM7 is an evolution of CONFIG_ARCH_CORTEXM7

In my mind. I want to compartmentalize the dual core. So if that part number of the part is a dual core I see dual core files and settings. If it is not I do not. I feel the naming of things that are specific to dual code, should indicate that. Hence adding something like CONFIG_ARCH_CHIP_STM32H7_DUAL_CORE_IS_M7 helps with that compartmentalization. The actual name could be more succinct. But the idea is where there is an intersection in dual core/non dual core is is clear.

@raiden00pl I am not as close to the problem or the solution space as you are, but my gut reaction is to duplicate the header files and remove some of the x in the naming to the fully defined STM32H745/755 and STM32H747/757 naming.

[raiden00pl]

@raiden00pl I am not as close to the problem or the solution space as you are, but my gut reaction is to duplicate the header files and remove some of the x in the naming to the fully defined STM32H745/755 and STM32H747/757 naming.

I don't think there is any need to duplicate the headers. The headers are the same for H7 chips except for literally a few extra bits when CM4 is supported.

[davids5]

@raiden00pl - Thank you!

[hartmannathan]

Reviewing...

[hartmannathan]

I understand that the desire is to enable the Cortex M4 core by default, but allow the option to disable it at Kconfig-time. However, CONFIG_STM32H7_CORTEXM4_DISABLED causes many preprocessor conditionals to be in the form #if !defined(CONFIG_STM32H7_CORTEXM4_DISABLED) where the word "DISABLED" is very prominent and the ! makes it a double-negative, so we see the word "DISABLED" but it really means "ENABLED"... Wouldn't it be better to call it CONFIG_STM32H7_CORTEXM4_ENABLED, enable it by default in Kconfig, and remove the double-negative from all the preprocessor conditionals? I know it's a lot of code churn for nothing, except (hopefully) being a little more clear in the future.

Also it might be nice to add some Documentation to clarify that NuttX needs to be built twice, once for each core. Right now, the only documentation of this fact is in the PR comments, and someone looking at the code in the future will have a difficult time figuring that out. This is already a pretty big PR so it's okay to add this kind of Documentation in a separate PR, but maybe add at least a comment or a hint somewhere, such as in the ---help--- text of ARCH_CHIP_STM32H7 or something?

[hartmannathan]

Suggested change

	* LEDs. If CONFIG_ARCH_LEDS is not defined, then the board_userled() is
	* LEDs. If CONFIG_ARCH_LEDS is not defined, then the board_userled() is

[hartmannathan]

Suggested change

	* available to control the LED from user application logic.
	* available to control the LED from user application logic.

[hartmannathan]

Suggested change

	* LEDs. If CONFIG_ARCH_LEDS is not defined, then the board_userled_all()
	* is available to control the LED from user application logic. NOTE: since
	* there is only a single LED on-board, this is function is not very useful.
	* LEDs. If CONFIG_ARCH_LEDS is not defined, then the board_userled_all()
	* is available to control the LED from user application logic. NOTE:
	* since there is only a single LED on-board, this is function is not very
	* useful.

[xiaoxiang781216]

@raiden00pl please rebase your patch to the last master, ci break is fixed.

[raiden00pl]

I understand that the desire is to enable the Cortex M4 core by default, but allow the option to disable it at Kconfig-time. However, CONFIG_STM32H7_CORTEXM4_DISABLED causes many preprocessor conditionals to be in the form #if !defined(CONFIG_STM32H7_CORTEXM4_DISABLED) where the word "DISABLED" is very prominent and the ! makes it a double-negative, so we see the word "DISABLED" but it really means "ENABLED"... Wouldn't it be better to call it CONFIG_STM32H7_CORTEXM4_ENABLED, enable it by default in Kconfig, and remove the double-negative from all the preprocessor conditionals? I know it's a lot of code churn for nothing, except (hopefully) being a little more clear in the future.

I agree, it makes sense. Changed.

Also it might be nice to add some Documentation to clarify that NuttX needs to be built twice, once for each core. Right now, the only documentation of this fact is in the PR comments, and someone looking at the code in the future will have a difficult time figuring that out. This is already a pretty big PR so it's okay to add this kind of Documentation in a separate PR, but maybe add at least a comment or a hint somewhere, such as in the ---help--- text of ARCH_CHIP_STM32H7 or something?

doc will come later. First we have to solve #10241 to avoid some dirty hacks for build process. I assume this PR will only be used with cmake, compiling 2 images with make is too annoying.

[slorquet]

It would be good to avoid cmake-specific features.

Is make a supported build system or was it deprecated?

If it's supported no feature should work only with cmake. Thanks.

[xiaoxiang781216]

It would be good to avoid cmake-specific features.

Is make a supported build system or was it deprecated?

If it's supported no feature should work only with cmake. Thanks.

The change should work with make too. But make doesn't support out of tree build, it's very annoying to develop AMP system like this patch because you have to make distclean and make every time.

[hartmannathan]

It would be good to avoid cmake-specific features.

Is make a supported build system or was it deprecated?

If it's supported no feature should work only with cmake. Thanks.

The change should work with make too. But make doesn't support out of tree build, it's very annoying to develop AMP system like this patch because you have to make distclean and make every time.

I started a discussion on the mailing list that is relevant to this: "[DISCUSS] Out-of-tree builds should be standard?" archived here: https://lists.apache.org/thread/l8mzkglx2dldx5jtxqpqtwmvyl4f4z8p

[raiden00pl]

@slorquet this PR includes make support, but there are some problems with OpenAMP build now (#10401).

Without out of tree build it's a waste of time and without this specific cmake feature this PR would never have been created. I had dual core H7 in my plans for a long time, but the building process was too annoying (and sloooow) to take on it.

I have no problem modifying both cmake and make, unless support for make is too much work.
There was a minimum of work here, but otherwise I'm not going to spend hours writing dedicated makefiles (unless someone pays me for this tedious job).

[raiden00pl]

UPDATE: this PR is not a breaking change, but the old configurations need refreshing. I wanted to avoid this, but Kconfig always add CONFIG_ARCH_CHIP_STM32H7_CORTEXM7=y to the refreshed configurations (probably the default behavior for choice options).