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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