STM32: Add missing patches

[nandojve]

Add missing STM patches.

[etienne-lms]

Thanks @nandojve for this update.

It seems cherry pick of commit 3d93882 is missing in your series.

Also, cherry-picking commit "STM32U5 : update HAL on U5 to version 1.6.1" likely requires commit a567837 is reverted first. Also this last commit modifies the flash layout (48kB added in boot_partition) and requires an update in Zephyr file boards/st/b_u585i_iot02a/b_u585i_iot02a_stm32u585xx_ns.dts.

I think mainline commit d090f3cd53 should also be cherry picked.

[nandojve]

Hi @etienne-lms ,

Could you revisit to check if sequence is on now. I'll start to work on the Zephyr PR to open ASAP.
BTW, even with the revert revert I still need to fix manually the update to 1.6.1.

[nandojve]

Hi @etienne-lms ,

I'm not sure if we can keep below without decrease the S or NS size.

#define FLASH_AREA_BL2_SIZE (0x16000) #define FLASH_AREA_BL2_SIZE (0x22000) 

The bootloader will increase to 272k which means that 384*2+512*2 already pass 2MB.
We could decrease S from 384 to 368. Then we have 272+16+2*368+2*512=2MB.

Or should I drop from NS ?

[etienne-lms]

I would be in favor to decrease the NS partitions. In any case, this also means changing the flash layout in TF-M since the secondary partition (either for S and NS) needs to be updated. Actually, from TF-M/MCUBoot there is a unique size value defined for both primary and secondary partitions of a firmware (being either S or NS). Well well well, in the end, it seems we'll have to ask TF-M to update it's flash layout.
Let me cross check that and 'ill come back to you.

[nandojve]

Hi @etienne-lms ,

Did you got some answers about the MCUboot partition size ?

[tomi-font]

Why so many cherry picks are cherry picks of cherry picks? Couldn't we just cherry pick the original commit directly?

[etienne-lms]

The series almost looks good to me but I have a few comments.

Coudl you remove the second (cherry picked from commit ...) lines in commit messages from the following commits to keep only the relevant cherry-pick references:
"Platform: STM32WBA6";
"STM32WBA6 : fix C sources build warning";
"STM32WBA6 : remove duplicated platform_bl2 directives";
"STM32WBA6 : disable MPU before its reconfigured";
"STM32WBA6 : remove unused BL2 macros";
"STM32WBA6: Split firmware and OTP binary images";
"STM32WBA6: Make flash offset gap configurable";
"STM32WBA6: fix typo on MCUBOOT_UPGRADE_STRATEGY config";
"STM32WBA: fix some issue".

In the same scope, fix the cherry-pick reference in commit "STM : Add STM32WBA && STM32U5 drivers to psa-arch-tests":

-(cherry picked from commit b3be2a36bbd1f9ada9b42024f737ca1392630732) +(cherry picked from commit 30fae6a9003b7551f4ad98f0239b0bfcf97cd51d)

Ditto for commit "STM32U5 : TFM doesn't work properly if NS enter/exit low power mode.":

-(cherry picked from commit 666982fc83d3cbb269781e87359f29c890572fc0) +(cherry picked from commit b4f636eac270b07832ee0c50b7b719665103d6d4)

Ditto for commit "STM32WBA6: fix C sources build warning in tfm_hal_platform.c":

-(cherry picked from commit 5a8b7d5747ac043e0fd97a9add488bcb978ed7a7) +(cherry picked from commit f140e4d5810835b8138db410c389c4b87788f81b)

Ditto for commit "STM32WBA: issue related to zephyr project.":

-(cherry picked from commit 6df3ce1d69058af8c472621c9dcc4db8835db3b7) +(cherry picked from commit ec61e691a08bd21d15437ee2cff19f1d4688d5ee)

Ditto for commit "STM : low_level_flash drivers doesn't disable icache":

-(cherry picked from commit 992f9896ac5ba8da0b96ee76644902ef926a4983) +(cherry picked from commit a68b8117235f12568bfd31e9c14b1bf0a1e7f957)

Regarding the 4 latest commits in your series, I would propose to discard them as for now and handle them in a dedicated P-R:

• Commit "STM32U5 : update HAL on U5 to version 1.3.0":
  Induces a regression when building with GCC11 or GCC12 (related to 2f13847), that is fixed by the next commit (bumping to STM32 HAL 1.6.1) but this commit also induces some issues;
• Commit "STM32U5 : update HAL on U5 to version 1.6.1":
  Changes the flash layout so it requires a change in Zephyr b_u585i_iot02a//ns board flash layout.
