[PATCH 0/8] i2c: nomadik: fix several issues in fault handling, optimize layout

[PATCH 0/8] i2c: nomadik: fix several issues in fault handling, optimize layout

[Dmitry Guzman]

This patchset fixes several issues in i2c-nomadik driver.

Patches #1..#5 were previously submitted as a part of a patchset that
introduces detailed fault reporting feature in i2c-nomadik and
i2c-designware drivers and i2c-core.

In patch #5, the new structure declaration is moved to different place
to make it independent of patch #3 (avoid merge conflicts).

Patches #6..#8 fix other pre-existing issues found by AI review of that
patchset.

All patches are mutually independent and may be applied separately.

Patch #1 Optimize struct layout.
Patch #2 Remove automatic retransfer on failures.
Patch #3 Fix error codes returned by xfer() callback.
Patch #4 Avoid excessive prints by replacing dev_err() with dev_dbg() on
	I2C faults.
Patch #5 Add quirks that describe hardware limitations of the controller.
Patch #6 Handle PM runtime errors.
Patch #7 Fix race condition with IRQ handler.
Patch #8 Fix possible long busy-wait loop in interrupt handler.

Link to the patch set that patches 1..5 were submitted in:

https://lore.kernel.org/r/20260903-i2c-fault-reporting-v2-0-fedeb91792e6@mobileye.com

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
Dmitry Guzman (8):
      i2c: nomadik: optimize struct layout, change unsigned char into u8
      i2c: nomadik: do not try to retransmit I2C message series on errors
      i2c: nomadik: return proper fault codes
      i2c: nomadik: change print level for fault messages to debug
      i2c: nomadik: add quirks max_len=2047 and no_zero_len_read
      i2c: nomadik: handle pm_runtime errors
      i2c: nomadik: fix race condition with interrupt handler
      i2c: nomadik: reduce FIFO flush timeout to avoid long interrupt handling

 drivers/i2c/busses/i2c-nomadik.c | 105 +++++++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 43 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261025-i2c-nomadik-fixes-73121cc61408

Best regards,
--  
Dmitry Guzman <Dmitry.Guzman@mobileye.com>

[PATCH 1/8] i2c: nomadik: optimize struct layout, change unsigned char into u8

[Dmitry Guzman]

Change type of `tft` and `rft` fields of `nmk_i2c_dev` structure from
`unsigned char` to `u8` and `buffer` field of `i2c_nmk_client` structure
from `unsigned char *` to `u8 *`.

