| 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:
<TYCPR01MB11332073ADC4063F21591554386A8A@TYCPR01MB11332.jpnprd01.prod.outlook.com>
Date: Thu, 18 Dec 2025 09:23:19 +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] i2c: riic: Move suspend handling to NOIRQ phase
Hi Tommaso,
> -----Original Message-----
> From: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> Sent: 12 December 2025 11:59
> Subject: [PATCH] i2c: riic: Move suspend handling to NOIRQ phase
>
> 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,
> };
>
> -static int riic_init_hw(struct riic_dev *riic)
> +static int __riic_init_hw(struct riic_dev *riic)
> {
> - int ret;
> unsigned long rate;
> unsigned long ns_per_tick;
> int total_ticks, cks, brl, brh;
> @@ -431,10 +430,6 @@ static int riic_init_hw(struct riic_dev *riic)
> rate / total_ticks, ((brl + 3) * 100) / (brl + brh + 6),
> t->scl_fall_ns / ns_per_tick, t->scl_rise_ns / ns_per_tick, cks, brl, brh);
>
> - ret = pm_runtime_resume_and_get(dev);
> - if (ret)
> - return ret;
> -
> /* Changing the order of accessing IICRST and ICE may break things! */
> riic_writeb(riic, ICCR1_IICRST | ICCR1_SOWP, RIIC_ICCR1);
> riic_clear_set_bit(riic, 0, ICCR1_ICE, RIIC_ICCR1); @@ -451,10 +446,25 @@ static int
> riic_init_hw(struct riic_dev *riic)
>
> riic_clear_set_bit(riic, ICCR1_IICRST, 0, RIIC_ICCR1);
>
> - pm_runtime_put_autosuspend(dev);
> return 0;
> }
>
> +static int riic_init_hw(struct riic_dev *riic) {
> + struct device *dev = riic->adapter.dev.parent;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
> +
> + ret = __riic_init_hw(riic);
> +
> + pm_runtime_put_autosuspend(dev);
> +
> + return ret;
> +}
> +
> static int riic_get_scl(struct i2c_adapter *adap) {
> struct riic_dev *riic = i2c_get_adapdata(adap); @@ -572,6 +582,8 @@ static int
> riic_i2c_probe(struct platform_device *pdev)
>
> i2c_parse_fw_timings(dev, &riic->i2c_t, true);
>
> + platform_set_drvdata(pdev, riic);
> +
> /* Default 0 to save power. Can be overridden via sysfs for lower latency. */
> pm_runtime_set_autosuspend_delay(dev, 0);
> pm_runtime_use_autosuspend(dev);
> @@ -585,8 +597,6 @@ static int riic_i2c_probe(struct platform_device *pdev)
> if (ret)
> goto out;
>
> - platform_set_drvdata(pdev, riic);
> -
> dev_info(dev, "registered with %dHz bus speed\n", riic->i2c_t.bus_freq_hz);
> return 0;
>
> @@ -668,27 +678,17 @@ static const struct riic_of_data riic_rz_t2h_info = {
> .num_irqs = ARRAY_SIZE(riic_rzt2h_irqs), };
>
> -static int riic_i2c_suspend(struct device *dev)
> +static int riic_i2c_runtime_suspend(struct device *dev)
> {
> struct riic_dev *riic = dev_get_drvdata(dev);
> - int ret;
> -
> - ret = pm_runtime_resume_and_get(dev);
> - if (ret)
> - return ret;
> -
> - i2c_mark_adapter_suspended(&riic->adapter);
>
> /* Disable output on SDA, SCL pins. */
> riic_clear_set_bit(riic, ICCR1_ICE, 0, RIIC_ICCR1);
>
> - pm_runtime_mark_last_busy(dev);
> - pm_runtime_put_sync(dev);
> -
> return reset_control_assert(riic->rstc); }
>
> -static int riic_i2c_resume(struct device *dev)
> +static int riic_i2c_runtime_resume(struct device *dev)
> {
> struct riic_dev *riic = dev_get_drvdata(dev);
> int ret;
> @@ -697,7 +697,7 @@ static int riic_i2c_resume(struct device *dev)
> if (ret)
> return ret;
>
> - ret = riic_init_hw(riic);
> + ret = __riic_init_hw(riic);
> if (ret) {
> /*
> * In case this happens there is no way to recover from this @@ -708,13 +708,30 @@ static
> int riic_i2c_resume(struct device *dev)
> return ret;
> }
>
> + return 0;
> +}
> +
> +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);
> +}
> +
> +static int riic_i2c_resume(struct device *dev) {
> + struct riic_dev *riic = dev_get_drvdata(dev);
> +
> i2c_mark_adapter_resumed(&riic->adapter);
Is it not possible reconfigure i2c controller here like [1]
So that we can avoid reinitializing the i2ccontroller after every transfer (ie, rpm)
[1]
https://elixir.bootlin.com/linux/v6.18-rc7/source/drivers/i2c/busses/i2c-tegra.c#L1965
Cheers,
Biju
Powered by blists - more mailing lists