Re: [PATCH v1 02/15] arm64: dts: freescale: imx8mn-var-som: move SD card support to Symphony

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Moving the `&usdhc2` base configuration out of `imx8mn-var-som.dtsi` completely disables SD card support on the `imx8mn-rve-gateway.dts` carrier board.
- [Medium] The patch silently introduces a functional change to the regulator power sequencing by adding `off-on-delay-us = <20000>;` to the SD card VMMC regulator.
--

commit 2f4cda5f00417bf4a0771aca7e26c1a6e92521a3
Author: Stefano Radaelli <stefano.r@variscite.com>

arm64: dts: freescale: imx8mn-var-som: move SD card support to Symphony

The commit moves the USDHC2 node and pinctrl configuration from the SOM device
tree to the Symphony carrier board device tree. It intends to keep the SOM dtsi
limited only to hardware present on the module itself.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> index b07e71d3adeee..7308613a48b2b 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
> @@ -28,6 +28,7 @@ reg_usdhc2_vmmc: regulator-usdhc2-vmmc {
>  		regulator-max-microvolt = <3300000>;
>  		gpio = <&gpio4 22 GPIO_ACTIVE_HIGH>;
>  		enable-active-high;
> +		off-on-delay-us = <20000>;
>  	};

[Severity: Medium]
Is it intentional to add off-on-delay-us here without mentioning it in the
commit message? This introduces a functional change to the regulator power
sequencing that seems unrelated to simply moving the nodes between files.

[ ... ]

> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mn-var-som.dtsi
> index 32f101ef28500..e1d2e2a72e1f5 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn-var-som.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som.dtsi
> @@ -295,20 +295,6 @@ brcmf: wifi@1 {
>  	};
>  };
>  
> -/* SD */
> -&usdhc2 {
> -	assigned-clocks = <&clk IMX8MN_CLK_USDHC2>;
> -	assigned-clock-rates = <200000000>;
> -	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> -	pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
> -	pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
> -	pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>;
> -	cd-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
> -	bus-width = <4>;
> -	vmmc-supply = <&reg_usdhc2_vmmc>;
> -	status = "okay";
> -};
> -

[Severity: High]
Does moving this base configuration out of the SOM dtsi break SD card support
on other carrier boards like the RVE gateway?

The imx8mn-rve-gateway.dts relies on the SOM dtsi setting status = "okay" and
configuring base hardware requirements for &usdhc2. Because the SoC devicetree
imx8mn.dtsi sets status = "disabled" for usdhc2 by default, moving this
enabling and setup exclusively to the Symphony board leaves the RVE gateway
with a disabled SD card interface lacking clock assignments and pinmux
configurations.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783067947.git.stefano.r@variscite.com?part=2