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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The patch completely omits necessary `bootph-all` tags for the `stm32746g-eval.dts` board.
- [High] Missing `bootph-all` on SDIO controller nodes despite the patch claiming to support SD boot.
- [High] The `&soc` parent bus node lacks the `bootph-all` tag in `stm32f769-disco.dts` and `stm32746g-eval.dts`.
- [High] The patch tags `pwrcfg` instead of the `syscfg` node claimed in the commit message, breaking `pinctrl` initialization.
- [Low] The patch erroneously adds `bootph-all` tags to display-related nodes (`&ltdc` and `&dsi`), which are unrelated to SD boot.
--

commit 616809e6aaaad31d633ea36ad66133cc44b7b361
Author: Patrice Chotard <patrice.chotard@foss.st.com>

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

The bootph-all flag was introduced in dt-schema to define node usage
across different boot phases. To ensure SD boot, timer, gpio, syscfg,
clock, and uart nodes need to be present in all boot stages, so this
patch adds the missing flags.

> diff --git a/arch/arm/boot/dts/st/stm32746g-eval.dts b/arch/arm/boot/dts/st/stm32746g-eval.dts
> --- a/arch/arm/boot/dts/st/stm32746g-eval.dts
> +++ b/arch/arm/boot/dts/st/stm32746g-eval.dts
[ ... ]
> @@ -226,6 +226,16 @@ &usart1 {
>  	status = "okay";
>  };
>  
> +&usart1_pins_a {
> +	bootph-all;
> +	pins1 {
> +		bootph-all;
> +	};
> +	pins2 {
> +		bootph-all;
> +	};
> +};

[Severity: High]
The commit message mentions adding tags to timer, gpio, syscfg, clock, and
uart nodes, but for this board only the usart1 pin configuration is modified.

Are controllers like &usart1, &timers5, &clk_hse, &pinctrl, &gpio*, &rcc,
and &soc also supposed to be tagged here?

Without these tags, the early bootloader (SPL) might drop these nodes,
causing the SD boot sequence to fail for this board.

Additionally, since the explicit goal of the commit is to support SD boot,
should the SD/MMC controller nodes (like &sdio1 and &sdio2) also be
tagged with bootph-all?

Without the tag, the device tree tooling for U-Boot SPL will prune the
SDIO nodes entirely, leaving the bootloader without knowledge of the SD
hardware.

> diff --git a/arch/arm/boot/dts/st/stm32f746-disco.dts b/arch/arm/boot/dts/st/stm32f746-disco.dts
> --- a/arch/arm/boot/dts/st/stm32f746-disco.dts
> +++ b/arch/arm/boot/dts/st/stm32f746-disco.dts
[ ... ]
> @@ -179,6 +224,7 @@ touchscreen@38 {
>  &ltdc {
>  	pinctrl-0 = <&ltdc_pins_a>;
>  	pinctrl-names = "default";
> +	bootph-all;
>  	status = "okay";

[Severity: Low]
Is bootph-all required on the LCD-TFT Display Controller for SD boot?

Retaining this node bloats the memory-constrained early bootloader device
tree (SPL) with display hardware nodes that may not be functionally
necessary for booting.

>  	port {
[ ... ]
> @@ -188,6 +234,22 @@ ltdc_out_rgb: endpoint {
>  	};
>  };
>  
> +&pinctrl {
> +	bootph-all;
> +};
> +
> +&pwrcfg {
> +	bootph-all;
> +};

[Severity: High]
The commit message indicates that the syscfg node needs to be present in all
boot stages, but &pwrcfg is tagged here instead.

Does this leave the actual syscfg node (syscon@40013800) untagged?

The pinctrl node relies on syscfg via a phandle. If the syscfg node is
pruned in SPL because it lacks a boot phase tag, will the pinctrl driver
fail to probe due to a broken phandle, cascading into GPIO initialization
failures?

> diff --git a/arch/arm/boot/dts/st/stm32f769-disco.dts b/arch/arm/boot/dts/st/stm32f769-disco.dts
> --- a/arch/arm/boot/dts/st/stm32f769-disco.dts
> +++ b/arch/arm/boot/dts/st/stm32f769-disco.dts
[ ... ]
> @@ -140,11 +136,13 @@ &cec {
>  
>  &clk_hse {
>  	clock-frequency = <25000000>;
> +	bootph-all;
>  };
>  
>  &dsi {
>  	#address-cells = <1>;
>  	#size-cells = <0>;
> +	bootph-all;
>  	status = "okay";

[Severity: Low]
Similar to the ltdc node above, is the MIPI DSI host required to be
present during the early SPL boot phase for SD boot?

>  	ports {
[ ... ]
> @@ -199,6 +242,19 @@ ltdc_out_dsi: endpoint {
>  	};
>  };
>  
> +&pinctrl {
> +	bootph-all;
> +};
> +
> +&pwrcfg {
> +	bootph-all;
> +};
> +
> +&rcc {
> +	compatible = "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
> +	bootph-all;
> +};

[Severity: High]
The &soc node was properly tagged in stm32f746-disco.dts, but it appears to
be omitted for stm32f769-disco.dts and stm32746g-eval.dts.

If a parent simple-bus node lacks a boot phase tag in U-Boot SPL processing,
its properties are stripped and the driver model will not bind to the bus.

Should &soc also be tagged with bootph-all here to ensure the children
nodes can probe successfully?

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