[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+HBbNF6r0eQLS01eTUX0DAZ3mGcQX7N3HTCnTanVPC0num2WQ@mail.gmail.com>
Date: Mon, 26 Jan 2026 10:12:35 +0100
From: Robert Marko <robert.marko@...tura.hr>
To: Gabor Juhos <j4g8y7@...il.com>
Cc: Wolfram Sang <wsa@...nel.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>,
Andi Shyti <andi.shyti@...nel.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Russell King <rmk+kernel@...linux.org.uk>, Andrew Lunn <andrew@...n.ch>,
Hanna Hawa <hhhawa@...zon.com>, Linus Walleij <linus.walleij@...aro.org>, linux-i2c@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v3 1/2] i2c: pxa: defer reset on Armada 3700 when recovery
is used
On Wed, Aug 27, 2025 at 7:14 PM Gabor Juhos <j4g8y7@...il.com> wrote:
>
> The I2C communication is completely broken on the Armada 3700 platform
> since commit 0b01392c18b9 ("i2c: pxa: move to generic GPIO recovery").
>
> For example, on the Methode uDPU board, probing of the two onboard
> temperature sensors fails ...
>
> [ 7.271713] i2c i2c-0: using pinctrl states for GPIO recovery
> [ 7.277503] i2c i2c-0: PXA I2C adapter
> [ 7.282199] i2c i2c-1: using pinctrl states for GPIO recovery
> [ 7.288241] i2c i2c-1: PXA I2C adapter
> [ 7.292947] sfp sfp-eth1: Host maximum power 3.0W
> [ 7.299614] sfp sfp-eth0: Host maximum power 3.0W
> [ 7.308178] lm75 1-0048: supply vs not found, using dummy regulator
> [ 32.489631] lm75 1-0048: probe with driver lm75 failed with error -121
> [ 32.496833] lm75 1-0049: supply vs not found, using dummy regulator
> [ 82.890614] lm75 1-0049: probe with driver lm75 failed with error -121
>
> ... and accessing the plugged-in SFP modules also does not work:
>
> [ 511.298537] sfp sfp-eth1: please wait, module slow to respond
> [ 536.488530] sfp sfp-eth0: please wait, module slow to respond
> ...
> [ 1065.688536] sfp sfp-eth1: failed to read EEPROM: -EREMOTEIO
> [ 1090.888532] sfp sfp-eth0: failed to read EEPROM: -EREMOTEIO
>
> After a discussion [1], there was an attempt to fix the problem by
> reverting the offending change by commit 7b211c767121 ("Revert "i2c:
> pxa: move to generic GPIO recovery""), but that only helped to fix
> the issue in the 6.1.y stable tree. The reason behind the partial succes
> is that there was another change in commit 20cb3fce4d60 ("i2c: Set i2c
> pinctrl recovery info from it's device pinctrl") in the 6.3-rc1 cycle
> which broke things further.
>
> The cause of the problem is the same in case of both offending commits
> mentioned above. Namely, the I2C core code changes the pinctrl state to
> GPIO while running the recovery initialization code. Although the PXA
> specific initialization also does this, but the key difference is that
> it happens before the controller is getting enabled in i2c_pxa_reset(),
> whereas in the case of the generic initialization it happens after that.
>
> Change the code to reset the controller only before the first transfer
> instead of before registering the controller. This ensures that the
> controller is not enabled at the time when the generic recovery code
> performs the pinctrl state changes, thus avoids the problem described
> above.
>
> As the result this change restores the original behaviour, which in
> turn makes the I2C communication to work again as it can be seen from
> the following log:
>
> [ 7.363250] i2c i2c-0: using pinctrl states for GPIO recovery
> [ 7.369041] i2c i2c-0: PXA I2C adapter
> [ 7.373673] i2c i2c-1: using pinctrl states for GPIO recovery
> [ 7.379742] i2c i2c-1: PXA I2C adapter
> [ 7.384506] sfp sfp-eth1: Host maximum power 3.0W
> [ 7.393013] sfp sfp-eth0: Host maximum power 3.0W
> [ 7.399266] lm75 1-0048: supply vs not found, using dummy regulator
> [ 7.407257] hwmon hwmon0: temp1_input not attached to any thermal zone
> [ 7.413863] lm75 1-0048: hwmon0: sensor 'tmp75c'
> [ 7.418746] lm75 1-0049: supply vs not found, using dummy regulator
> [ 7.426371] hwmon hwmon1: temp1_input not attached to any thermal zone
> [ 7.432972] lm75 1-0049: hwmon1: sensor 'tmp75c'
> [ 7.755092] sfp sfp-eth1: module MENTECHOPTO POS22-LDCC-KR rev 1.0 sn MNC208U90009 dc 200828
> [ 7.764997] mvneta d0040000.ethernet eth1: unsupported SFP module: no common interface modes
> [ 7.785362] sfp sfp-eth0: module Mikrotik S-RJ01 rev 1.0 sn 61B103C55C58 dc 201022
> [ 7.803426] hwmon hwmon2: temp1_input not attached to any thermal zone
>
> Link: https://lore.kernel.org/r/20230926160255.330417-1-robert.marko@sartura.hr #1
>
> Cc: stable@...r.kernel.org # 6.3+
> Fixes: 20cb3fce4d60 ("i2c: Set i2c pinctrl recovery info from it's device pinctrl")
> Signed-off-by: Gabor Juhos <j4g8y7@...il.com>
> ---
Tested-by: Robert Marko <robert.marko@...tura.hr>
> Changes in v3:
> - rebase on tip of i2c/for-current
> - rework the patch and use a different approach which does not requires
> modification in the I2C core code and update commit description
> acccordingly
> - remove Imre's SoB tag, it should have been a Reviewed-by tag, but due
> to the rework this is an entirely different patch so that does not
> apply anyway
> - use Link tag for the URL of the referenced LKML thread
> - Link to v2: https://lore.kernel.org/r/20250811-i2c-pxa-fix-i2c-communication-v2-2-ca42ea818dc9@gmail.com
>
> Changes in v2:
> - rebase and retest on tip of i2c/for-current
> - Link to v1: https://lore.kernel.org/r/20250511-i2c-pxa-fix-i2c-communication-v1-2-e9097d09a015@gmail.com
> ---
> drivers/i2c/busses/i2c-pxa.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 968a8b8794dac3398a68d827c567aa5bb73ae3d7..70acf33e1d573231f84a1f09cffb376a8277351d 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -268,6 +268,7 @@ struct pxa_i2c {
> struct pinctrl *pinctrl;
> struct pinctrl_state *pinctrl_default;
> struct pinctrl_state *pinctrl_recovery;
> + bool reset_before_xfer;
> };
>
> #define _IBMR(i2c) ((i2c)->reg_ibmr)
> @@ -1144,6 +1145,11 @@ static int i2c_pxa_xfer(struct i2c_adapter *adap,
> {
> struct pxa_i2c *i2c = adap->algo_data;
>
> + if (i2c->reset_before_xfer) {
> + i2c_pxa_reset(i2c);
> + i2c->reset_before_xfer = false;
> + }
> +
> return i2c_pxa_internal_xfer(i2c, msgs, num, i2c_pxa_do_xfer);
> }
>
> @@ -1521,7 +1527,16 @@ static int i2c_pxa_probe(struct platform_device *dev)
> }
> }
>
> - i2c_pxa_reset(i2c);
> + /*
> + * Skip reset on Armada 3700 when recovery is used to avoid
> + * controller hang due to the pinctrl state changes done by
> + * the generic recovery initialization code. The reset will
> + * be performed later, prior to the first transfer.
> + */
> + if (i2c_type == REGS_A3700 && i2c->adap.bus_recovery_info)
> + i2c->reset_before_xfer = true;
> + else
> + i2c_pxa_reset(i2c);
>
> ret = i2c_add_numbered_adapter(&i2c->adap);
> if (ret < 0)
>
> --
> 2.50.1
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@...tura.hr
Web: www.sartura.hr
Powered by blists - more mailing lists