Put two bool variables `xfer_done` and `has_32b_bus` and two u8 variables
`tft` and `rft` together in order to reduce struct size wasted for padding.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index c3878670da9c..45b4815b0945 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -170,7 +170,7 @@ enum i2c_operating_mode {
 struct i2c_nmk_client {
 	unsigned short		slave_adr;
 	unsigned long		count;
-	unsigned char		*buffer;
+	u8			*buffer;
 	unsigned long		xfer_bytes;
 	enum i2c_operation	operation;
 };
@@ -187,13 +187,13 @@ struct i2c_nmk_client {
  * @clk_freq: clock frequency for the operation mode
  * @tft: Tx FIFO Threshold in bytes
  * @rft: Rx FIFO Threshold in bytes
+ * @xfer_done: xfer done boolean.
+ * @has_32b_bus: controller is on a bus that only supports 32-bit accesses.
  * @timeout_usecs: Slave response timeout
  * @sm: speed mode
  * @stop: stop condition.
  * @xfer_wq: xfer done wait queue.
- * @xfer_done: xfer done boolean.
  * @result: controller propogated result.
- * @has_32b_bus: controller is on a bus that only supports 32-bit accesses.
  */
 struct nmk_i2c_dev {
 	struct i2c_vendor_data		*vendor;
@@ -204,15 +204,15 @@ struct nmk_i2c_dev {
 	struct clk			*clk;
 	struct i2c_nmk_client		cli;
 	u32				clk_freq;
-	unsigned char			tft;
-	unsigned char			rft;
+	u8				tft;
+	u8				rft;
+	bool				xfer_done;
+	bool				has_32b_bus;
 	u32				timeout_usecs;
 	enum i2c_freq_mode		sm;
 	int				stop;
 	struct wait_queue_head		xfer_wq;
-	bool				xfer_done;
 	int				result;
-	bool				has_32b_bus;
 };
 
 /* controller's abort causes */

-- 
2.43.0

[PATCH 2/8] i2c: nomadik: do not try to retransmit I2C message series on errors

[Dmitry Guzman]

i2c-nomadik driver of I2C bus controller in xfer() callback retransmits
the whole message series in cause of any fault, and returns fault only
after third failed attempt. This behavior contradicts with API because
not only it hides hardware faults, but also re-sends messages, while
they are not guaranteed to be idempotent.

Remove the triple attempt to send messages in xfer() callback.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 45b4815b0945..ca9366e09e71 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -716,27 +716,21 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	int status = 0;
 	int i;
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
-	int j;
 
 	pm_runtime_get_sync(&priv->adev->dev);
 
-	/* Attempt three times to send the message queue */
-	for (j = 0; j < 3; j++) {
-		/* setup the i2c controller */
-		setup_i2c_controller(priv);
-
-		for (i = 0; i < num_msgs; i++) {
-			priv->cli.slave_adr	= msgs[i].addr;
-			priv->cli.buffer		= msgs[i].buf;
-			priv->cli.count		= msgs[i].len;
-			priv->stop = (i < (num_msgs - 1)) ? 0 : 1;
-			priv->result = 0;
-
-			status = nmk_i2c_xfer_one(priv, msgs[i].flags);
-			if (status != 0)
-				break;
-		}
-		if (status == 0)
+	/* setup the i2c controller */
+	setup_i2c_controller(priv);
+
+	for (i = 0; i < num_msgs; i++) {
+		priv->cli.slave_adr	= msgs[i].addr;
+		priv->cli.buffer		= msgs[i].buf;
+		priv->cli.count		= msgs[i].len;
+		priv->stop = (i < (num_msgs - 1)) ? 0 : 1;
+		priv->result = 0;
+
+		status = nmk_i2c_xfer_one(priv, msgs[i].flags);
+		if (status != 0)
 			break;
 	}
 

-- 
2.43.0

[PATCH 3/8] i2c: nomadik: return proper fault codes

[Dmitry Guzman]

I2C documentation Documentation/i2c/fault-codes.rst defines fault codes
for different negative results in I2C transmittion. Previously,
i2c-nomadik driver didn't implement them properly - it returned
ETIMEDOUT on most errors and EIO on master arbitration lost.

To comply with the documentation, return the proper fault codes for
different conditions, namely:

- EAGAIN if arbitration lost
- EOVERFLOW if message is too long (>2047 bytes)
- ENXIO if target address is not acknowledged
- EIO on other errors detected by controller (for example, NACK on data)
- ETIMEDOUT if driver gets timeout waiting for message completion
  without any fault condition detected by the controller (for example,
too long message, or SDA/SCL line stuck on 0).

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index ca9366e09e71..a3cef4b66190 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -226,6 +226,18 @@ static const char *abort_causes[] = {
 	"overflow, maxsize is 2047 bytes",
 };
 
+/* Linux fault codes for controller abort causes */
+static int fault_codes[] = {
+	ENXIO,
+	EIO,
+	EIO,
+	EAGAIN,
+	EIO,
+	EIO,
+	EOVERFLOW,
+	EIO
+};
+
 static inline void i2c_set_bit(void __iomem *reg, u32 mask)
 {
 	writel(readl(reg) | mask, reg);
@@ -653,6 +665,8 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 				cause >= ARRAY_SIZE(abort_causes) ?
 				"unknown reason" :
 				abort_causes[cause]);
+			priv->result = -fault_codes[cause];
+			status = priv->result;
 		}
 
 		init_hw(priv);
@@ -865,7 +879,7 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 
 	/* Master Arbitration lost interrupt */
 	case I2C_IT_MAL:
-		priv->result = -EIO;
+		priv->result = -EAGAIN;
 		init_hw(priv);
 
 		i2c_set_bit(priv->virtbase + I2C_ICR, I2C_IT_MAL);

-- 
2.43.0

[PATCH 4/8] i2c: nomadik: change print level for fault messages to debug

[Dmitry Guzman]

i2c-nomadik driver prints error message on every faulted message. This
is not a good practice, because in I2C a fault not always is an error,
sometimes it is the expected result. For example, scanning bus with
`i2cdetect` prints over 100 messages in dmesg (two messages per each
target address).

To avoid excessive prints in the log, change the print level from err to
debug.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index a3cef4b66190..9c7658148bd4 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -627,7 +627,7 @@ static int write_i2c(struct nmk_i2c_dev *priv, u16 flags)
 
 	if (!xfer_done) {
 		/* Controller timed out */
-		dev_err(&priv->adev->dev, "write to slave 0x%x timed out\n",
+		dev_dbg(&priv->adev->dev, "write to slave 0x%x timed out\n",
 			priv->cli.slave_adr);
 		status = -ETIMEDOUT;
 	}
@@ -661,7 +661,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 		i2c_sr = readl(priv->virtbase + I2C_SR);
 		if (FIELD_GET(I2C_SR_STATUS, i2c_sr) == I2C_ABORT) {
 			cause = FIELD_GET(I2C_SR_CAUSE, i2c_sr);
-			dev_err(&priv->adev->dev, "%s\n",
+			dev_dbg(&priv->adev->dev, "%s\n",
 				cause >= ARRAY_SIZE(abort_causes) ?
 				"unknown reason" :
 				abort_causes[cause]);

-- 
2.43.0

[PATCH 5/8] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read

[Dmitry Guzman]

In Nomadik I2c controller, register I2C_MCR has 11-bit wide LENGTH
field. Its maximum value is 2047, so this is the maximum length of a
single message. It is less than the common maximum I2C message length in
I2C subsystem (8192), so define a quirk in order to report the
unsupported message without any attempt to transfer it.

Zero length reading doesn't work properly on this controller, so add
`I2C_AQ_NO_ZERO_LEN_READ` quirk flag.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 9c7658148bd4..d07246d11b35 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -79,6 +79,9 @@
 #define I2C_MCR_STOP		BIT(14)		/* Stop condition */
 #define I2C_MCR_LENGTH		GENMASK(25, 15)	/* Transaction length */
 
+/* Controller hardware limitation of the message length */
+#define I2C_MAX_MSG_LENGTH	(I2C_MCR_LENGTH >> 15)
+
 /* Status register (SR) */
 #define I2C_SR_OP		GENMASK(1, 0)	/* Operation */
 #define I2C_SR_STATUS		GENMASK(3, 2)	/* controller status */
@@ -1087,6 +1090,12 @@ static const struct of_device_id nmk_i2c_eyeq_match_table[] = {
 	{ /* sentinel */ }
 };
 
+static const struct i2c_adapter_quirks nmk_i2c_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+	.max_read_len  = I2C_MAX_MSG_LENGTH,
+	.max_write_len = I2C_MAX_MSG_LENGTH,
+};
+
 static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 {
 	struct i2c_vendor_data *vendor = id->data;
@@ -1161,6 +1170,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	adap->class = I2C_CLASS_DEPRECATED;
 	adap->algo = &nmk_i2c_algo;
 	adap->timeout = usecs_to_jiffies(priv->timeout_usecs);
+	adap->quirks = &nmk_i2c_quirks;
 	snprintf(adap->name, sizeof(adap->name),
 		 "Nomadik I2C at %pR", &adev->res);
 

-- 
2.43.0

Re: [PATCH 5/8] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read

[Linus Walleij]

On Tue, Nov 3, 2026 at 9:29 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:

> In Nomadik I2c controller, register I2C_MCR has 11-bit wide LENGTH
> field. Its maximum value is 2047, so this is the maximum length of a
> single message. It is less than the common maximum I2C message length in
> I2C subsystem (8192), so define a quirk in order to report the
> unsupported message without any attempt to transfer it.
>
> Zero length reading doesn't work properly on this controller, so add
> `I2C_AQ_NO_ZERO_LEN_READ` quirk flag.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Dmitry Guzman]

In nmk_i2c_xfer(), pm_runtime_get_sync() is called on start and then
the controller registers are accessed without checking the return value.
If pm_runtime_get_sync() fails, access to the register of the device that
was not correctly powered on may cause a bus fault or other unpredictable
system behavior.

Check the return value of pm_runtime_get_sync() and it if is not 0,
return the error immediately.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index d07246d11b35..361a5fc6a587 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -734,7 +734,9 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	int i;
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
 
-	pm_runtime_get_sync(&priv->adev->dev);
+	i = pm_runtime_get_sync(&priv->adev->dev);
+	if (i)
+		return i;
 
 	/* setup the i2c controller */
 	setup_i2c_controller(priv);

-- 
2.43.0

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Linus Walleij]

On Tue, Nov 3, 2026 at 9:30 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:

> In nmk_i2c_xfer(), pm_runtime_get_sync() is called on start and then
> the controller registers are accessed without checking the return value.
> If pm_runtime_get_sync() fails, access to the register of the device that
> was not correctly powered on may cause a bus fault or other unpredictable
> system behavior.
>
> Check the return value of pm_runtime_get_sync() and it if is not 0,
> return the error immediately.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
(...)
> -       pm_runtime_get_sync(&priv->adev->dev);
> +       i = pm_runtime_get_sync(&priv->adev->dev);
> +       if (i)
> +               return i;

It's a bit odd to reuse the i (= iterator...) variable for this.
I would just add an int ret or int err for this.

Either way semantics are right so:
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

On Thu, Nov 05, 2026 at 09:42:02AM +0100, Linus Walleij wrote:
> On Tue, Nov 3, 2026 at 9:30 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:
> 
> > In nmk_i2c_xfer(), pm_runtime_get_sync() is called on start and then
> > the controller registers are accessed without checking the return value.
> > If pm_runtime_get_sync() fails, access to the register of the device that
> > was not correctly powered on may cause a bus fault or other unpredictable
> > system behavior.
> >
> > Check the return value of pm_runtime_get_sync() and it if is not 0,
> > return the error immediately.
> >
> > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
> (...)
> > -       pm_runtime_get_sync(&priv->adev->dev);
> > +       i = pm_runtime_get_sync(&priv->adev->dev);
> > +       if (i)
> > +               return i;
> 
> It's a bit odd to reuse the i (= iterator...) variable for this.
> I would just add an int ret or int err for this.

Yes, please.

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

On Thu, Nov 05, 2026 at 10:51:07AM +0200, Wolfram Sang wrote:
> On Thu, Nov 05, 2026 at 09:42:02AM +0100, Linus Walleij wrote:
> > On Tue, Nov 3, 2026 at 9:30 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:
> > 
> > > In nmk_i2c_xfer(), pm_runtime_get_sync() is called on start and then
> > > the controller registers are accessed without checking the return value.
> > > If pm_runtime_get_sync() fails, access to the register of the device that
> > > was not correctly powered on may cause a bus fault or other unpredictable
> > > system behavior.
> > >
> > > Check the return value of pm_runtime_get_sync() and it if is not 0,
> > > return the error immediately.
> > >
> > > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
> > (...)
> > > -       pm_runtime_get_sync(&priv->adev->dev);
> > > +       i = pm_runtime_get_sync(&priv->adev->dev);
> > > +       if (i)
> > > +               return i;
> > 
> > It's a bit odd to reuse the i (= iterator...) variable for this.
> > I would just add an int ret or int err for this.
> 
> Yes, please.

I changed this to:

@@ -730,11 +730,12 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *priv, u16 flags)
 static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 		struct i2c_msg msgs[], int num_msgs)
 {
-	int status = 0;
-	int i;
+	int status, i;
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
 
-	pm_runtime_get_sync(&priv->adev->dev);
+	status = pm_runtime_get_sync(&priv->adev->dev);
+	if (status)
+		return status;
 
 	/* setup the i2c controller */
 	setup_i2c_controller(priv);

while applying.

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

> -	pm_runtime_get_sync(&priv->adev->dev);
> +	i = pm_runtime_get_sync(&priv->adev->dev);
> +	if (i)
> +		return i;

Testbots revealed a smatch warning:

drivers/i2c/busses/i2c-nomadik.c:737 nmk_i2c_xfer() warn: pm_runtime_get_sync() also returns 1 on success

I will fix it here.

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

On Mon, Nov 09, 2026 at 08:03:09AM +0100, Wolfram Sang wrote:
> 
> > -	pm_runtime_get_sync(&priv->adev->dev);
> > +	i = pm_runtime_get_sync(&priv->adev->dev);
> > +	if (i)
> > +		return i;
> 
> Testbots revealed a smatch warning:
> 
> drivers/i2c/busses/i2c-nomadik.c:737 nmk_i2c_xfer() warn: pm_runtime_get_sync() also returns 1 on success
> 
> I will fix it here.

Like this:

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 104da275ff91..8cf1d443a25c 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -734,13 +734,13 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
 
 	status = pm_runtime_get_sync(&priv->adev->dev);
-	if (status)
+	if (status < 0)
 		return status;
 
 	/* setup the i2c controller */
 	setup_i2c_controller(priv);
 
-	for (i = 0; i < num_msgs; i++) {
+	for (i = 0, status = 0; i < num_msgs; i++) {
 		priv->cli.slave_adr	= msgs[i].addr;
 		priv->cli.buffer		= msgs[i].buf;
 		priv->cli.count		= msgs[i].len;

Note that setting status = 0 is not really required as the I2C core will
ensure num_msgs to be > 0. But if people really hack around it, they
could circumvent that. And code checkers will recognize this path until
we fix the loophole.

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Dmitry Guzman]

Hi Wolfram,

On Mon, 9 Nov 2026 08:15:29 +0100
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> > Testbots revealed a smatch warning:
> > 
> > drivers/i2c/busses/i2c-nomadik.c:737 nmk_i2c_xfer() warn: pm_runtime_get_sync() also returns 1 on success
> > 
> > I will fix it here.
> 
> Like this:
> 
> diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> index 104da275ff91..8cf1d443a25c 100644
> --- a/drivers/i2c/busses/i2c-nomadik.c
> +++ b/drivers/i2c/busses/i2c-nomadik.c
> @@ -734,13 +734,13 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
>  	struct nmk_i2c_dev *priv = i2c_get_adapdata(i2c_adap);
>  
>  	status = pm_runtime_get_sync(&priv->adev->dev);
> -	if (status)
> +	if (status < 0)
>  		return status;

There is another problem with this patch: 

include/linux/pm_runtime.h:

> * Note that the runtime PM usage counter of @dev remains incremented in all
> * cases, even if it returns an error code. Consider using
> * pm_runtime_resume_and_get() instead, especially if the return value is
> * checked by the caller, as this is likely to result in cleaner code.

I think we just need to replace pm_runtime_get_sync(&priv->adev->dev) with
pm_runtime_resume_and_get(&priv->adev->dev).

This solves both the positive return code issue and the usage counter leak.

Do you agree?

Best regards,
--
Dmitry Guzman <Dmitry.Guzman@mobileye.com>

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

> This solves both the positive return code issue and the usage counter leak.
> 
> Do you agree?

Having a leak less and cleaner code? Yes, I am sold :)

Re: [PATCH 6/8] i2c: nomadik: handle pm_runtime errors

[Wolfram Sang]

> Having a leak less and cleaner code? Yes, I am sold :)

Can you please resend an updated patch? The code change is easy for a
rebase but the commit message needs to be rewritten completely which
calls for a new version of the patch, I'd think.

[PATCH 7/8] i2c: nomadik: fix race condition with interrupt handler

[Dmitry Guzman]

If nmk_i2c_wait_xfer_done() gets timeout on waiting for wakeup from
interrupt handler, there is no synchronization with interrupt handler
that may run in parallel. This can potentially lead to various race
conditions between thread and interrupt handler.

Disable and clear all pending interrupt requests and wait until running
interrupt handler finishes after the timeout.

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 361a5fc6a587..cc8b5a357f06 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -505,6 +505,11 @@ static bool nmk_i2c_wait_xfer_done(struct nmk_i2c_dev *priv)
 		wait_event_timeout(priv->xfer_wq, priv->xfer_done, timeout);
 	}
 
+	/* Disable interrupts and wait for pending interrupts */
+	disable_all_interrupts(priv);
+	clear_all_interrupts(priv);
+	synchronize_irq(priv->irq);
+
 	return priv->xfer_done;
 }
 

