| 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
| ||
|
Message-ID: <aWUgEeouTeow1pvB@tom-desktop>
Date: Mon, 12 Jan 2026 17:23:45 +0100
From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
To: tomm.merciai@...il.com
Cc: linux-renesas-soc@...r.kernel.org, biju.das.jz@...renesas.com,
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-kernel@...r.kernel.org,
stable@...r.kernel.org
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>
> ---
> 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