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

Am 24.08.26 um 15:12 schrieb Vincent Jardin via B4 Relay:
> From: Vincent Jardin <[email protected]>
>
> 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 <[email protected]>
> ---
>  arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi | 4 ++--
>  1 file changed, 2 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..fae705dd07669 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx216x.dtsi
> @@ -1688,11 +1688,11 @@ i2c6_i2c7_pins: iic8-iic7-i2c-pins {
>  
>  			/* RCWSR14 */
>  			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)>;

Perhaps add a short comment here on reserved bits, e.g.:

> LX2160A RM describes bits 11 and 12 as reserved, but writing 0 to them was confirmed harmless.

Feel free to rephrase.

> +				pinctrl-single,bits = <0x8 (0x1 << 10) (0x7 << 10)>;
>  			};
>  		};
>