[PATCH 0/4] i2c: riic: Fixes and cleanups

[PATCH 0/4] i2c: riic: Fixes and cleanups

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds one fix and few cleanups for the Renesas RIIC I2C driver.

Thank you,
Claudiu

Claudiu Beznea (4):
  i2c: riic: Abort the transfer on completion timeout
  i2c: riic: Use the "dev_name:irq_name" format for the interrupt name
  i2c: riic: Drop the space in front of the "out" goto label
  i2c: riic: Drop empty line

 drivers/i2c/busses/i2c-riic.c | 41 ++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 1/4] i2c: riic: Abort the transfer on completion timeout

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

riic_xfer() configures a transfer and waits for the interrupt handler to
signal its completion. If the completion times out, the device may be
powered off through runtime PM. However, the transfer interrupt may still
fire after the timeout.

To avoid cases where riic_xfer() powers off the device (via runtime PM)
while the interrupt handler is still running, disable and synchronize the
interrupts after the completion times out.

Fixes: 310c18a41450 ("i2c: riic: add driver")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

 drivers/i2c/busses/i2c-riic.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 9e3595b3623e..68d359ce2e66 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -112,6 +112,7 @@ struct riic_dev {
 	void __iomem *base;
 	u8 *buf;
 	struct i2c_msg *msg;
+	int *irqs;
 	int bytes_left;
 	int err;
 	int is_last;
@@ -165,6 +166,20 @@ static int riic_bus_barrier(struct riic_dev *riic)
 	return 0;
 }
 
+static void riic_abort_xfer(struct riic_dev *riic)
+{
+	/*
+	 * Disable interrupts. Read back registers to confirm writes have
+	 * fully propagated.
+	 */
+	riic_writeb(riic, 0, RIIC_ICIER);
+	riic_readb(riic, RIIC_ICIER);
+
+	/* Synchronize IRQs */
+	for (unsigned int i = 0; i < riic->info->num_irqs; i++)
+		synchronize_irq(riic->irqs[i]);
+}
+
 static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 {
 	struct riic_dev *riic = i2c_get_adapdata(adap);
@@ -196,8 +211,10 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		riic_writeb(riic, start_bit, RIIC_ICCR2);
 
 		time_left = wait_for_completion_timeout(&riic->msg_done, riic->adapter.timeout);
-		if (time_left == 0)
+		if (time_left == 0) {
+			riic_abort_xfer(riic);
 			riic->err = -ETIMEDOUT;
+		}
 
 		if (riic->err)
 			break;
@@ -543,16 +560,20 @@ static int riic_i2c_probe(struct platform_device *pdev)
 
 	riic->info = of_device_get_match_data(dev);
 
+	riic->irqs = devm_kcalloc(&pdev->dev, riic->info->num_irqs,
+				  sizeof(*riic->irqs), GFP_KERNEL);
+	if (!riic->irqs)
+		return -ENOMEM;
+
 	for (i = 0; i < riic->info->num_irqs; i++) {
 		const struct riic_irq_desc *irq_desc;
-		int irq;
 
 		irq_desc = &riic->info->irqs[i];
-		irq = platform_get_irq(pdev, irq_desc->res_num);
-		if (irq < 0)
-			return irq;
+		riic->irqs[i] = platform_get_irq(pdev, irq_desc->res_num);
+		if (riic->irqs[i] < 0)
+			return riic->irqs[i];
 
-		ret = devm_request_irq(dev, irq, irq_desc->isr, 0, irq_desc->name, riic);
+		ret = devm_request_irq(dev, riic->irqs[i], irq_desc->isr, 0, irq_desc->name, riic);
 		if (ret)
 			return dev_err_probe(dev, ret, "failed to request irq %s\n",
 					     irq_desc->name);
-- 
2.43.0

Re: [PATCH 1/4] i2c: riic: Abort the transfer on completion timeout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On Wed, May 20, 2026 at 04:12:03PM +0300, Claudiu Beznea wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> riic_xfer() configures a transfer and waits for the interrupt handler to
> signal its completion. If the completion times out, the device may be
> powered off through runtime PM. However, the transfer interrupt may still
> fire after the timeout.
> 
> To avoid cases where riic_xfer() powers off the device (via runtime PM)
> while the interrupt handler is still running, disable and synchronize the
> interrupts after the completion times out.
> 
> Fixes: 310c18a41450 ("i2c: riic: add driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

How did you test this?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] i2c: riic: Abort the transfer on completion timeout

[Claudiu Beznea]

Hi, Wolfram,

On 5/28/26 16:57, Wolfram Sang wrote:
> On Wed, May 20, 2026 at 04:12:03PM +0300, Claudiu Beznea wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> riic_xfer() configures a transfer and waits for the interrupt handler to
>> signal its completion. If the completion times out, the device may be
>> powered off through runtime PM. However, the transfer interrupt may still
>> fire after the timeout.
>>
>> To avoid cases where riic_xfer() powers off the device (via runtime PM)
>> while the interrupt handler is still running, disable and synchronize the
>> interrupts after the completion times out.
>>
>> Fixes: 310c18a41450 ("i2c: riic: add driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> How did you test this?

I haven't managed to reproduce it on i2c. I reproduced a similar behavior while 
working on renesas-i3c driver and decided to implement this on this i2c driver 
as well.

-- 
Thank you,
Claudiu

Re: [PATCH 1/4] i2c: riic: Abort the transfer on completion timeout

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]


> I haven't managed to reproduce it on i2c. I reproduced a similar behavior
> while working on renesas-i3c driver and decided to implement this on this
> i2c driver as well.

I think the issue is real, but I am not convinced the solution is
complete without a verified testcase.

Maybe it is not too hard to test: you could hack the timeout to be lower
and then write a block of data to an EEPROM. Its erase cycle can trigger
the timeout then...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/4] i2c: riic: Use the "dev_name:irq_name" format for the interrupt name

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Use the "dev_name:irq_name" format for the interrupt names. This makes it
easier to identify interrupts in systems where multiple devices may request
interrupts with the same name.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

 drivers/i2c/busses/i2c-riic.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 68d359ce2e66..57b64d82b681 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -567,13 +567,18 @@ static int riic_i2c_probe(struct platform_device *pdev)
 
 	for (i = 0; i < riic->info->num_irqs; i++) {
 		const struct riic_irq_desc *irq_desc;
+		char *irqname;
 
 		irq_desc = &riic->info->irqs[i];
 		riic->irqs[i] = platform_get_irq(pdev, irq_desc->res_num);
 		if (riic->irqs[i] < 0)
 			return riic->irqs[i];
 
-		ret = devm_request_irq(dev, riic->irqs[i], irq_desc->isr, 0, irq_desc->name, riic);
+		irqname = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", dev_name(dev), irq_desc->name);
+		if (!irqname)
+			return -ENOMEM;
+
+		ret = devm_request_irq(dev, riic->irqs[i], irq_desc->isr, 0, irqname, riic);
 		if (ret)
 			return dev_err_probe(dev, ret, "failed to request irq %s\n",
 					     irq_desc->name);
-- 
2.43.0

Re: [PATCH 2/4] i2c: riic: Use the "dev_name:irq_name" format for the interrupt name

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

On Wed, May 20, 2026 at 04:12:04PM +0300, Claudiu Beznea wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Use the "dev_name:irq_name" format for the interrupt names. This makes it
> easier to identify interrupts in systems where multiple devices may request
> interrupts with the same name.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Looks good. I can do testing next week.

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] i2c: riic: Drop the space in front of the "out" goto label

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Drop the space in front of the "out" goto label from riic_xfer() to follow
the pattern used in tree.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

 drivers/i2c/busses/i2c-riic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 57b64d82b681..c15f79868031 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -222,7 +222,7 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		start_bit = ICCR2_RS;
 	}
 
