[PATCH v3 0/3] arm64: dts: lx2160a: fix the malformed pinmux entries

[PATCH v3 0/3] arm64: dts: lx2160a: fix the malformed pinmux entries

[Vincent Jardin via B4 Relay]

Every pinctrl-single,bits entry in the lx2160a pinmux@70010012c node was
reviewed against what pinctrl-single actually accepts in bit-per-mux mode:
 the offset has to name one of the three registers the node maps,
 the mask has to equal pinctrl-single,function-mask shifted to the field,
 the value has to fit inside that mask.

Three entries fail one of those rules, and each fails differently:

  1/3 the three gpio1 groups put the field value at bit 0 instead of at
      the field, so pcs_parse_bits_in_pinctrl_entry() masks it away and
      the mux write does nothing;

  2/3 the two iic1 groups carry a one bit mask, which the driver rejects
      outright with "Invalid submask", dropping both pinctrl states of
      i2c0 along with its bus recovery;

  3/3 iic5-spi3-pins has the field value in the offset cell, leaving an
      invalid offset and a value that selects the neighbouring sdhc1
      function rather than SPI3.

After the series no entry in that node is malformed anymore.

Only 1/3 was sent with the v1, as a single patch. The other two turned up
while checking whether it was the only one of its kind in that node.

They are 3 patches rather than 1 because each fails differently and
each requires the reviewer to check something different:
 1/3 is mask arithmetic,
 3/3 needs the IIC5_PMUX encoding table to confirm which
     function value is right,
 2/3 is a partial revert of a change that was made deliberately.

1/3 and 3/3 share a Fixes: tag and are still not folded together:
  - one is a value that is never shifted into its field,
  - the other is a value that ended up in the offset cell and also names
    the wrong function
and both they touch unrelated pads.

I can squash any of them if you would rather have them merged.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
Changes in v3:
- 2/3: RCWSR14[11:12] are reserved, writing 0 to them is harmless (Josua Mayer)
- Add Reviewed-by from Josua Mayer on 1/3 and 3/3
- Link to v2: https://lore.kernel.org/r/20260824-for-upstream-lx2160a-pinmux-fix-v2-0-0a525d5f4cd5@free.fr

