[PATCH] i2c: ocores: re-check status after poll timeout to avoid false errors

[PATCH] i2c: ocores: re-check status after poll timeout to avoid false errors

[Martin Aberer]

The polling task can be preempted at any point inside ocores_wait(),
including just before the time_after() check. If the scheduler does
not resume the task until after the 1ms deadline, ocores_wait()
returns -ETIMEDOUT even though the hardware already cleared the
status bit.

Re-read the status register after a timeout before declaring failure.
This avoids spurious timeout warnings under high CPU load.

Signed-off-by: Martin Aberer <martin.aberer@bachmann.info>
---
 drivers/i2c/busses/i2c-ocores.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 0f67e57cdeff..6f5aece94d57 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -313,12 +313,24 @@ static int ocores_poll_wait(struct ocores_i2c *i2c)
 	/*
 	 * once we are here we expect to get the expected result immediately
 	 * so if after 1ms we timeout then something is broken.
+	 *
+	 * The polling task can be preempted at any point inside ocores_wait(),
+	 * including just before the time_after() check. If the scheduler does
+	 * not resume the task until after the 1ms deadline, ocores_wait()
+	 * returns -ETIMEDOUT even though the hardware already cleared the
+	 * status bit.
+
+	 * Re-read the status register after a timeout before declaring failure.
+	 * This avoids spurious timeout warnings under high CPU load.
 	 */
 	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
-	if (err)
+	if (err) {
+		if ((oc_getreg(i2c, OCI2C_STATUS) & mask) == 0)
+			return 0;
 		dev_warn(i2c->adap.dev.parent,
 			 "%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",
 			 __func__, mask);
+	}
 	return err;
 }
 
-- 
2.43.0

Re: [PATCH] i2c: ocores: re-check status after poll timeout to avoid false errors

[Christian Gmeiner]

Hey Martin

> The polling task can be preempted at any point inside ocores_wait(),
> including just before the time_after() check. If the scheduler does
> not resume the task until after the 1ms deadline, ocores_wait()
> returns -ETIMEDOUT even though the hardware already cleared the
> status bit.
>
> Re-read the status register after a timeout before declaring failure.
> This avoids spurious timeout warnings under high CPU load.
>
> Signed-off-by: Martin Aberer <martin.aberer@bachmann.info>
> ---
>  drivers/i2c/busses/i2c-ocores.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index 0f67e57cdeff..6f5aece94d57 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -313,12 +313,24 @@ static int ocores_poll_wait(struct ocores_i2c *i2c)
>         /*
>          * once we are here we expect to get the expected result immediately
>          * so if after 1ms we timeout then something is broken.
> +        *
> +        * The polling task can be preempted at any point inside ocores_wait(),
> +        * including just before the time_after() check. If the scheduler does
> +        * not resume the task until after the 1ms deadline, ocores_wait()
> +        * returns -ETIMEDOUT even though the hardware already cleared the
> +        * status bit.
> +
> +        * Re-read the status register after a timeout before declaring failure.
> +        * This avoids spurious timeout warnings under high CPU load.
>          */
>         err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
> -       if (err)
> +       if (err) {
> +               if ((oc_getreg(i2c, OCI2C_STATUS) & mask) == 0)
> +                       return 0;
>                 dev_warn(i2c->adap.dev.parent,
>                          "%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",
>                          __func__, mask);
> +       }
>         return err;
>  }
>

In my view, a proper fix would be to switch to read_poll_timeout_atomic().
Something like this - untested:

---8<----

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 0f67e57cdeff..df6ebf32d6e8 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -24,6 +24,7 @@
#include <linux/io.h>
#include <linux/log2.h>
#include <linux/spinlock.h>
+#include <linux/iopoll.h>
#include <linux/jiffies.h>
/*
@@ -258,7 +259,7 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
* @reg: register to query
* @mask: bitmask to apply on register value
* @val: expected result
- * @timeout: timeout in jiffies
+ * @timeout_us: timeout in microseconds
*
* Timeout is necessary to avoid to stay here forever when the chip
* does not answer correctly.
@@ -267,21 +268,14 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
*/
static int ocores_wait(struct ocores_i2c *i2c,
int reg, u8 mask, u8 val,
- const unsigned long timeout)
+ unsigned long timeout_us)
{
- unsigned long j;
+ u8 status;
- j = jiffies + timeout;
- while (1) {
- u8 status = oc_getreg(i2c, reg);
-
- if ((status & mask) == val)
- break;
-
- if (time_after(jiffies, j))
- return -ETIMEDOUT;
- }
- return 0;
+ return read_poll_timeout_atomic(oc_getreg, status,
+ (status & mask) == val,
+ 0, timeout_us, false,
+ i2c, reg);
}
/**
@@ -314,7 +308,7 @@ static int ocores_poll_wait(struct ocores_i2c *i2c)
* once we are here we expect to get the expected result immediately
* so if after 1ms we timeout then something is broken.
*/
- err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
+ err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, 1000);
if (err)
dev_warn(i2c->adap.dev.parent,
"%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",

---8<---

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH] i2c: ocores: re-check status after poll timeout to avoid false errors