- out:
+out:
 	pm_runtime_put_autosuspend(dev);
 
 	return riic->err ?: num;
-- 
2.43.0

Re: [PATCH 3/4] i2c: riic: Drop the space in front of the "out" goto label

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On Wed, May 20, 2026 at 04:12:05PM +0300, Claudiu Beznea wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Drop the space in front of the "out" goto label from riic_xfer() to follow
> the pattern used in tree.

Time for this quote again :)

> > "> It is generally accepted to indent labels with a single space. This
> >  > avoids breaking the -p option of diff."

And yeah, I have been bitten by that.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] i2c: riic: Drop the space in front of the "out" goto label

[Claudiu Beznea]

On 5/28/26 17:08, Wolfram Sang wrote:
> On Wed, May 20, 2026 at 04:12:05PM +0300, Claudiu Beznea wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Drop the space in front of the "out" goto label from riic_xfer() to follow
>> the pattern used in tree.
> 
> Time for this quote again :)
> 
>>> "> It is generally accepted to indent labels with a single space. This
>>>   > avoids breaking the -p option of diff."

OK, I wasn't aware of this. Should we align the other goto label to this, for 
consistency?

> 
> And yeah, I have been bitten by that.
> 

-- 
Thank you,
Claudiu

Re: [PATCH 3/4] i2c: riic: Drop the space in front of the "out" goto label

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 106 bytes --]


> OK, I wasn't aware of this. Should we align the other goto label to this,
> for consistency?

Can do.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/4] i2c: riic: Drop empty line

[Claudiu Beznea]

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Drop empty line to follow the coding style.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

 drivers/i2c/busses/i2c-riic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index c15f79868031..33cc96125b73 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -584,7 +584,6 @@ static int riic_i2c_probe(struct platform_device *pdev)
 					     irq_desc->name);
 	}
 
-
 	adap = &riic->adapter;
 	i2c_set_adapdata(adap, riic);
 	strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name));
-- 
2.43.0

Re: [PATCH 4/4] i2c: riic: Drop empty line

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On Wed, May 20, 2026 at 04:12:06PM +0300, Claudiu Beznea wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Drop empty line to follow the coding style.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]