Re: [PATCH v5 01/10] arm64: dts: lx2160a-cex7/lx2162a-sr-som: fix usd-cd & gpio pinmux

[Josua Mayer <josua@solid-run.com>]

Am 14.03.26 um 13:05 schrieb Josua Mayer:
> Commit 8a1365c7bbc1 ("arm64: dts: lx2160a: add pinmux and i2c gpio to
> support bus recovery") introduced pinmux nodes for lx2160 i2c
> interfaces, allowing runtime change between i2c and gpio functions
> implementing bus recovery.
>
> However, the dynamic configuration area (overwrite MUX) used by the
> pinctrl-single driver initially reads as zero and does not reflect the
> actual hardware state set by the Reset Configuration Word (RCW) at
> power-on.
>
> Because multiple groups of pins are configured from a single 32-bit
> register, the first write from the pinctrl driver unintentionally clears
> all other bits to zero.
>
> For example, on the LX2162A Clearfog, RCWSR12 is initialized to
> 0x08000006. When any i2c pinmux is applied, it clears all other fields.
> This inadvertently disables SD card-detect (IIC2_PMUX) and some GPIOs
> (SDHC1_DIR_PMUX):
>
> LX2162-CF RCWSR12: 0b0000100000000000 0000000000000110
> IIC2_PMUX              |||   |||   || |   |||   |||XXX : I2C/GPIO/CD-WP
> SDHC1_DIR_PMUX         XXX   |||   || |   |||   |||    : SDHC/GPIO/SPI
>
> Reverting the commit in question was considered but bus recovery is an
> important feature.
>
> Instead add pinmux nodes for those pins that were unintentionally
> reconfigured on SolidRun LX2160A Clearfog-CX and LX2162A Clearfog
> boards.
>
> Fixes: 8a1365c7bbc1 ("arm64: dts: lx2160a: add pinmux and i2c gpio to support bus recovery")
> Cc: stable@vger.kernel.org
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  .../arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi |  7 +++++++
>  .../dts/freescale/fsl-lx2160a-clearfog-itx.dtsi    |  2 ++
>  arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi     | 24 ++++++++++++++++++++++
>  .../boot/dts/freescale/fsl-lx2162a-clearfog.dts    |  2 ++
>  .../boot/dts/freescale/fsl-lx2162a-sr-som.dtsi     |  7 +++++++
>  5 files changed, 42 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi
> index eec2cd6c6d32a..7f6e39e27ce5c 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-cex7.dtsi
> @@ -162,6 +162,8 @@ rtc@51 {
>  };
>  
>  &fspi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&fspi_data74_pins>, <&fspi_data30_pins>, <&fspi_dqs_sck_cs10_pins>;
>  	status = "okay";
>  
>  	flash@0 {
> @@ -177,6 +179,11 @@ flash@0 {
>  	};
>  };
>  
> +&pinmux_i2crv {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gpio0_14_12_pins>;
> +};
> +
>  &usb0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi
> index af6258b2fe826..580ee9b3026e3 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-clearfog-itx.dtsi
> @@ -89,6 +89,8 @@ &emdio2 {
>  };
>  
>  &esdhc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&esdhc0_cd_wp_pins>, <&esdhc0_cmd_data30_clk_vsel_pins>;
>  	sd-uhs-sdr104;
>  	sd-uhs-sdr50;
>  	sd-uhs-sdr25;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> index 853b01452813a..af74e77efabc5 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -1721,6 +1721,10 @@ i2c1_scl_gpio: i2c1-scl-gpio-pins {
>  				pinctrl-single,bits = <0x0 0x1 0x7>;
>  			};
>  
> +			esdhc0_cd_wp_pins: iic2-sdhc-pins {
> +				pinctrl-single,bits = <0x0 0x6 0x7>;
> +			};
> +
>  			i2c2_scl: i2c2-scl-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 3)>;
>  			};
> @@ -1753,6 +1757,26 @@ i2c5_scl_gpio: i2c5-scl-gpio-pins {
>  				pinctrl-single,bits = <0x0 (0x1 << 12) (0x7 << 12)>;
>  			};
>  
> +			fspi_data74_pins: xspi1-data74-pins {
> +				pinctrl-single,bits = <0x0 0x0 (0x7 << 15)>;
> +			};
> +
> +			fspi_data30_pins: xspi1-data30-pins {
> +				pinctrl-single,bits = <0x0 0x0 (0x7 << 18)>;
> +			};
> +
> +			fspi_dqs_sck_cs10_pins: xspi1-base-pins {
> +				pinctrl-single,bits = <0x0 0x0 (0x7 << 21)>;
> +			};
> +
> +			esdhc0_cmd_data30_clk_vsel_pins: sdhc1-base-sdhc-vsel-pins {
> +				pinctrl-single,bits = <0x0 0x0 (0x7 << 24)>;
> +			};
Since xspi1 and sdhc1-base pins bits are set zero here, I wonder if this patch for stable
should limit to just the non-zero esdhc0_cd_wp_pins and gpio0_14_12_pins.

But then I need another separate patch linking them to the board dts.

Personally I prefer to explicitly set all bits, including zero, also for stable.

> +
> +			gpio0_14_12_pins: sdhc1-dir-gpio-pins {
> +				pinctrl-single,bits = <0x0 (0x1 << 27) (0x7 << 27)>;
> +			};
> +
>  			i2c6_scl: i2c6-scl-pins {
>  				pinctrl-single,bits = <0x4 0x2 0x7>;
>  			};
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts b/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
> index eafef8718a0fe..8920326a06735 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-clearfog.dts
> @@ -223,6 +223,8 @@ ethernet_phy8: ethernet-phy@15 {
>  };
>  
>  &esdhc0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&esdhc0_cd_wp_pins>, <&esdhc0_cmd_data30_clk_vsel_pins>;
>  	sd-uhs-sdr104;
>  	sd-uhs-sdr50;
>  	sd-uhs-sdr25;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
> index e914291e63a1a..e1344942eaaee 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2162a-sr-som.dtsi
> @@ -30,6 +30,8 @@ &esdhc1 {
>  };
>  
>  &fspi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&fspi_data74_pins>, <&fspi_data30_pins>, <&fspi_dqs_sck_cs10_pins>;
>  	status = "okay";
>  
>  	flash@0 {
> @@ -80,3 +82,8 @@ rtc@6f {
>  		reg = <0x6f>;
>  	};
>  };
> +
> +&pinmux_i2crv {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gpio0_14_12_pins>;
> +};
>