Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for STMicroElectronics Nucleo-F439ZI #85679

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

man-gc
Copy link
Contributor

@man-gc man-gc commented Feb 12, 2025

This board is the same as the Nucleo-F429ZI but with a STM32F439ZI SOC. The only difference between this SoC and the STM32F429ZI is the presence of a hardware cryptographic accelerator.

@zephyrbot zephyrbot added the platform: STM32 ST Micro STM32 label Feb 12, 2025
@man-gc man-gc force-pushed the port-nucleo_stm32f439zi branch 2 times, most recently from c39415d to 7307df1 Compare February 12, 2025 15:27
Copy link
Collaborator

@JarmouniA JarmouniA left a comment

Choose a reason for hiding this comment

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

Some initial remarks.

@man-gc
Copy link
Contributor Author

man-gc commented Feb 13, 2025

@JarmouniA I've read your initial remarks but, before I proceed to update these commits, I want to say that all files have been directly copied from the in-tree Nucleo-F429ZI.
Therefore, all remarks that apply here should also be applied to this board, shouldn't they ? Or maybe I should update the documentation to link to the other board, and simply mention that this board only provide a hardware cryptographic accelerator ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

convert all to webp, then put through https://tinypng.com/

Supported Features
==================

The Zephyr nucleo_f439zi board configuration supports the following hardware features:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The Zephyr nucleo_f439zi board configuration supports the following hardware features:
The Zephyr ``nucleo_f439zi`` board supports the following hardware features:

@JarmouniA
Copy link
Collaborator

I want to say that all files have been directly copied from the in-tree Nucleo-F429ZI. Therefore, all remarks that apply here should also be applied to this board, shouldn't they ?

@man-gc I think the images with the pin info shouldn't be in the other board either, the pins are assigned in the board's devicetree.

Or maybe I should update the documentation to link to the other board, and simply mention that this board only provide a hardware cryptographic accelerator ?

Yes, I think that would be best, instead of duplicating everything without obvious added value.

@man-gc man-gc force-pushed the port-nucleo_stm32f439zi branch from 7307df1 to add4322 Compare February 17, 2025 14:14
@kartben kartben added this to the v4.2.0 milestone Feb 17, 2025
@man-gc man-gc requested review from nordicjm and JarmouniA February 21, 2025 07:55
nordicjm
nordicjm previously approved these changes Feb 24, 2025
@man-gc
Copy link
Contributor Author

man-gc commented Feb 25, 2025

@JarmouniA Due to the flash layout of the STM32F439ZI, would you be OK with the following partitioning for MCUBoot default Zephyr config ?

&flash0 {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		/* 32KB for bootloader */
		boot_partition: partition@0 {
			label = "mcuboot";
			reg = <0x00000000 DT_SIZE_K(32)>;
			read-only;
		};

		/*
		 * nvs subsystem requires 2 sectors with a max total of 32K
		 * On F4 series, the only option is to use the following
		 * partition, which is compatible with mcuboot usage.
		 * Keep it commented in order it is not used by CI.
		 *
		 * storage_partition: partition@8000 {
		 *	label = "storage";
		 *	reg = <0x0008000 DT_SIZE_K(32)>;
		 * };
		 */

		/* free sector: 64KB
		 *
		 * free0_partition: partition@10000 {
		 *	label = "free-0";
		 *	reg = <0x00010000 DT_SIZE_K(64)>;
		 * };
		 */

		/* application image slot: 384KB */
		slot0_partition: partition@20000 {
			label = "image-0";
			reg = <0x00020000 DT_SIZE_K(384)>;
		};

		/* free sectors: 4 * 16KB + 64KB
		 *
		 * free1_partition: partition@10000 {
		 *	label = "free-1";
		 *	reg = <0x00010000 DT_SIZE_K(128)>;
		 * };
		 */

		/* backup slot: 256KB */
		slot1_partition: partition@a0000 {
			label = "image-1";
			reg = <0x000a0000 DT_SIZE_K(256)>;
		};

		/* free sector: 128KB
		 *
		 * free2_partition: partition@e0000 {
		 *	label = "free-2";
		 *	reg = <0x000e0000 DT_SIZE_K(128)>;
		 * };
		 */
	};
};

It seems also it would be better to use MCUBOOT_BOOTLOADER_MODE_SWAP_USING_OFFSET with this board in order to have two image slots of 384KB. Maybe I should modify some Kconfig to set it and use the following partitioning to better use the available flash space

