Re: [PATCH v3 2/5] arm64: dts: lx2160a: complete pinmux for rcwsr12 configuration word

[Josua Mayer]

Am 04.03.26 um 12:21 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.
>
> Firstly this can cause unintended side-effects because the dynamic
> configuration area where pinmux is modified at runtime reads zero
> initially, causing application of the first pinmux node to clear any
> other bits previously set during reset configuration.
>
> It is possible for bootloaders to implement a workaround and copy from
> the reset values from read-only area to dynamic configuration area,
> however bootloader updates should not be required for kernel updates.
>
> Alternatively pinmux in device-tree can be made complete, to cover all
> bits of a 32-bit register, not just i2c & gpio functions, e.g. for
> RCWSR12 register:
>
> LX2162-CF RCWSR12: 0b0000100000000000 0000000000000110
> IIC2_PMUX              |||   |||   || |   |||   |||XXX : I2C/GPIO/CD-WP
> IIC3_PMUX              |||   |||   || |   |||   XXX    : I2C/GPIO/CAN/EVT
> IIC4_PMUX              |||   |||   || |   |||XXX|||    : I2C/GPIO/CAN/EVT
> IIC5_PMUX              |||   |||   || |   XXX   |||    : I2C/GPIO/SDHC-CLK
> IIC6_PMUX              |||   |||   || |XXX|||   |||    : I2C/GPIO/SDHC-CLK
> XSPI1_A_DATA74_PMUX    |||   |||   XX X   |||   |||    : XSPI/GPIO
> XSPI1_A_DATA30_PMUX    |||   |||XXX|| |   |||   |||    : XSPI/GPIO
> XSPI1_A_BASE_PMUX      |||   XXX   || |   |||   |||    : XSPI/GPIO
> SDHC1_BASE_PMUX        |||XXX|||   || |   |||   |||    : SDHC/GPIO/SPI
> SDHC1_DIR_PMUX         XXX   |||   || |   |||   |||    : SDHC/GPIO/SPI
> RESERVED             XX|||   |||   || |   |||   |||    :
>
> Secondly the i2c bus recovery descriptions use explicit "_scl" suffix,
> suggesting that only scl pin is configured.
>
> However the pinmux in LX2160 always configures i2c pins in pairs, i.e.
> both sda and scl are both switched between i2c and gpio functions. Yet
> sda-gpios was not set.
>
> Thirdly i2c6 and i2c7 pinmux nodes are identical, setting the same bits
> because both pairs of pins are configured together in lx2160.
>
> Describing them in duplicate nodes is confusing and hides the fact that
> changing the mux for one, changes the other too.
>
> Add description for all bits of RCWSR12 register, including i2c, gpio,
> flextimer, spi, can and sdhc.
>
> Rename the existing i2c pinmux nodes to be consistent with the newly
> added alternate function nodes, and set sda-gpios on each i2c
> controller.
>
> Merge the i2c6 and i2c7 into a single node.
I realized that adding new nodes and renaming together makes the diff
quite unreadable.

So I will split this up into two changes for next version.

