Re: [PATCH] i2c: riic: Move suspend handling to NOIRQ phase

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Andi,

Thanks for your review!

On Wed, Dec 17, 2025 at 01:10:27AM +0100, Andi Shyti wrote:
> Hi Tommaso,
> 
> On Fri, Dec 12, 2025 at 12:58:57PM +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.
> > 
> > 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@vger.kernel.org
> > Fixes: 53326135d0e0 ("i2c: riic: Add suspend/resume support")
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> can I have an ack from Chris here?
> 
> ...
> 
> > +static int riic_i2c_suspend(struct device *dev)
> > +{
> > +	struct riic_dev *riic = dev_get_drvdata(dev);
> > +
> > +	i2c_mark_adapter_suspended(&riic->adapter);
> > +
> > +	return pm_runtime_force_suspend(dev);
> 
> We should perhaps swap i2c_mark_adapter_suspended() and
> pm_runtime_force_suspend()?

Please correct me if I'm wrong but I'm seeing similar pattern into:

 - drivers/i2c/busses/i2c-sprd.c -> sprd_i2c_suspend_noirq()

What do you think?

Thanks & Regards,
Tommaso

> 
> Andi
> 
> > +}