• Commit "platform: stm32u5xx: Add missing stm32u5a5xx.h":
  Is needed only if former commit is merged;
  • (edited) Actually I don't think it's really needed. At least I manage to build Zephyr b_u585i_iot02a//ns without it, assuming I hack in the flash layout to overcome the issue mentioned above)
• Commit "platform: stm: b_u585i_iot02a: Change flash layout":
  Requires other changes in Zephyr b_u585i_iot02a//ns board flash layout, so preferably addressed through dedicated TF-M and Zephyr P-Rs.

If you really want these 4 commits to be merged:

• Zephyr's b_u585i_iot02a_stm32u585xx_ns.dts DTS file needs to be updated, using the external flash.
• (cherry pick from commit ...) lines in commit messages would need to be fixed for commits:
  "STM32U5 : update HAL on U5 to version 1.3.0" and "STM32U5 : update HAL on U5 to version 1.6.1".
• Cherry picking commit "STM32U5 : update HAL on U5 to version 1.6.1" (f8744e5d8e8d) really needs we revert Zephyr local commit a567837. If we were to merge that commit, to better address this previous review comment, I strongly think you should have a dedicated revert commit before, with an explicit commit message like:

Revert "Revert "stm : fix error on b_u585i_iot02a with TF-Mv2.1.0"" This reverts commit a567837cef762839e88995ceb4bc9ec4634d7def. The reverted commit was local to Zephyr TF-M fork and it needs to be reverted in order to properly cherry-pick mainline commit f8744e5d8e8d ("STM32U5 : update HAL on U5 to version 1.6.1"). Signed-off-by: ... 

Last, there is also another commit that needs to be picked for WBA65 support:
eed47eff0e5a ("platform: stm32wba65i_dk: Fix install for STM32_FLASH_LAYOUT_BEGIN_OFFSET").
Would you be okay to add it to your series? Alternatively, I can create a dedicated P-R for this change.

[nandojve]

Hi @etienne-lms ,

Thank you for your precise review. I hope I did not forget anything.

(edited) Actually I don't think it's really needed. At least I manage to build Zephyr b_u585i_iot02a//ns without it, assuming I hack in the flash layout to overcome the issue mentioned above)

This is not for U585 it is for U5A5, this is why you still can build without any issue.

If you really want these 4 commits to be merged:

Yes, we need to fix all the issues with U5xxx MCU in Zephyr. I'll provide the fixes on Zephyr side.

Would you be okay to add it to your series? Alternatively, I can create a dedicated P-R for this change.

No worries, I'll add as many as necessary. Btw, this series still require the below (which I believe will land tomorrow in mainline):

https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/41827
https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/41828
https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/41829

I'll start to update Zephyr side to get ready.

[etienne-lms]

Hi @nandojve, looks all good to me but some spurious 2nd Change-id: tags in a few commits (each time at last line of the commit message):
"STM32WBA6 : remove duplicated platform_bl2 directives",
"STM32WBA6 : disable MPU before its reconfigured",
"STM32WBA6 : remove unused BL2 macros",
"STM32WBA6: Split firmware and OTP binary images",
"STM32WBA6: Make flash offset gap configurable",
"STM32WBA6: fix typo on MCUBOOT_UPGRADE_STRATEGY config",
"STM32WBA: fix some issue.",
"STM : Add STM32WBA && STM32U5 drivers to psa-arch-tests",
"STM32U5 : TFM doesn't work properly if NS enter/exit low power mode.",
"STM32WBA6: fix C sources build warning in tfm_hal_platform.c",
"STM32WBA: issue related to zephyr project.",
"STM : low_level_flash drivers doesn't disable icache" and
"STM32U5 : update HAL on U5 to version 1.3.0".
With that addressed, you have my approval for the series.

Regarding the remaining changes you needs, let's wait they are merged in TF-M, which should happen soon as you said. If merged before this current P-R is merged, you can still add them to this series.

[nandojve]

Hi @etienne-lms ,

Thank you for review. I'll check again for those.

Regarding the remaining changes you needs, let's wait they are merged in TF-M, which should happen soon as you said. If merged before this current P-R is merged, you can still add them to this series.

They got merged few hours ago. I'll review again and cherry-pick those.

[nandojve]

Hi @etienne-lms ,

I update the series and remove the git hook that always add the change-id after I removed it.
I force a push in Zephyr to see how CI will reacts.

[erwango]

@nashif, @tomi-font Is there anything preventing this PR to be merged ?

[nashif]

@nashif, @tomi-font Is there anything preventing this PR to be merged ?

one the maintainers needs to merge, I only work here....