>
> Other configuration words, i.e. RCWSR13 & RCWSR14 may be added in the
> future for boards setting non-zero values there.
>
> Fixes: 8a1365c7bbc1 ("arm64: dts: lx2160a: add pinmux and i2c gpio to support bus recovery")
> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 153 ++++++++++++++++++-------
>  1 file changed, 114 insertions(+), 39 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> index be0ccab5a626b..920198694e2fb 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -750,9 +750,10 @@ i2c0: i2c@2000000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c0_scl>;
> -			pinctrl-1 = <&i2c0_scl_gpio>;
> +			pinctrl-0 = <&i2c0_pins>;
> +			pinctrl-1 = <&gpio0_3_2_pins>;
>  			scl-gpios = <&gpio0 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -766,9 +767,10 @@ i2c1: i2c@2010000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c1_scl>;
> -			pinctrl-1 = <&i2c1_scl_gpio>;
> +			pinctrl-0 = <&i2c1_pins>;
> +			pinctrl-1 = <&gpio0_31_30_pins>;
>  			scl-gpios = <&gpio0 31 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 30 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -782,9 +784,10 @@ i2c2: i2c@2020000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c2_scl>;
> -			pinctrl-1 = <&i2c2_scl_gpio>;
> +			pinctrl-0 = <&i2c2_pins>;
> +			pinctrl-1 = <&gpio0_29_28_pins>;
>  			scl-gpios = <&gpio0 29 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 28 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -798,9 +801,10 @@ i2c3: i2c@2030000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c3_scl>;
> -			pinctrl-1 = <&i2c3_scl_gpio>;
> +			pinctrl-0 = <&i2c3_pins>;
> +			pinctrl-1 = <&gpio0_27_26_pins>;
>  			scl-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 26 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -814,9 +818,10 @@ i2c4: i2c@2040000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c4_scl>;
> -			pinctrl-1 = <&i2c4_scl_gpio>;
> +			pinctrl-0 = <&i2c4_pins>;
> +			pinctrl-1 = <&gpio0_25_24_pins>;
>  			scl-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 24 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -830,9 +835,10 @@ i2c5: i2c@2050000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c5_scl>;
> -			pinctrl-1 = <&i2c5_scl_gpio>;
> +			pinctrl-0 = <&i2c5_pins>;
> +			pinctrl-1 = <&gpio0_23_22_pins>;
>  			scl-gpios = <&gpio0 23 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 22 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -846,9 +852,10 @@ i2c6: i2c@2060000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c6_scl>;
> -			pinctrl-1 = <&i2c6_scl_gpio>;
> +			pinctrl-0 = <&i2c6_i2c7_pins>;
> +			pinctrl-1 = <&gpio1_18_15_pins>;
>  			scl-gpios = <&gpio1 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio1 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -862,9 +869,10 @@ i2c7: i2c@2070000 {
>  			clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>  					    QORIQ_CLK_PLL_DIV(16)>;
>  			pinctrl-names = "default", "gpio";
> -			pinctrl-0 = <&i2c7_scl>;
> -			pinctrl-1 = <&i2c7_scl_gpio>;
> +			pinctrl-0 = <&i2c6_i2c7_pins>;
> +			pinctrl-1 = <&gpio1_18_15_pins>;
>  			scl-gpios = <&gpio1 18 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +			sda-gpios = <&gpio0 17 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>  			status = "disabled";
>  		};
>  
> @@ -1713,92 +1721,159 @@ pinmux_i2crv: pinmux@70010012c {
>  			pinctrl-single,register-width = <32>;
>  			pinctrl-single,function-mask = <0x7>;
>  
> -			i2c1_scl: i2c1-scl-pins {
> -				pinctrl-single,bits = <0x0 0 0x7>;
> +			/* RCWSR12 */
> +			i2c1_pins: iic2-i2c-pins {
> +				pinctrl-single,bits = <0x0 0x0 0x7>;
>  			};
>  
> -			i2c1_scl_gpio: i2c1-scl-gpio-pins {
> +			gpio0_31_30_pins: iic2-gpio-pins {
>  				pinctrl-single,bits = <0x0 0x1 0x7>;
>  			};
>  
> +			ftm0_ch10_pins: iic2-ftm-pins {
> +				pinctrl-single,bits = <0x0 0x2 0x7>;
> +			};
> +
>  			esdhc0_cd_wp_pins: iic2-sdhc-pins {
>  				pinctrl-single,bits = <0x0 0x6 0x7>;
>  			};
>  
> -			i2c2_scl: i2c2-scl-pins {
> +			i2c2_pins: iic3-i2c-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 3)>;
>  			};
>  
> -			i2c2_scl_gpio: i2c2-scl-gpio-pins {
> +			gpio0_29_28_pins: iic3-gpio-pins {
>  				pinctrl-single,bits = <0x0 (0x1 << 3) (0x7 << 3)>;
>  			};
>  
> -			i2c3_scl: i2c3-scl-pins {
> +			can0_pins: iic3-can-pins {
> +				pinctrl-single,bits = <0x0 (0x2 << 3) (0x7 << 3)>;
> +			};
> +
> +			event65_pins: iic3-event-pins {
> +				pinctrl-single,bits = <0x0 (0x6 << 3) (0x7 << 3)>;
> +			};
> +
> +			i2c3_pins: iic4-i2c-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 6)>;
>  			};
>  
> -			i2c3_scl_gpio: i2c3-scl-gpio-pins {
> +			gpio0_27_26_pins: iic4-gpio-pins {
>  				pinctrl-single,bits = <0x0 (0x1 << 6) (0x7 << 6)>;
>  			};
>  
> -			i2c4_scl: i2c4-scl-pins {
> +			can1_pins: iic4-can-pins {
> +				pinctrl-single,bits = <0x0 (0x2 << 6) (0x7 << 6)>;
> +			};
> +
> +			event87_pins: iic4-event-pins {
> +				pinctrl-single,bits = <0x0 (0x6 << 6) (0x7 << 6)>;
> +			};
> +
> +			i2c4_pins: iic5-i2c-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 9)>;
>  			};
>  
> -			i2c4_scl_gpio: i2c4-scl-gpio-pins {
> +			gpio0_25_24_pins: iic5-gpio-pins {
>  				pinctrl-single,bits = <0x0 (0x1 << 9) (0x7 << 9)>;
>  			};
>  
> -			i2c5_scl: i2c5-scl-pins {
> +			esdhc0_clksync_pins: iic5-sdhc-clk-pins {
> +				pinctrl-single,bits = <0x0 (0x2 << 9) (0x7 << 9)>;
> +			};
> +
> +			dspi2_miso_mosi_pins: iic5-spi3-pins {
> +				pinctrl-single,bits = <0x3 (0x2 << 9) (0x7 << 9)>;
> +			};
> +
> +			i2c5_pins: iic6-i2c-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 12)>;
>  			};
>  
> -			i2c5_scl_gpio: i2c5-scl-gpio-pins {
> +			gpio0_23_22_pins: iic6-gpio-pins {
>  				pinctrl-single,bits = <0x0 (0x1 << 12) (0x7 << 12)>;
>  			};
>  
> +			esdhc1_clksync_pins: iic6-sdhc-clk-pins {
> +				pinctrl-single,bits = <0x0 (0x2 << 12) (0x7 << 12)>;
> +			};
> +
>  			fspi_data74_pins: xspi1-data74-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 15)>;
>  			};
>  
> +			gpio1_31_28_pins: xspi1-data74-gpio-pins {
> +				pinctrl-single,bits = <0x0 0x1 (0x7 << 15)>;
> +			};
> +
>  			fspi_data30_pins: xspi1-data30-pins {
>  				pinctrl-single,bits = <0x0 0 (0x7 << 18)>;
>  			};
>  
> +			gpio1_27_24_pins: xspi1-data30-gpio-pins {
> +				pinctrl-single,bits = <0x0 0x1 (0x7 << 18)>;
> +			};
> +
>  			fspi_dqs_sck_cs10_pins: xspi1-base-pins {
>  				pinctrl-single,bits = <0x0 0x0 (0x7 << 21)>;
>  			};
>  
> +			gpio1_23_20_pins: xspi1-base-gpio-pins {
> +				pinctrl-single,bits = <0x0 0x1 (0x7 << 21)>;
> +			};
> +
>  			esdhc0_cmd_data30_clk_vsel_pins: sdhc1-base-sdhc-vsel-pins {
>  				pinctrl-single,bits = <0x0 0x0 (0x7 << 24)>;
>  			};
>  
> +			gpio0_21_15_pins: sdhc1-base-gpio-pins {
> +				pinctrl-single,bits = <0x0 (0x1 << 24) (0x7 << 24)>;
> +			};
> +
> +			dspi0_pins: sdhc1-base-spi1-pins {
> +				pinctrl-single,bits = <0x0 (0x2 << 24) (0x7 << 24)>;
> +			};
> +
> +			esdhc0_cmd_data30_clk_dspi2_cs0_pins: sdhc1-base-sdhc-spi3-pins {
> +				pinctrl-single,bits = <0x0 (0x3 << 24) (0x7 << 24)>;
> +			};
> +
> +			esdhc0_cmd_data30_clk_data4_pins: sdhc1-base-sdhc-data4-pins {
> +				pinctrl-single,bits = <0x0 (0x4 << 24) (0x7 << 24)>;
> +			};
> +
> +			esdhc0_dir_pins: sdhc1-dir-pins {
> +				pinctrl-single,bits = <0x0 0x0 (0x7 << 27)>;
> +			};
> +
>  			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>;
> +			dspi2_cs31_pins: sdhc1-dir-spi3-pins {
> +				pinctrl-single,bits = <0x0 (0x3 << 27) (0x7 << 27)>;
>  			};
>  
> -			i2c6_scl_gpio: i2c6-scl-gpio-pins {
> -				pinctrl-single,bits = <0x4 0x1 0x7>;
> +			esdhc0_data75_pins: sdhc1-dir-sdhc-pins {
> +				pinctrl-single,bits = <0x0 (0x4 << 27) (0x7 << 27)>;
>  			};
>  
> -			i2c7_scl: i2c7-scl-pins {
> -				pinctrl-single,bits = <0x4 0x2 0x7>;
> +			/* RCWSR13 */
> +			gpio1_18_15_pins: iic8-iic7-gpio-pins {
> +				pinctrl-single,bits = <0x4 0x1 0x7>;
>  			};
>  
> -			i2c7_scl_gpio: i2c7-scl-gpio-pins {
> -				pinctrl-single,bits = <0x4 0x1 0x7>;
> +			i2c6_i2c7_pins: iic8-iic7-i2c-pins {
> +				pinctrl-single,bits = <0x4 0x2 0x7>;
>  			};
>  
> -			i2c0_scl: i2c0-scl-pins {
> -				pinctrl-single,bits = <0x8 0 (0x7 << 10)>;
> +			/* RCWSR14 */
> +			i2c0_pins: iic1-i2c-pins {
> +				pinctrl-single,bits = <0x8 0x0 (0x1 << 10)>;
>  			};
>  
> -			i2c0_scl_gpio: i2c0-scl-gpio-pins {
> -				pinctrl-single,bits = <0x8 (0x1 << 10) (0x7 << 10)>;
> +			gpio0_3_2_pins: iic1-gpio-pins {
> +				pinctrl-single,bits = <0x8 (0x1 << 10) (0x1 << 10)>;
>  			};
>  		};
>  
>