Re: [PATCH 1/2] arm64: dts: st: move Engicam MicroGEA-STM32MP257D-RMM display to an overlay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dts: brittle overlay redefining unrelated SPI CS pins
--

commit c7c6310b15d9d720d97a1d231991f453b51ab1cf
Author: Dario Binacchi <dario.binacchi@amarulasolutions.com>

arm64: dts: st: move Engicam MicroGEA-STM32MP257D-RMM display to an overlay

This commit moves the RK050HR345-CT106A RGB panel and its touchscreen
configuration from the base device tree to a separate overlay. It leaves
common board elements intact to support multiple display variants.

> diff --git a/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm-overlay-rk050hr345-ct106a.dtso b/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm-overlay-rk050hr345-ct106a.dtso
> --- /dev/null
> +++ b/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm-overlay-rk050hr345-ct106a.dtso
[ ... ]
> +&spi1 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	cs-gpios = <&gpioh 8 GPIO_ACTIVE_LOW>, <&gpioh 3 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Since this overlay redefines the entire cs-gpios array, including the
unrelated CS1 pin (<&gpioh 3 GPIO_ACTIVE_HIGH>), could this silently revert
and break the CS1 device if the base board's CS1 wiring changes in the
future?

> +
> +	display@0 {
[ ... ]
> diff --git a/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm.dts b/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm.dts
> --- a/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm.dts
> +++ b/arch/arm64/boot/dts/st/stm32mp257d-engicam-microgea-rmm.dts
[ ... ]
> @@ -257,23 +228,9 @@ &spi1 {
>  	pinctrl-1 = <&spi1_sleep_pins_a>;
>  	#address-cells = <1>;
>  	#size-cells = <0>;
> -	cs-gpios = <&gpioh 8 GPIO_ACTIVE_LOW>, <&gpioh 3 GPIO_ACTIVE_HIGH>;
> +	/* CS0 is reserved for the display fitted through overlay */
> +	cs-gpios = <0>, <&gpioh 3 GPIO_ACTIVE_HIGH>;

[Severity: Low]
Does setting the base board's CS0 pin to a <0> placeholder force the overlay
to redefine the entire cs-gpios array? Would it be safer to let the base
board define the full physical cs-gpios array here (including CS0), allowing
the overlay to only append the SPI device node?

>  	status = "okay";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923152435.563925-1-dario.binacchi@amarulasolutions.com?part=1