[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB11346F947E11B80E51D8B6DEC8681A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Mon, 12 Jan 2026 16:51:25 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>, Tommaso Merciai
<tomm.merciai@...il.com>
CC: "linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
Chris Brandt <Chris.Brandt@...esas.com>, Andi Shyti <andi.shyti@...nel.org>,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2] i2c: riic: Move suspend handling to NOIRQ phase
Hi Tommaso,
Thanks for the patch.
> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> Sent: 12 January 2026 16:24
> Subject: Re: [PATCH v2] i2c: riic: Move suspend handling to NOIRQ phase
>
> Dear All,
> A gentle ping on this patch.
>
> Kind Regards,
> Tommaso
>
> On Thu, Dec 18, 2025 at 04:10:21PM +0100, Tommaso Merciai wrote:
> > Commit 53326135d0e0 ("i2c: riic: Add suspend/resume support") added
> > suspend support for the Renesas I2C driver and following this change
> > on RZ/G3E the following WARNING is seen on entering suspend ...
> >
> > [ 134.275704] Freezing remaining freezable tasks completed (elapsed
> > 0.001 seconds) [ 134.285536] ------------[ cut here ]------------ [
> > 134.290298] i2c i2c-2: Transfer while suspended [ 134.295174]
> > WARNING: drivers/i2c/i2c-core.h:56 at __i2c_smbus_xfer+0x1e4/0x214,
> > CPU#0: systemd-sleep/388 [ 134.365507] Tainted: [W]=WARN [
> > 134.368485] Hardware name: Renesas SMARC EVK version 2 based on
> > r9a09g047e57 (DT) [ 134.375961] pstate: 60400005 (nZCv daif +PAN -UAO
> > -TCO -DIT -SSBS BTYPE=--) [ 134.382935] pc :
> > __i2c_smbus_xfer+0x1e4/0x214 [ 134.387329] lr :
> > __i2c_smbus_xfer+0x1e4/0x214 [ 134.391717] sp : ffff800083f23860 [
> > 134.395040] x29: ffff800083f23860 x28: 0000000000000000 x27:
> > ffff800082ed5d60 [ 134.402226] x26: 0000001f4395fd74 x25:
> > 0000000000000007 x24: 0000000000000001 [ 134.409408] x23:
> > 0000000000000000 x22: 000000000000006f x21: ffff800083f23936 [
> > 134.416589] x20: ffff0000c090e140 x19: ffff0000c090e0d0 x18:
> > 0000000000000006 [ 134.423771] x17: 6f63657320313030 x16:
> > 2e30206465737061 x15: ffff800083f23280 [ 134.430953] x14:
> > 0000000000000000 x13: ffff800082b16ce8 x12: 0000000000000f09 [
> > 134.438134] x11: 0000000000000503 x10: ffff800082b6ece8 x9 :
> > ffff800082b16ce8 [ 134.445315] x8 : 00000000ffffefff x7 :
> > ffff800082b6ece8 x6 : 80000000fffff000 [ 134.452495] x5 : 0000000000000504 x4 : 0000000000000000
> x3 : 0000000000000000 [ 134.459672] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000c9ee9e80
> [ 134.466851] Call trace:
> > [ 134.469311] __i2c_smbus_xfer+0x1e4/0x214 (P) [ 134.473715]
> > i2c_smbus_xfer+0xbc/0x120 [ 134.477507]
> > i2c_smbus_read_byte_data+0x4c/0x84
> > [ 134.482077] isl1208_i2c_read_time+0x44/0x178 [rtc_isl1208] [
> > 134.487703] isl1208_rtc_read_time+0x14/0x20 [rtc_isl1208] [
> > 134.493226] __rtc_read_time+0x44/0x88 [ 134.497012]
> > rtc_read_time+0x3c/0x68 [ 134.500622] rtc_suspend+0x9c/0x170
> >
> > The warning is triggered because I2C transfers can still be attempted
> > while the controller is already suspended, due to inappropriate
> > ordering of the system sleep callbacks.
> >
> > If the controller is autosuspended, there is no way to wake it up once
> > runtime PM disabled (in suspend_late()). During system resume, the I2C
> > controller will be available only after runtime PM is re-enabled (in
> > resume_early()). However, this may be too late for some devices.
> >
> > Wake up the controller in the suspend() callback while runtime PM is
> > still enabled. The I2C controller will remain available until the
> > suspend_noirq() callback (pm_runtime_force_suspend()) is called.
> > During resume, the I2C controller can be restored by the
> > resume_noirq() callback (pm_runtime_force_resume()). Finally, the
> > resume() callback re-enables autosuspend. As a result, the I2C
> > controller can remain available until the system enters suspend_noirq() and from resume_noirq().
> >
> > Cc: stable@...r.kernel.org
> > Fixes: 53326135d0e0 ("i2c: riic: Add suspend/resume support")
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
Tested-by: Biju Das <biju.das.jz@...renesas.com>
Tested on RZ/G3E SMARC EVK by putting the display into deep sleep
and resume the display that has ADV bridge connected to it.
Cheers,
Biju
> > ---
> > v1->v2:
> > - Taking as reference commit:
> > 4262df2a69c3 ("i2c: imx-lpi2c: make controller available until the system
> > enters suspend_noirq() and from resume_noirq().") reworked the patch with
> > a similar approach. Updated commit body accordingly.
> >
> > drivers/i2c/busses/i2c-riic.c | 46
> > +++++++++++++++++++++++++++++------
> > 1 file changed, 39 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-riic.c
> > b/drivers/i2c/busses/i2c-riic.c index 3e8f126cb7f7..9e3595b3623e
> > 100644
> > --- a/drivers/i2c/busses/i2c-riic.c
> > +++ b/drivers/i2c/busses/i2c-riic.c
> > @@ -670,12 +670,39 @@ static const struct riic_of_data
> > riic_rz_t2h_info = {
> >
> > static int riic_i2c_suspend(struct device *dev) {
> > - struct riic_dev *riic = dev_get_drvdata(dev);
> > - int ret;
> > + /*
> > + * Some I2C devices may need the I2C controller to remain active
> > + * during resume_noirq() or suspend_noirq(). If the controller is
> > + * autosuspended, there is no way to wake it up once runtime PM is
> > + * disabled (in suspend_late()).
> > + *
> > + * During system resume, the I2C controller will be available only
> > + * after runtime PM is re-enabled (in resume_early()). However, this
> > + * may be too late for some devices.
> > + *
> > + * Wake up the controller in the suspend() callback while runtime PM
> > + * is still enabled. The I2C controller will remain available until
> > + * the suspend_noirq() callback (pm_runtime_force_suspend()) is
> > + * called. During resume, the I2C controller can be restored by the
> > + * resume_noirq() callback (pm_runtime_force_resume()).
> > + *
> > + * Finally, the resume() callback re-enables autosuspend, ensuring
> > + * the I2C controller remains available until the system enters
> > + * suspend_noirq() and from resume_noirq().
> > + */
> > + return pm_runtime_resume_and_get(dev); }
> >
> > - ret = pm_runtime_resume_and_get(dev);
> > - if (ret)
> > - return ret;
> > +static int riic_i2c_resume(struct device *dev) {
> > + pm_runtime_put_autosuspend(dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int riic_i2c_suspend_noirq(struct device *dev) {
> > + struct riic_dev *riic = dev_get_drvdata(dev);
> >
> > i2c_mark_adapter_suspended(&riic->adapter);
> >
> > @@ -683,12 +710,12 @@ static int riic_i2c_suspend(struct device *dev)
> > riic_clear_set_bit(riic, ICCR1_ICE, 0, RIIC_ICCR1);
> >
> > pm_runtime_mark_last_busy(dev);
> > - pm_runtime_put_sync(dev);
> > + pm_runtime_force_suspend(dev);
> >
> > return reset_control_assert(riic->rstc); }
> >
> > -static int riic_i2c_resume(struct device *dev)
> > +static int riic_i2c_resume_noirq(struct device *dev)
> > {
> > struct riic_dev *riic = dev_get_drvdata(dev);
> > int ret;
> > @@ -697,6 +724,10 @@ static int riic_i2c_resume(struct device *dev)
> > if (ret)
> > return ret;
> >
> > + ret = pm_runtime_force_resume(dev);
> > + if (ret)
> > + return ret;
> > +
> > ret = riic_init_hw(riic);
> > if (ret) {
> > /*
> > @@ -714,6 +745,7 @@ static int riic_i2c_resume(struct device *dev) }
> >
> > static const struct dev_pm_ops riic_i2c_pm_ops = {
> > + NOIRQ_SYSTEM_SLEEP_PM_OPS(riic_i2c_suspend_noirq,
> > +riic_i2c_resume_noirq)
> > SYSTEM_SLEEP_PM_OPS(riic_i2c_suspend, riic_i2c_resume) };
> >
> > --
> > 2.43.0
> >
Powered by blists - more mailing lists