Skip to content

Commit e383f09

Browse files
Tommaso MerciaiWolfram Sang
authored andcommitted
i2c: riic: Move suspend handling to NOIRQ phase
Commit 5332613 ("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@vger.kernel.org Fixes: 5332613 ("i2c: riic: Add suspend/resume support") Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Tested-by: Biju Das <biju.das.jz@bp.renesas.com> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
1 parent 747d246 commit e383f09

1 file changed

Lines changed: 39 additions & 7 deletions

File tree

drivers/i2c/busses/i2c-riic.c

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -670,25 +670,52 @@ static const struct riic_of_data riic_rz_t2h_info = {
670670

671671
static int riic_i2c_suspend(struct device *dev)
672672
{
673-
struct riic_dev *riic = dev_get_drvdata(dev);
674-
int ret;
673+
/*
674+
* Some I2C devices may need the I2C controller to remain active
675+
* during resume_noirq() or suspend_noirq(). If the controller is
676+
* autosuspended, there is no way to wake it up once runtime PM is
677+
* disabled (in suspend_late()).
678+
*
679+
* During system resume, the I2C controller will be available only
680+
* after runtime PM is re-enabled (in resume_early()). However, this
681+
* may be too late for some devices.
682+
*
683+
* Wake up the controller in the suspend() callback while runtime PM
684+
* is still enabled. The I2C controller will remain available until
685+
* the suspend_noirq() callback (pm_runtime_force_suspend()) is
686+
* called. During resume, the I2C controller can be restored by the
687+
* resume_noirq() callback (pm_runtime_force_resume()).
688+
*
689+
* Finally, the resume() callback re-enables autosuspend, ensuring
690+
* the I2C controller remains available until the system enters
691+
* suspend_noirq() and from resume_noirq().
692+
*/
693+
return pm_runtime_resume_and_get(dev);
694+
}
675695

676-
ret = pm_runtime_resume_and_get(dev);
677-
if (ret)
678-
return ret;
696+
static int riic_i2c_resume(struct device *dev)
697+
{
698+
pm_runtime_put_autosuspend(dev);
699+
700+
return 0;
701+
}
702+
703+
static int riic_i2c_suspend_noirq(struct device *dev)
704+
{
705+
struct riic_dev *riic = dev_get_drvdata(dev);
679706

680707
i2c_mark_adapter_suspended(&riic->adapter);
681708

682709
/* Disable output on SDA, SCL pins. */
683710
riic_clear_set_bit(riic, ICCR1_ICE, 0, RIIC_ICCR1);
684711

685712
pm_runtime_mark_last_busy(dev);
686-
pm_runtime_put_sync(dev);
713+
pm_runtime_force_suspend(dev);
687714

688715
return reset_control_assert(riic->rstc);
689716
}
690717

691-
static int riic_i2c_resume(struct device *dev)
718+
static int riic_i2c_resume_noirq(struct device *dev)
692719
{
693720
struct riic_dev *riic = dev_get_drvdata(dev);
694721
int ret;
@@ -697,6 +724,10 @@ static int riic_i2c_resume(struct device *dev)
697724
if (ret)
698725
return ret;
699726

727+
ret = pm_runtime_force_resume(dev);
728+
if (ret)
729+
return ret;
730+
700731
ret = riic_init_hw(riic);
701732
if (ret) {
702733
/*
@@ -714,6 +745,7 @@ static int riic_i2c_resume(struct device *dev)
714745
}
715746

716747
static const struct dev_pm_ops riic_i2c_pm_ops = {
748+
NOIRQ_SYSTEM_SLEEP_PM_OPS(riic_i2c_suspend_noirq, riic_i2c_resume_noirq)
717749
SYSTEM_SLEEP_PM_OPS(riic_i2c_suspend, riic_i2c_resume)
718750
};
719751

0 commit comments

Comments
 (0)