-- 
2.43.0

Re: [PATCH 7/8] i2c: nomadik: fix race condition with interrupt handler

[Linus Walleij]

On Tue, Nov 3, 2026 at 9:30 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:

> If nmk_i2c_wait_xfer_done() gets timeout on waiting for wakeup from
> interrupt handler, there is no synchronization with interrupt handler
> that may run in parallel. This can potentially lead to various race
> conditions between thread and interrupt handler.
>
> Disable and clear all pending interrupt requests and wait until running
> interrupt handler finishes after the timeout.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

[PATCH 8/8] i2c: nomadik: reduce FIFO flush timeout to avoid long interrupt handling

[Dmitry Guzman]

In flush_i2c_fifo(), the driver sets FTX and FRX flags and waits until
they are reset (these bits are self-clearing). If after the first read
one or both of these bits is set, it waits until both are reset. The
timeout is 10 * I2C transfer timeout. I2C transfer timeout is by default
200 milliseconds. 2 seconds are much more than needed: as stated in the
comment above, the flushing operation takes just several cycles before
completed.

flush_i2c_fifo() is called from init_hw() that may be called from
interrupt handler. So, if the I2C controller is stuck and the FIFO does
not get flushed, the CPU may spend up to 2 seconds in the interrupt
handler and cause watchdog timeout or other unwanted consequences.
Moreover, if the interrupt is handled by the same CPU core that updates
jiffies, the loop may not finish at all.