&flash0 {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		/* 64KB for bootloader */
		boot_partition: partition@0 {
			label = "mcuboot";
			reg = <0x00000000 DT_SIZE_K(64)>;
			read-only;
		};

		/* free sector: 64KB
		 *
		 * free0_partition: partition@10000 {
		 *	label = "free-0";
		 *	reg = <0x00010000 DT_SIZE_K(64)>;
		 * };
		 */

		/* application image slot: 384KB */
		slot0_partition: partition@20000 {
			label = "image-0";
			reg = <0x00020000 DT_SIZE_K(384)>;
		};

		/*
		 * Keep it commented in order it is not used by CI.
		 *
		 * storage_partition: partition@80000 {
		 *	label = "storage";
		 *	reg = <0x0080000 DT_SIZE_K(64)>;
		 * };
		 */

		/* free sector: 64KB
		 *
		 * free1_partition: partition@90000 {
		 *	label = "free-1";
		 *	reg = <0x00090000 DT_SIZE_K(64)>;
		 * };
		 */

		/* backup slot: 384KB */
		slot1_partition: partition@a0000 {
			label = "image-1";
			reg = <0x000a0000 DT_SIZE_K(384)>;
		};
	};
};

What do you think before I push another version for this PR ?

@man-gc
Copy link
Contributor Author

man-gc commented Feb 25, 2025

@JarmouniA Due to the flash layout of the STM32F439ZI, would you be OK with the following partitioning for MCUBoot default Zephyr config ?

[...]

It seems also it would be better to use MCUBOOT_BOOTLOADER_MODE_SWAP_USING_OFFSET with this board in order to have two image slots of 384KB. Maybe I should modify some Kconfig to set it and use the following partitioning to better use the available flash space

[...]

What do you think before I push another version for this PR ?

Sorry, this is using the 1MB version and not the 2MB one, so the offsets must be recalculated accordingly

FRASTM
FRASTM previously approved these changes Feb 28, 2025
@man-gc
Copy link
Contributor Author

man-gc commented Mar 4, 2025

Hi all,

I've seen that this PR now has 2 approvals but before merging, should I fix tests and force push ? Also, which partitioning should I use ? Or, should these modifications be done in another PR ?

I've also seen the recent PR #86485, should documentation of this board be modified accordingly ?

man-gc added 2 commits March 4, 2025 08:49
STM32F439 SoC is an STM32F429 with an integrated crypto/hash processor
providing hardware acceleration for encryption (AES and TDES) and hash
(MD5, SHA-1 and SHA-2).

Signed-off-by: Mathieu Anquetin <[email protected]>
Since this SoC is equivalent to the STM32F429 SoC, simply include its
device tree and add the 'cryp' node for the hardware cryptographic
accelerator.

Signed-off-by: Mathieu Anquetin <[email protected]>
@erwango
Copy link
Member

erwango commented Mar 4, 2025

I've seen that this PR now has 2 approvals but before merging, should I fix tests and force push ?

Indeed, PR can't be merged w/o a clean CI sheet.

I've also seen the recent PR #86485, should documentation of this board be modified accordingly ?

Exactly, thanks.

Add support for the nucleo_f439zi board.

Signed-off-by: Mathieu Anquetin <[email protected]>
@man-gc man-gc dismissed stale reviews from FRASTM and nordicjm via 2291a71 March 4, 2025 10:49
@man-gc man-gc force-pushed the port-nucleo_stm32f439zi branch from add4322 to 2291a71 Compare March 4, 2025 10:49
@zephyrbot zephyrbot added area: ADC Analog-to-Digital Converter (ADC) area: DAC Digital-to-Analog Converter labels Mar 4, 2025
@zephyrbot zephyrbot requested review from anangl and martinjaeger March 4, 2025 10:49
@man-gc man-gc requested review from JarmouniA, nordicjm and FRASTM March 4, 2025 10:50
@martinjaeger martinjaeger removed their request for review March 5, 2025 19:29
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Some comments on the doc, but otherwise this is a clean work, thanks!

Comment on lines +22 to +24
This board provides the same features as the
:zephyr:board:`nucleo_f429zi`. You can refer to it for further
information.
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Please remove. Each board should provide an independent documentation, adn in current case, it will be provided by the generated table.

*************************

The Nucleo F439ZI board includes an ST-LINK/V2-1 embedded debug tool interface.

Copy link
Member

Choose a reason for hiding this comment

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

Please add build instructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Boards area: DAC Digital-to-Analog Converter platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants