| 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: <CA+HBbNE9yKj6pnZZm3m_+vsUjOvMPtuQQJ2SjZ9XSgvBf0n0Kw@mail.gmail.com>
Date: Mon, 26 Jan 2026 10:12:49 +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, Imre Kaloz <kaloz@...nwrt.org>
Subject: Re: [PATCH v3 2/2] i2c: pxa: handle 'Early Bus Busy' condition on
Armada 3700
On Wed, Aug 27, 2025 at 7:14 PM Gabor Juhos <j4g8y7@...il.com> wrote:
>
> Under some circumstances I2C recovery fails on Armada 3700. At least
> on the Methode uDPU board, removing and replugging an SFP module fails
> often, like this:
>
> [ 36.953127] sfp sfp-eth1: module removed
> [ 38.468549] i2c i2c-1: i2c_pxa: timeout waiting for bus free
> [ 38.486960] sfp sfp-eth1: module MENTECHOPTO POS22-LDCC-KR rev 1.0 sn MNC208U90009 dc 200828
> [ 38.496867] mvneta d0040000.ethernet eth1: unsupported SFP module: no common interface modes
> [ 38.521448] hwmon hwmon2: temp1_input not attached to any thermal zone
> [ 39.249196] sfp sfp-eth1: module removed
> ...
> [ 292.568799] sfp sfp-eth1: please wait, module slow to respond
> ...
> [ 625.208814] sfp sfp-eth1: failed to read EEPROM: -EREMOTEIO
>
> Note that the 'unsupported SFP module' messages are not relevant. The
> module is used only for testing the I2C recovery funcionality, because
> the error can be triggered easily with this specific one.
>
> Enabling debug in the i2c-pxa driver reveals the following:
>
> [ 82.034678] sfp sfp-eth1: module removed
> [ 90.008654] i2c i2c-1: slave_0x50 error: timeout with active message
> [ 90.015112] i2c i2c-1: msg_num: 2 msg_idx: 0 msg_ptr: 0
> [ 90.020464] i2c i2c-1: IBMR: 00000003 IDBR: 000000a0 ICR: 000007e0 ISR: 00000802
> [ 90.027906] i2c i2c-1: log:
> [ 90.030787]
>
> This continues until the retries are exhausted ...
>
> [ 110.192489] i2c i2c-1: slave_0x50 error: exhausted retries
> [ 110.198012] i2c i2c-1: msg_num: 2 msg_idx: 0 msg_ptr: 0
> [ 110.203323] i2c i2c-1: IBMR: 00000003 IDBR: 000000a0 ICR: 000007e0 ISR: 00000802
> [ 110.210810] i2c i2c-1: log:
> [ 110.213633]
>
> ... then the whole sequence starts again ...
>
> [ 115.368641] i2c i2c-1: slave_0x50 error: timeout with active message
>
> ... while finally the SFP core gives up:
>
> [ 671.975258] sfp sfp-eth1: failed to read EEPROM: -EREMOTEIO
>
> When we analyze the log, it can be seen that bit 1 and 11 is set in the
> ISR (Interface Status Register). Bit 1 indicates the ACK/NACK status, but
> the purpose of bit 11 is not documented in the driver code unfortunately.
>
> The 'Functional Specification' document of the Armada 3700 SoCs family
> however says that this bit indicates an 'Early Bus Busy' condition. The
> document also notes that whenever this bit is set, it is not possible to
> initiate a transaction on the I2C bus. The observed behaviour corresponds
> to this statement.
>
> Unfortunately, I2C recovery does not help as it never runs in this
> special case. Although the driver checks the busyness of the bus at
> several places, but since it does not consider the A3700 specific bit
> in these checks it can't determine the actual status of the bus correctly
> which results in the errors above.
>
> In order to fix the problem, add a new member to struct 'i2c_pxa' to
> store a controller specific bitmask containing the bits indicating the
> busy status, and use that in the code while checking the actual status
> of the bus. This ensures that the correct status can be determined on
> the Armada 3700 based devices without causing functional changes on
> devices based on other SoCs.
>
> With the change applied, the driver detects the busy condition, and runs
> the recovery process:
>
> [ 742.617312] i2c i2c-1: state:i2c_pxa_wait_bus_not_busy:449: ISR=00000802, ICR=000007e0, IBMR=03
> [ 742.626099] i2c i2c-1: i2c_pxa: timeout waiting for bus free
> [ 742.631933] i2c i2c-1: recovery: resetting controller, ISR=0x00000802
> [ 742.638421] i2c i2c-1: recovery: IBMR 0x00000003 ISR 0x00000000
>
> This clears the EBB bit in the ISR register, so it makes it possible to
> initiate transactions on the I2C bus again.
>
> After this patch, the SFP module used for testing can be removed and
> replugged numerous times without causing the error described at the
> beginning. Previously, the error happened after a few such attempts.
>
> The patch has been tested also with the following kernel versions:
> 5.10.240, 5.15.189, 6.1.148, 6.6.102, 6.12.42, 6.14.11, 6.15.10, 6.16.1
> It improves recovery on all of them.
>
> Cc: stable@...r.kernel.org # 5.10+
> Reviewed-by: Imre Kaloz <kaloz@...nwrt.org>
> 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
> - use Reviewed-by tag for Imre
> - remove Fixes tag as the problem is not caused by the previously mentioned
> commit, simply it is not handled by the code yet
> - update list of tested kernels
> - Link to v2: https://lore.kernel.org/r/20250811-i2c-pxa-fix-i2c-communication-v2-3-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-3-e9097d09a015@gmail.com
> ---
> drivers/i2c/busses/i2c-pxa.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 70acf33e1d573231f84a1f09cffb376a8277351d..19f5da08def11ded1d3de968f50fa5b5851066f5 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -71,6 +71,7 @@
> #define ISR_GCAD (1 << 8) /* general call address detected */
> #define ISR_SAD (1 << 9) /* slave address detected */
> #define ISR_BED (1 << 10) /* bus error no ACK/NAK */
> +#define ISR_A3700_EBB (1 << 11) /* early bus busy for armada 3700 */
>
> #define ILCR_SLV_SHIFT 0
> #define ILCR_SLV_MASK (0x1FF << ILCR_SLV_SHIFT)
> @@ -263,6 +264,7 @@ struct pxa_i2c {
> bool highmode_enter;
> u32 fm_mask;
> u32 hs_mask;
> + u32 busy_mask;
>
> struct i2c_bus_recovery_info recovery;
> struct pinctrl *pinctrl;
> @@ -430,7 +432,7 @@ static int i2c_pxa_wait_bus_not_busy(struct pxa_i2c *i2c)
>
> while (1) {
> isr = readl(_ISR(i2c));
> - if (!(isr & (ISR_IBB | ISR_UB)))
> + if (!(isr & i2c->busy_mask))
> return 0;
>
> if (isr & ISR_SAD)
> @@ -467,7 +469,7 @@ static int i2c_pxa_wait_master(struct pxa_i2c *i2c)
> * quick check of the i2c lines themselves to ensure they've
> * gone high...
> */
> - if ((readl(_ISR(i2c)) & (ISR_UB | ISR_IBB)) == 0 &&
> + if ((readl(_ISR(i2c)) & i2c->busy_mask) == 0 &&
> readl(_IBMR(i2c)) == (IBMR_SCLS | IBMR_SDAS)) {
> if (i2c_debug > 0)
> dev_dbg(&i2c->adap.dev, "%s: done\n", __func__);
> @@ -488,7 +490,7 @@ static int i2c_pxa_set_master(struct pxa_i2c *i2c)
> if (i2c_debug)
> dev_dbg(&i2c->adap.dev, "setting to bus master\n");
>
> - if ((readl(_ISR(i2c)) & (ISR_UB | ISR_IBB)) != 0) {
> + if ((readl(_ISR(i2c)) & i2c->busy_mask) != 0) {
> dev_dbg(&i2c->adap.dev, "%s: unit is busy\n", __func__);
> if (!i2c_pxa_wait_master(i2c)) {
> dev_dbg(&i2c->adap.dev, "%s: error: unit busy\n", __func__);
> @@ -514,7 +516,7 @@ static int i2c_pxa_wait_slave(struct pxa_i2c *i2c)
> dev_dbg(&i2c->adap.dev, "%s: %ld: ISR=%08x, ICR=%08x, IBMR=%02x\n",
> __func__, (long)jiffies, readl(_ISR(i2c)), readl(_ICR(i2c)), readl(_IBMR(i2c)));
>
> - if ((readl(_ISR(i2c)) & (ISR_UB|ISR_IBB)) == 0 ||
> + if ((readl(_ISR(i2c)) & i2c->busy_mask) == 0 ||
> (readl(_ISR(i2c)) & ISR_SAD) != 0 ||
> (readl(_ICR(i2c)) & ICR_SCLE) == 0) {
> if (i2c_debug > 1)
> @@ -1177,7 +1179,7 @@ static int i2c_pxa_pio_set_master(struct pxa_i2c *i2c)
> /*
> * Wait for the bus to become free.
> */
> - while (timeout-- && readl(_ISR(i2c)) & (ISR_IBB | ISR_UB))
> + while (timeout-- && readl(_ISR(i2c)) & i2c->busy_mask)
> udelay(1000);
>
> if (timeout < 0) {
> @@ -1322,7 +1324,7 @@ static void i2c_pxa_unprepare_recovery(struct i2c_adapter *adap)
> * handing control of the bus back to avoid the bus changing state.
> */
> isr = readl(_ISR(i2c));
> - if (isr & (ISR_UB | ISR_IBB)) {
> + if (isr & i2c->busy_mask) {
> dev_dbg(&i2c->adap.dev,
> "recovery: resetting controller, ISR=0x%08x\n", isr);
> i2c_pxa_do_reset(i2c);
> @@ -1479,6 +1481,10 @@ static int i2c_pxa_probe(struct platform_device *dev)
> i2c->fm_mask = pxa_reg_layout[i2c_type].fm;
> i2c->hs_mask = pxa_reg_layout[i2c_type].hs;
>
> + i2c->busy_mask = ISR_UB | ISR_IBB;
> + if (i2c_type == REGS_A3700)
> + i2c->busy_mask |= ISR_A3700_EBB;
> +
> if (i2c_type != REGS_CE4100)
> i2c->reg_isar = i2c->reg_base + pxa_reg_layout[i2c_type].isar;
>
>
> --
> 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