linux-kernel - RE: [PATCH] i2c: riic: Move suspend handling to NOIRQ phase

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] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <TYCPR01MB11332BE48C9501DFC331D296886A8A@TYCPR01MB11332.jpnprd01.prod.outlook.com>
Date: Thu, 18 Dec 2025 08:34:07 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>, Claudiu.Beznea
	<claudiu.beznea@...on.dev>
CC: Tommaso Merciai <tomm.merciai@...il.com>,
	"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] i2c: riic: Move suspend handling to NOIRQ phase

Hi Tommaso,

> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> Sent: 18 December 2025 08:09
> Subject: Re: [PATCH] i2c: riic: Move suspend handling to NOIRQ phase
> 
> Hi Claudiu,
> Thanks for your review!
> 
> 
> On Wed, Dec 17, 2025 at 11:41:43AM +0200, Claudiu Beznea wrote:
> > Hi, Tommaso,
> >
> > On 12/12/25 13:58, 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.
> > >
> > > Fix this by moving the system sleep suspend/resume callbacks to the
> > > NOIRQ phase, ensuring the adapter is fully quiesced after late
> > > suspend and properly resumed before the early resume phase.
> > >
> > > To support NOIRQ resume, the hardware initialization path must not
> > > invoke runtime PM APIs. Split the init code so that the low-level
> > > hardware setup can be executed without pm_runtime_get/put(). This
> > > avoids violating the constraint introduced by commit 1e2ef05bb8cf
> > > ("PM: Limit race conditions between runtime PM and system sleep
> > > (v2)"), which forbids runtime PM calls during early resume.
> > >
> > > Cc: stable@...r.kernel.org
> > > Fixes: 53326135d0e0 ("i2c: riic: Add suspend/resume support")
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> > > ---
> > >  drivers/i2c/busses/i2c-riic.c | 65
> > > ++++++++++++++++++++++-------------
> > >  1 file changed, 41 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-riic.c
> > > b/drivers/i2c/busses/i2c-riic.c index 3e8f126cb7f7..9acc8936cdf7
> > > 100644
> > > --- a/drivers/i2c/busses/i2c-riic.c
> > > +++ b/drivers/i2c/busses/i2c-riic.c
> > > @@ -349,9 +349,8 @@ static const struct i2c_algorithm riic_algo = {
> > >  	.functionality = riic_func,
> > >  };
> > >
> >
> > Moving reset assert/de-assert and controller deinit/re-initialization
> > on runtime suspend/resume may increase the runtime suspend/resume
> > latency. In case of consecutive requests this may translate into
> > controller performance downgrade. If you keep it like this, you may
> > want to increase the default autosuspend delay, at least, to avoid
> > controller reconfiguration after each individual struct i2c_algorithm::xfer() call.
> 
> 
> We can set autosuspend delay to 250 ms.

Just a question, What is the delay in current driver? 

Cheers,
Biju