| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <14bda26b-932e-4a95-89a1-d308d27fb55f@solid-run.com>
Date: Sat, 19 Jul 2025 09:14:34 +0000
From: Josua Mayer <josua@...id-run.com>
To: Frank Li <Frank.li@....com>
CC: Shawn Guo <shawnguo@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Carlos
Song <carlos.song@....com>, Jon Nettleton <jon@...id-run.com>, Rabeeh Khoury
<rabeeh@...id-run.com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux@...tq-group.com"
<linux@...tq-group.com>, "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2] Revert "arm64: dts: lx2160a: add pinmux and i2c gpio
to support bus recovery"
Hi Frank,
Am 14.07.25 um 16:34 schrieb Frank Li:
> On Mon, Jul 14, 2025 at 10:44:13AM +0300, Josua Mayer wrote:
>> This reverts commit 8a1365c7bbc122bd843096f0008d259e7a8afc61.
>>
>> The commit in questions most notably breaks SD-Card on SolidRun
>> LX2162A Clearfog by corrupting the pinmux in dynamic configuration area
>> for non-i2c pins without pinmux node.
>> It is further expected that it breaks SD Card-Detect, as well as CAN,
>> DSPI and GPIOs on any board based on LX2160 SoC.
> Thank you for your patch. I remember we met similar issue before. Let's
> wait for carlos is back about in next week.
>
> I remember uboot should copy RCWSR to 0x70010012c.
Knowing when and where this was implemented would be helpful,
however need to keep in mind existing systems running stable bootloader.
> I2C recover also is
> important feature.
Agreed.
There is an additional problem with the patch in question:
The pinmux affects both SCL+SDA together, changing between I2C and GPIO,
but in the i2c controller nodes only scl-gpios was specified.
Hence for sda the recovery relies on undefined state of gpio controller registers.
>> Background:
>>
>> The LX2160 SoC is configured at power-on from RCW (Reset
>> Configuration Word) typically located in the first 4k of boot media.
>> This blob configures various clock rates and pin functions.
>> The pinmux for i2c specifically is part of configuration words RCWSR12,
>> RCWSR13 and RCWSR14 size 32 bit each.
>> These values are accessible at read-only addresses 0x01e0012c following.
>>
>> For runtime (re-)configuration the SoC has a dynamic configuration area
>> where alternative settings can be applied. The counterparts of
>> RCWSR[12-14] can be overridden at 0x70010012c following.
>>
>> The commit in question used this area to switch i2c pins between i2c and
>> gpio function at runtime using the pinctrl-single driver - which reads a
>> 32-bit value, makes particular changes by bitmask and writes back the
>> new value.
>>
>> SolidRun have observed that if the dynamic configuration is read first
>> (before a write), it reads as zero regardless the initial values set by
>> RCW. After the first write consecutive reads reflect the written value.
>>
>> Because multiple pins are configured from a single 32-bit value, this
>> causes unintentional change of all bits (except those for i2c) being set
>> to zero when the pinctrl driver applies the first configuration.
>>
>> See below a short list of which functions RCWSR12 alone controls:
>>
>> 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||| ||| || | ||| ||| :
>>
>> On LX2162A Clearfog the initial (ant intended) value is 0x08000006 -
>> enabling card-detect on IIC2_PMUX and some LEDs on SDHC1_DIR_PMUX.
>> Everything else is intentional zero (enabling I2C & XSPI).
>>
>> By reading zero from dynamic configuration area, the commit in question
>> changes IIC2_PMUX to value 0 (I2C function), and SDHC1_DIR_PMUX to 0
>> (SDHC data direction function) - breaking card-detect and led gpios.
>>
>> This issue should affect any board based on LX2160 SoC that is using the
>> same or earlier versions of NXP bootloader as SolidRun have tested, in
>> particular: LSDK-21.08 and LS-5.15.71-2.2.0.
>>
>> Whether NXP added some extra initialisation in the bootloader on later
>> releases was not investigated. However bootloader upgrade should not be
>> necessary to run a newer Linux kernel.
>>
>> To work around this issue it is possible to explicitly define ALL pins
>> controlled by any 32-bit value so that gradually after processing all
>> pinctrl nodes the correct value is reached on all bits.
>>
>> This is a large task that should be done carefully on a per-board basis
>> and not globally through the SoC dtsi.
>> Therefore the commit in question is reverted.
>>
>> Fixes: 8a1365c7bbc1 ("arm64: dts: lx2160a: add pinmux and i2c gpio to support bus recovery")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Josua Mayer <josua@...id-run.com>
>> ---
>> Changes in v2:
>> - changed to revert problematic commit, workaround is large effort
>> - Link to v1: https://lore.kernel.org/r/f32c5525-3162-4acd-880c-99fc46d3a63d@solid-run.com
>> ---
>> arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 106 -------------------------
>> 1 file changed, 106 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
>> index c9541403bcd8..eb1b4e607e2b 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
>> @@ -749,10 +749,6 @@ i2c0: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c0_scl>;
>> - pinctrl-1 = <&i2c0_scl_gpio>;
>> - scl-gpios = <&gpio0 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -765,10 +761,6 @@ i2c1: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c1_scl>;
>> - pinctrl-1 = <&i2c1_scl_gpio>;
>> - scl-gpios = <&gpio0 31 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -781,10 +773,6 @@ i2c2: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c2_scl>;
>> - pinctrl-1 = <&i2c2_scl_gpio>;
>> - scl-gpios = <&gpio0 29 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -797,10 +785,6 @@ i2c3: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c3_scl>;
>> - pinctrl-1 = <&i2c3_scl_gpio>;
>> - scl-gpios = <&gpio0 27 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -813,10 +797,6 @@ i2c4: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c4_scl>;
>> - pinctrl-1 = <&i2c4_scl_gpio>;
>> - scl-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -829,10 +809,6 @@ i2c5: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c5_scl>;
>> - pinctrl-1 = <&i2c5_scl_gpio>;
>> - scl-gpios = <&gpio0 23 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -845,10 +821,6 @@ i2c6: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c6_scl>;
>> - pinctrl-1 = <&i2c6_scl_gpio>;
>> - scl-gpios = <&gpio1 16 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -861,10 +833,6 @@ i2c7: i2c@...0000 {
>> clock-names = "ipg";
>> clocks = <&clockgen QORIQ_CLK_PLATFORM_PLL
>> QORIQ_CLK_PLL_DIV(16)>;
>> - pinctrl-names = "default", "gpio";
>> - pinctrl-0 = <&i2c7_scl>;
>> - pinctrl-1 = <&i2c7_scl_gpio>;
>> - scl-gpios = <&gpio1 18 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> status = "disabled";
>> };
>>
>> @@ -1700,80 +1668,6 @@ pcs18: ethernet-phy@0 {
>> };
>> };
>>
>> - pinmux_i2crv: pinmux@...10012c {
>> - compatible = "pinctrl-single";
>> - reg = <0x00000007 0x0010012c 0x0 0xc>;
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - pinctrl-single,bit-per-mux;
>> - pinctrl-single,register-width = <32>;
>> - pinctrl-single,function-mask = <0x7>;
>> -
>> - i2c1_scl: i2c1-scl-pins {
>> - pinctrl-single,bits = <0x0 0 0x7>;
>> - };
>> -
>> - i2c1_scl_gpio: i2c1-scl-gpio-pins {
>> - pinctrl-single,bits = <0x0 0x1 0x7>;
>> - };
>> -
>> - i2c2_scl: i2c2-scl-pins {
>> - pinctrl-single,bits = <0x0 0 (0x7 << 3)>;
>> - };
>> -
>> - i2c2_scl_gpio: i2c2-scl-gpio-pins {
>> - pinctrl-single,bits = <0x0 (0x1 << 3) (0x7 << 3)>;
>> - };
>> -
>> - i2c3_scl: i2c3-scl-pins {
>> - pinctrl-single,bits = <0x0 0 (0x7 << 6)>;
>> - };
>> -
>> - i2c3_scl_gpio: i2c3-scl-gpio-pins {
>> - pinctrl-single,bits = <0x0 (0x1 << 6) (0x7 << 6)>;
>> - };
>> -
>> - i2c4_scl: i2c4-scl-pins {
>> - pinctrl-single,bits = <0x0 0 (0x7 << 9)>;
>> - };
>> -
>> - i2c4_scl_gpio: i2c4-scl-gpio-pins {
>> - pinctrl-single,bits = <0x0 (0x1 << 9) (0x7 << 9)>;
>> - };
>> -
>> - i2c5_scl: i2c5-scl-pins {
>> - pinctrl-single,bits = <0x0 0 (0x7 << 12)>;
>> - };
>> -
>> - i2c5_scl_gpio: i2c5-scl-gpio-pins {
>> - pinctrl-single,bits = <0x0 (0x1 << 12) (0x7 << 12)>;
>> - };
>> -
>> - i2c6_scl: i2c6-scl-pins {
>> - pinctrl-single,bits = <0x4 0x2 0x7>;
>> - };
>> -
>> - i2c6_scl_gpio: i2c6-scl-gpio-pins {
>> - pinctrl-single,bits = <0x4 0x1 0x7>;
>> - };
>> -
>> - i2c7_scl: i2c7-scl-pins {
>> - pinctrl-single,bits = <0x4 0x2 0x7>;
>> - };
>> -
>> - i2c7_scl_gpio: i2c7-scl-gpio-pins {
>> - pinctrl-single,bits = <0x4 0x1 0x7>;
>> - };
>> -
>> - i2c0_scl: i2c0-scl-pins {
>> - pinctrl-single,bits = <0x8 0 (0x7 << 10)>;
>> - };
>> -
>> - i2c0_scl_gpio: i2c0-scl-gpio-pins {
>> - pinctrl-single,bits = <0x8 (0x1 << 10) (0x7 << 10)>;
>> - };
>> - };
>> -
>> fsl_mc: fsl-mc@...000000 {
>> compatible = "fsl,qoriq-mc";
>> reg = <0x00000008 0x0c000000 0 0x40>,
>>
>> ---
>> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
>> change-id: 20250710-lx2160-sd-cd-00bf38ae169e
>>
>> Best regards,
>> --
>> Josua Mayer <josua@...id-run.com>
>>
Powered by blists - more mailing lists