[Andrew Lunn]

On Mon, Mar 23, 2026 at 03:34:24PM +0100, Christian Gmeiner wrote:
> Hey Martin
> 
> > The polling task can be preempted at any point inside ocores_wait(),
> > including just before the time_after() check. If the scheduler does
> > not resume the task until after the 1ms deadline, ocores_wait()
> > returns -ETIMEDOUT even though the hardware already cleared the
> > status bit.
> >
> > Re-read the status register after a timeout before declaring failure.
> > This avoids spurious timeout warnings under high CPU load.
> >
> > Signed-off-by: Martin Aberer <martin.aberer@bachmann.info>
> > ---
> >  drivers/i2c/busses/i2c-ocores.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> > index 0f67e57cdeff..6f5aece94d57 100644
> > --- a/drivers/i2c/busses/i2c-ocores.c
> > +++ b/drivers/i2c/busses/i2c-ocores.c
> > @@ -313,12 +313,24 @@ static int ocores_poll_wait(struct ocores_i2c *i2c)
> >         /*
> >          * once we are here we expect to get the expected result immediately
> >          * so if after 1ms we timeout then something is broken.
> > +        *
> > +        * The polling task can be preempted at any point inside ocores_wait(),
> > +        * including just before the time_after() check. If the scheduler does
> > +        * not resume the task until after the 1ms deadline, ocores_wait()
> > +        * returns -ETIMEDOUT even though the hardware already cleared the
> > +        * status bit.
> > +
> > +        * Re-read the status register after a timeout before declaring failure.
> > +        * This avoids spurious timeout warnings under high CPU load.
> >          */
> >         err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
> > -       if (err)
> > +       if (err) {
> > +               if ((oc_getreg(i2c, OCI2C_STATUS) & mask) == 0)
> > +                       return 0;
> >                 dev_warn(i2c->adap.dev.parent,
> >                          "%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",
> >                          __func__, mask);
> > +       }
> >         return err;
> >  }
> >
> 
> In my view, a proper fix would be to switch to read_poll_timeout_atomic().
> Something like this - untested:
> 
> ---8<----
> 
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index 0f67e57cdeff..df6ebf32d6e8 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -24,6 +24,7 @@
> #include <linux/io.h>
> #include <linux/log2.h>
> #include <linux/spinlock.h>
> +#include <linux/iopoll.h>
> #include <linux/jiffies.h>
> /*
> @@ -258,7 +259,7 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
> * @reg: register to query
> * @mask: bitmask to apply on register value
> * @val: expected result
> - * @timeout: timeout in jiffies
> + * @timeout_us: timeout in microseconds
> *
> * Timeout is necessary to avoid to stay here forever when the chip
> * does not answer correctly.
> @@ -267,21 +268,14 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
> */
> static int ocores_wait(struct ocores_i2c *i2c,
> int reg, u8 mask, u8 val,
> - const unsigned long timeout)
> + unsigned long timeout_us)
> {
> - unsigned long j;
> + u8 status;

Hi Christian

It looks like your email client has corrupted the patch, messing up
the white space.

Please post a real patch.

FYI: I agree iopoll.h is the way to go, i keep pointing out this class
a bugs in various places and always point developers at those macros.

  Andrew

[PATCH] i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

[Martin Aberer]

Replace the manual polling loop in ocores_wait() with the kernel helper
read_poll_timeout_atomic(). This simplifies the code and ensures robust
timeout handling. By using this helper, we avoid spurious timeout errors
that could occur under high CPU load or preemption, as the macro handles
timing and condition checks atomically.

Signed-off-by: Martin Aberer <martin.aberer@bachmann.info>
---
 drivers/i2c/busses/i2c-ocores.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 0f67e57cdeff..df6ebf32d6e8 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -24,6 +24,7 @@
 #include <linux/io.h>
 #include <linux/log2.h>
 #include <linux/spinlock.h>
+#include <linux/iopoll.h>
 #include <linux/jiffies.h>
 
 /*
@@ -258,7 +259,7 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
  * @reg: register to query
  * @mask: bitmask to apply on register value
  * @val: expected result
- * @timeout: timeout in jiffies
+ * @timeout_us: timeout in microseconds
  *
  * Timeout is necessary to avoid to stay here forever when the chip
  * does not answer correctly.
@@ -267,21 +268,14 @@ static void ocores_process_timeout(struct ocores_i2c *i2c)
  */
 static int ocores_wait(struct ocores_i2c *i2c,
 		       int reg, u8 mask, u8 val,
-		       const unsigned long timeout)
+		       unsigned long timeout_us)
 {
-	unsigned long j;
+	u8 status;
 
-	j = jiffies + timeout;
-	while (1) {
-		u8 status = oc_getreg(i2c, reg);
-
-		if ((status & mask) == val)
-			break;
-
-		if (time_after(jiffies, j))
-			return -ETIMEDOUT;
-	}
-	return 0;
+	return read_poll_timeout_atomic(oc_getreg, status,
+					(status & mask) == val,
+					0, timeout_us, false,
+					i2c, reg);
 }
 
 /**
@@ -314,7 +308,7 @@ static int ocores_poll_wait(struct ocores_i2c *i2c)
 	 * once we are here we expect to get the expected result immediately
 	 * so if after 1ms we timeout then something is broken.
 	 */
