Re: [PATCH v7 1/7] ARM: dts: stm32: Add boot phase tags for STMicroelectronics f4 boards

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The `&sdio` controller nodes are missing the `bootph-all` property, contradicting the stated goal of supporting SD boot.
- [Medium] The `bootph-all` flag is missing for the clock nodes (e.g., `&clk_hse`) in `stm32f469-disco.dts`.
- [Medium] UART controller nodes are omitted from `bootph-all` tagging despite the commit message claiming otherwise.
- [Low] The `bootph-all` property is incorrectly added to the LCD-TFT display controller (`&ltdc`) in `stm32f469-disco.dts`.
- [Low] A duplicate `&rcc` node reference is created in `stm32f469-disco.dts`.
--

commit 7a940b6d4b0cb2432e629736b8cac85e1dd3638f
Author: Patrice Chotard <patrice.chotard@foss.st.com>

ARM: dts: stm32: Add boot phase tags for STMicroelectronics f4 boards

This commit introduces the bootph-all flag to several device tree nodes across
STM32 f4 boards. It targets nodes like timer, gpio, syscfg, clock, and uart
to ensure they are present during all boot stages for SD boot support.

> diff --git a/arch/arm/boot/dts/st/stm32429i-eval.dts b/arch/arm/boot/dts/st/stm32429i-eval.dts
> index f4b1c4eb64f28..8a08b9f6b8371 100644
> --- a/arch/arm/boot/dts/st/stm32429i-eval.dts
> +++ b/arch/arm/boot/dts/st/stm32429i-eval.dts
[ ... ]
> @@ -293,6 +358,10 @@ &sdio {
>  	max-frequency = <12500000>;
>  };

[Severity: High]
The commit message states the goal is to ensure SD boot, but the &sdio node
does not receive the bootph-all tag here. Will this cause the SDIO controller
to be stripped from the SPL device tree and break early SD boot capabilities?

>  
> +&syscfg {
> +	bootph-all;
> +};
> +
>  &timers1 {
>  	status = "okay";
[ ... ]
> @@ -339,6 +409,16 @@ &usart1 {
>  	status = "okay";
>  };

[Severity: Medium]
The commit message mentions adding the tag to uart nodes. While the pinctrl
configuration for the UART receives the tag below, the core &usart1 node does
not. Could this result in the UART controller being omitted during early boot
stages, breaking early console initialization?

>  
> +&usart1_pins_a {
> +	bootph-all;
> +	pins1 {
> +		bootph-all;
> +	};
[ ... ]
> diff --git a/arch/arm/boot/dts/st/stm32f469-disco.dts b/arch/arm/boot/dts/st/stm32f469-disco.dts
> index 943afba06b5fe..ecd33d6003b34 100644
> --- a/arch/arm/boot/dts/st/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/st/stm32f469-disco.dts

[Severity: Medium]
The commit message states the bootph-all flag is added to clock nodes, and
other boards in this patch update their &clk_hse nodes. Is it missing for
the &clk_hse node in stm32f469-disco.dts, and might this cause the clock to be
stripped out during early boot?

> @@ -181,7 +181,52 @@ dsi_panel_in: endpoint {
[ ... ]
>  &ltdc {
> +	bootph-all;
>  	status = "okay";
>  
>  	port {

[Severity: Low]
Does the LCD-TFT display controller need to be present in early boot phases
like SPL or TPL? Including it might needlessly inflate the device tree size
in SRAM, as display initialization isn't typically required for SD boot.

> @@ -191,10 +236,26 @@ ltdc_out_dsi: endpoint {
>  	};
>  };
>  
> +&pinctrl {
> +	bootph-all;
> +};
> +
> +&pwrcfg {
> +	bootph-all;
> +};
> +
> +&rcc {
> +	bootph-all;
> +};

[Severity: Low]
Does appending this new &rcc block create a duplicate node reference? Since
an &rcc block already exists earlier in the file, would it be better to add
this property to the existing block to keep related properties grouped
together?

> +
>  &rtc {
>  	status = "okay";
>  };
[ ... ]
> @@ -252,6 +314,16 @@ &usart3 {
>  	status = "okay";
>  };

[Severity: Medium]
Similar to the &usart1 node in stm32429i-eval.dts, the core &usart3 node here
is missing the bootph-all tag, even though the pins below receive it. Should
the uart node itself be tagged to ensure it is preserved during early boot?

>  
> +&usart3_pins_a	{
> +	bootph-all;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-upstream_uboot_properties-v7-0-e17cd424d5db@foss.st.com?part=1