To avoid too long waiting, reduce the timeout to 100 microseconds and
use read_poll_timeout_atomic() instead of relying on jiffies.

Fixes: 3f9900f17cdb ("i2c: Add support for Ux500/Nomadik I2C controller")

Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index cc8b5a357f06..1f332e66a243 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -278,9 +278,8 @@ static inline void nmk_i2c_writeb(const struct nmk_i2c_dev *priv, u32 val,
  */
 static int flush_i2c_fifo(struct nmk_i2c_dev *priv)
 {
-#define LOOP_ATTEMPTS 10
-	ktime_t timeout;
-	int i;
+#define FIFO_FLUSH_TIMEOUT_US 100
+	u32 reg, err;
 
 	/*
 	 * flush the transmit and receive FIFO. The flushing
@@ -291,21 +290,16 @@ static int flush_i2c_fifo(struct nmk_i2c_dev *priv)
 	 */
 	writel((I2C_CR_FTX | I2C_CR_FRX), priv->virtbase + I2C_CR);
 
-	for (i = 0; i < LOOP_ATTEMPTS; i++) {
-		timeout = ktime_add_us(ktime_get(), priv->timeout_usecs);
-
-		while (ktime_after(timeout, ktime_get())) {
-			if ((readl(priv->virtbase + I2C_CR) &
-				(I2C_CR_FTX | I2C_CR_FRX)) == 0)
-				return 0;
-		}
-	}
+	/* This may execute in interrupt handler, so use read_poll_timeout_atomic() */
+	err = read_poll_timeout_atomic(readl, reg, (reg & (I2C_CR_FTX | I2C_CR_FRX)) == 0,
+				       1, FIFO_FLUSH_TIMEOUT_US, 0, priv->virtbase + I2C_CR);
 
-	dev_err(&priv->adev->dev,
-		"flushing operation timed out giving up after %d attempts",
-		LOOP_ATTEMPTS);
+	if (err)
+		dev_err(&priv->adev->dev,
+			"flushing operation timed out giving up after %d microseconds",
+			FIFO_FLUSH_TIMEOUT_US);
 
-	return -ETIMEDOUT;
+	return err;
 }
 
 /**

-- 
2.43.0

Re: [PATCH 8/8] i2c: nomadik: reduce FIFO flush timeout to avoid long interrupt handling

[Linus Walleij]

On Tue, Nov 3, 2026 at 9:30 AM Dmitry Guzman <Dmitry.Guzman@mobileye.com> wrote:

> In flush_i2c_fifo(), the driver sets FTX and FRX flags and waits until
> they are reset (these bits are self-clearing). If after the first read
> one or both of these bits is set, it waits until both are reset. The
> timeout is 10 * I2C transfer timeout. I2C transfer timeout is by default
> 200 milliseconds. 2 seconds are much more than needed: as stated in the
> comment above, the flushing operation takes just several cycles before
> completed.
>
> flush_i2c_fifo() is called from init_hw() that may be called from
> interrupt handler. So, if the I2C controller is stuck and the FIFO does
> not get flushed, the CPU may spend up to 2 seconds in the interrupt
> handler and cause watchdog timeout or other unwanted consequences.
> Moreover, if the interrupt is handled by the same CPU core that updates
> jiffies, the loop may not finish at all.
>
> To avoid too long waiting, reduce the timeout to 100 microseconds and
> use read_poll_timeout_atomic() instead of relying on jiffies.
>
> Fixes: 3f9900f17cdb ("i2c: Add support for Ux500/Nomadik I2C controller")

Is it really "fixing" that? I don't know if I agree that this is an error
in the initial submission in 2010...

>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>

Surplus newline above? Maintainer can remove it when applying.
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH 8/8] i2c: nomadik: reduce FIFO flush timeout to avoid long interrupt handling

[Wolfram Sang]

> > In flush_i2c_fifo(), the driver sets FTX and FRX flags and waits until
> > they are reset (these bits are self-clearing). If after the first read
> > one or both of these bits is set, it waits until both are reset. The
> > timeout is 10 * I2C transfer timeout. I2C transfer timeout is by default
> > 200 milliseconds. 2 seconds are much more than needed: as stated in the
> > comment above, the flushing operation takes just several cycles before
> > completed.
> >
> > flush_i2c_fifo() is called from init_hw() that may be called from
> > interrupt handler. So, if the I2C controller is stuck and the FIFO does
> > not get flushed, the CPU may spend up to 2 seconds in the interrupt
> > handler and cause watchdog timeout or other unwanted consequences.
> > Moreover, if the interrupt is handled by the same CPU core that updates
> > jiffies, the loop may not finish at all.
> >
> > To avoid too long waiting, reduce the timeout to 100 microseconds and
> > use read_poll_timeout_atomic() instead of relying on jiffies.
> >
> > Fixes: 3f9900f17cdb ("i2c: Add support for Ux500/Nomadik I2C controller")
> 
> Is it really "fixing" that? I don't know if I agree that this is an error
> in the initial submission in 2010...

Removed it.

> > Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
> 
> Surplus newline above? Maintainer can remove it when applying.

Fixed this.

Thanks for the reviews, Linus.

Re: [PATCH 0/8] i2c: nomadik: fix several issues in fault handling, optimize layout

[Wolfram Sang]

Hi Dmitry,

> Patches #1..#5 were previously submitted as a part of a patchset that
> introduces detailed fault reporting feature in i2c-nomadik and
> i2c-designware drivers and i2c-core.

It was a good move to seperate these patches from the bigger series. I
trust Linus' review, fixed the minor issues, and applied it to my
for-andi breanch.

Happy hacking,

   Wolfram