Changes in v2:
- Rebased onto current mainline. fsl-lx2160a.dtsi was renamed to
  fsl-lx216x.dtsi by commit 285560f0d39d ("arm64: dts: lx2160a: split the
  dtsi to avoid delete-property"), which landed in the v7.3 merge window,
  so v1 no longer applied.
- Added 2/3 and 3/3: two more malformed entries in the same pinmux node,
  found by auditing the rest of it.
- Link to v1: https://lore.kernel.org/r/20260824-for-upstream-lx2160a-pinmux-fix-v1-1-33a3c8d5ea8a@free.fr

---
Vincent Jardin (3):
      arm64: dts: lx2160a: fix incorrect pinmux
      arm64: dts: lx2160a: fix IIC1 pinmux submask rejected by pinctrl-single
      arm64: dts: lx2160a: fix the iic5 spi3 pinmux offset and value

 arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
---
base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
change-id: 20260824-for-upstream-lx2160a-pinmux-fix-6d09e82bd5cb

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>

[PATCH v3 1/3] arm64: dts: lx2160a: fix incorrect pinmux

[Vincent Jardin via B4 Relay]

From: Vincent Jardin <vjardin@free.fr>

For gpio1 value set for pinctrl is outside the mask and so will do
nothing.

Add the missing offset for value field to be the same as the mask as
expected.

Fixes: 284ad7064aaa ("arm64: dts: lx2160a: complete pinmux for rcwsr12 configuration word")
Suggested-by: Thibaut Collet <thbt.cllt@gmail.com>
Reviewed-by: Josua Mayer <josua@solid-run.com>
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
index ae6e37870c5e5..d4522a19405a9 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
@@ -1622,7 +1622,7 @@ fspi_data74_pins: xspi1-data74-pins {
 			};
 
 			gpio1_31_28_pins: xspi1-data74-gpio-pins {
-				pinctrl-single,bits = <0x0 0x1 (0x7 << 15)>;
+				pinctrl-single,bits = <0x0 (0x1 << 15) (0x7 << 15)>;
 			};
 
 			fspi_data30_pins: xspi1-data30-pins {
@@ -1630,7 +1630,7 @@ fspi_data30_pins: xspi1-data30-pins {
 			};
 
 			gpio1_27_24_pins: xspi1-data30-gpio-pins {
-				pinctrl-single,bits = <0x0 0x1 (0x7 << 18)>;
+				pinctrl-single,bits = <0x0 (0x1 << 18) (0x7 << 18)>;
 			};
 
 			fspi_dqs_sck_cs10_pins: xspi1-base-pins {
@@ -1638,7 +1638,7 @@ fspi_dqs_sck_cs10_pins: xspi1-base-pins {
 			};
 
 			gpio1_23_20_pins: xspi1-base-gpio-pins {
-				pinctrl-single,bits = <0x0 0x1 (0x7 << 21)>;
+				pinctrl-single,bits = <0x0 (0x1 << 21) (0x7 << 21)>;
 			};
 
 			esdhc0_cmd_data30_clk_vsel_pins: sdhc1-base-sdhc-vsel-pins {

-- 
2.43.0

[PATCH v3 2/3] arm64: dts: lx2160a: fix IIC1 pinmux submask rejected by pinctrl-single

[Vincent Jardin via B4 Relay]

From: Vincent Jardin <vjardin@free.fr>

IIC1_PMUX is a single bit field at RCWSR14 bit 10, so the mask for the
two iic1 pinmux nodes was narrowed from 0x7 to 0x1 to avoid writing the
two reserved bits next to it.

With pinctrl-single,bit-per-mux the driver does not take the supplied
mask as the bits to modify. It derives the field from the node's
pinctrl-single,function-mask, which is 0x7 here, and then requires the
supplied mask to match it exactly:

	bit_pos = __ffs(mask);			/* 10 */
	mask_pos = ((pcs->fmask) << bit_pos);	/* 0x7 << 10 = 0x1c00 */
	submask = mask & mask_pos;		/* 0x400 */
	...
	if (submask != mask_pos) {
		dev_warn(pcs->dev,
			 "Invalid submask 0x%x for %pOFn at 0x%x\n",
			 submask, np, offset);
		continue;
	}

A one bit mask gives submask 0x400 against mask_pos 0x1c00, so both
nodes are rejected and the pins are never muxed with this message:

  pinctrl-single 70010012c.pinmux: Invalid submask 0x400 for iic1-i2c-pins
  pinctrl-single 70010012c.pinmux: Invalid submask 0x400 for iic1-gpio-pins

The values are not changed:
  - 0x0 selects IIC1
  - 0x1 << 10 selects GPIO_1[3:2].

Fixes: 7a3cc49ad1fc ("arm64: dts: lx2160a: change i2c0 (iic1) pinmux mask to one bit")
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
index d4522a19405a9..d9d4500a40510 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
@@ -1687,12 +1687,19 @@ i2c6_i2c7_pins: iic8-iic7-i2c-pins {
 			};
 
 			/* RCWSR14 */
+			/*
+			 * IIC1_PMUX is a single bit at RCWSR14[10], but bit-per-mux
+			 * uses the mask for the field to clear before applying the
+			 * value.
+			 * So bits 11 and 12, which are reserved, are written as 0
+			 * on every mux switch, it is harmless.
+			 */
 			i2c0_pins: iic1-i2c-pins {
-				pinctrl-single,bits = <0x8 0x0 (0x1 << 10)>;
+				pinctrl-single,bits = <0x8 0x0 (0x7 << 10)>;
 			};
 
 			gpio0_3_2_pins: iic1-gpio-pins {
-				pinctrl-single,bits = <0x8 (0x1 << 10) (0x1 << 10)>;
+				pinctrl-single,bits = <0x8 (0x1 << 10) (0x7 << 10)>;
 			};
 		};
 

-- 
2.43.0

Re: [PATCH v3 2/3] arm64: dts: lx2160a: fix IIC1 pinmux submask rejected by pinctrl-single

[Josua Mayer]

Am 24.08.26 um 23:25 schrieb Vincent Jardin via B4 Relay:
> From: Vincent Jardin <vjardin@free.fr>
>
> IIC1_PMUX is a single bit field at RCWSR14 bit 10, so the mask for the
> two iic1 pinmux nodes was narrowed from 0x7 to 0x1 to avoid writing the
> two reserved bits next to it.
>
> With pinctrl-single,bit-per-mux the driver does not take the supplied
> mask as the bits to modify. It derives the field from the node's
> pinctrl-single,function-mask, which is 0x7 here, and then requires the
> supplied mask to match it exactly:
>
> 	bit_pos = __ffs(mask);			/* 10 */
> 	mask_pos = ((pcs->fmask) << bit_pos);	/* 0x7 << 10 = 0x1c00 */
> 	submask = mask & mask_pos;		/* 0x400 */
> 	...
> 	if (submask != mask_pos) {
> 		dev_warn(pcs->dev,
> 			 "Invalid submask 0x%x for %pOFn at 0x%x\n",
> 			 submask, np, offset);
> 		continue;
> 	}
>
> A one bit mask gives submask 0x400 against mask_pos 0x1c00, so both
> nodes are rejected and the pins are never muxed with this message:
>
>   pinctrl-single 70010012c.pinmux: Invalid submask 0x400 for iic1-i2c-pins
>   pinctrl-single 70010012c.pinmux: Invalid submask 0x400 for iic1-gpio-pins
>
> The values are not changed:
>   - 0x0 selects IIC1
>   - 0x1 << 10 selects GPIO_1[3:2].
>
> Fixes: 7a3cc49ad1fc ("arm64: dts: lx2160a: change i2c0 (iic1) pinmux mask to one bit")
> Signed-off-by: Vincent Jardin <vjardin@free.fr>
> ---
>  arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
> index d4522a19405a9..d9d4500a40510 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
> @@ -1687,12 +1687,19 @@ i2c6_i2c7_pins: iic8-iic7-i2c-pins {
>  			};
>  
>  			/* RCWSR14 */
> +			/*
> +			 * IIC1_PMUX is a single bit at RCWSR14[10], but bit-per-mux
> +			 * uses the mask for the field to clear before applying the
> +			 * value.
> +			 * So bits 11 and 12, which are reserved, are written as 0
> +			 * on every mux switch, it is harmless.
> +			 */
>  			i2c0_pins: iic1-i2c-pins {
> -				pinctrl-single,bits = <0x8 0x0 (0x1 << 10)>;
> +				pinctrl-single,bits = <0x8 0x0 (0x7 << 10)>;
>  			};
>  
>  			gpio0_3_2_pins: iic1-gpio-pins {
> -				pinctrl-single,bits = <0x8 (0x1 << 10) (0x1 << 10)>;
> +				pinctrl-single,bits = <0x8 (0x1 << 10) (0x7 << 10)>;
>  			};
>  		};
>  
>
Reviewed-by: Josua Mayer <josua@solid-run.com>

[PATCH v3 3/3] arm64: dts: lx2160a: fix the iic5 spi3 pinmux offset and value

[Vincent Jardin via B4 Relay]

From: Vincent Jardin <vjardin@free.fr>

The iic5-spi3-pins node has both of its first two cells wrong.

The offset selects the register within the node's reg range, where:
  0x0 is RCWSR12,
  0x4 is RCWSR13,
  0x8 is RCWSR14.

An offset of 0x3 is not one of them and is not even aligned for the
declared 32 bit register width. It is the only node in this pinmux that
does not use 0x0, 0x4 neither 0x8.

The value is wrong too:
  IIC5_PMUX is RCWSR12[11:9] and it encodes 0b000 for IIC5,
  0b001 for GPIO_1[25:24],
  0b010 for SDHC1_CLK_SYNC_OUT/IN,
  0b011 for {IIC5_SCL,IIC5_SDA} = {SPI3_SOUT,SPI3_SIN}.

This node selects the last of those, so it needs 0x3, not the 0x2, which
is the value of the neighbouring sdhc1 clock sync node it was copied
from.

Fixes: 284ad7064aaa ("arm64: dts: lx2160a: complete pinmux for rcwsr12 configuration word")
Reviewed-by: Josua Mayer <josua@solid-run.com>
Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
index d9d4500a40510..bccf2d6730230 100644
--- a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
@@ -1602,7 +1602,7 @@ esdhc0_clksync_pins: iic5-sdhc-clk-pins {
 			};
 
 			dspi2_miso_mosi_pins: iic5-spi3-pins {
-				pinctrl-single,bits = <0x3 (0x2 << 9) (0x7 << 9)>;
+				pinctrl-single,bits = <0x0 (0x3 << 9) (0x7 << 9)>;
 			};
 
 			i2c5_pins: iic6-i2c-pins {

-- 
2.43.0