-	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, msecs_to_jiffies(1));
+	err = ocores_wait(i2c, OCI2C_STATUS, mask, 0, 1000);
 	if (err)
 		dev_warn(i2c->adap.dev.parent,
 			 "%s: STATUS timeout, bit 0x%x did not clear in 1ms\n",
-- 
2.43.0

Re: [PATCH] i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

[Andi Shyti]

Hi,

On Tue, Mar 24, 2026 at 03:05:56PM +0100, Martin Aberer wrote:
> Replace the manual polling loop in ocores_wait() with the kernel helper
> read_poll_timeout_atomic(). This simplifies the code and ensures robust
> timeout handling. By using this helper, we avoid spurious timeout errors
> that could occur under high CPU load or preemption, as the macro handles
> timing and condition checks atomically.
> 
> Signed-off-by: Martin Aberer <martin.aberer@bachmann.info>

Please, next time don't send this as --in-reply-to, but please
send it as a v2 with a proper changelog.

Other than this, Christian, Andrew, anyone who wants to give this
a look?

Thanks,
Andi

Re: [PATCH] i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

[Andrew Lunn]

On Tue, Mar 24, 2026 at 03:05:56PM +0100, Martin Aberer wrote:
> Replace the manual polling loop in ocores_wait() with the kernel helper
> read_poll_timeout_atomic(). This simplifies the code and ensures robust
> timeout handling. By using this helper, we avoid spurious timeout errors
> that could occur under high CPU load or preemption, as the macro handles
> timing and condition checks atomically.

It is not that it does it atomically, but that it always does a check
after the delay, even if the delay has taken us past the timeout.

Apart from that, and the other comments:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH] i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

[Andi Shyti]

Hi,

On Fri, Mar 27, 2026 at 01:27:16AM +0100, Andrew Lunn wrote:
> On Tue, Mar 24, 2026 at 03:05:56PM +0100, Martin Aberer wrote:
> > Replace the manual polling loop in ocores_wait() with the kernel helper
> > read_poll_timeout_atomic(). This simplifies the code and ensures robust
> > timeout handling. By using this helper, we avoid spurious timeout errors
> > that could occur under high CPU load or preemption, as the macro handles
> > timing and condition checks atomically.
> 
> It is not that it does it atomically, but that it always does a check
> after the delay, even if the delay has taken us past the timeout.

I took the liberty of changing the commit log to this:

    i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

    Replace the manual polling loop in ocores_wait() with the kernel helper
    read_poll_timeout_atomic(). This simplifies the code and ensures robust
    timeout handling.

    In particular, the helper guarantees a condition check after the
    delay, even if the delay exceeds the timeout, avoiding spurious
    timeout errors under load or preemption.

Let me know if it doesn't work.

I merged the patch to i2c/i2c-host.

Thanks,
Andi

Re: [PATCH] i2c: ocores: Use read_poll_timeout_atomic to avoid false poll timeouts

[Andrew Lunn]

>     In particular, the helper guarantees a condition check after the
>     delay, even if the delay exceeds the timeout, avoiding spurious
>     timeout errors under load or preemption.
> 
> Let me know if it doesn't work.

That is good, thanks.

     Andrew