All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: imx: disable autosuspend on remove
@ 2026-09-14  9:15 Guangshuo Li
  2026-09-14  9:27 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Guangshuo Li @ 2026-09-14  9:15 UTC (permalink / raw)
  To: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Uwe Kleine-König, Gao Pan,
	Fugang Duan, Wolfram Sang, linux-i2c, imx, linux-arm-kernel,
	linux-kernel
  Cc: Guangshuo Li, stable

i2c_imx_probe() enables runtime PM autosuspend with
pm_runtime_use_autosuspend(). The probe error path correctly undoes
this setting with pm_runtime_dont_use_autosuspend(), but the normal
remove path only disables runtime PM.

The runtime PM API requires pm_runtime_use_autosuspend() to be undone
with pm_runtime_dont_use_autosuspend() at driver exit unless runtime PM
was enabled with devm_pm_runtime_enable(). Leaving the autosuspend flag
set therefore leaves the runtime PM state incompletely cleaned up after
the driver is unbound.

Add the missing pm_runtime_dont_use_autosuspend() call to the remove
path.

This issue was found by manual code inspection.

Fixes: 588eb93ea49f ("i2c: imx: add runtime pm support to improve the performance")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/i2c/busses/i2c-imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 19ec056b00af..4dc3df6a41a4 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1920,6 +1920,7 @@ static void i2c_imx_remove(struct platform_device *pdev)
 
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 }
 
 static int i2c_imx_runtime_suspend(struct device *dev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: imx: disable autosuspend on remove
  2026-09-14  9:15 [PATCH] i2c: imx: disable autosuspend on remove Guangshuo Li
@ 2026-09-14  9:27 ` sashiko-bot
  2026-09-14 14:02 ` Frank Li
  2026-09-16  5:28 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-14  9:27 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: imx, Frank.Li

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The shared interrupt handler accesses hardware registers without checking if the device is active, causing a potential system crash.
- [High] DMA channels are permanently leaked if `i2c_add_numbered_adapter()` fails during probe.
- [High] Race condition in `i2c_imx_unreg_slave()` allows `slave_timer` to be re-armed after cancellation, leading to a use-after-free.
--

commit 37a8cb58f3db8dddb256f2da1347377502c8b5b3
Author: Guangshuo Li <lgs201920130244@gmail.com>
Subject: i2c: imx: disable autosuspend on remove

This commit adds a missing pm_runtime_dont_use_autosuspend() call in the
remove path to properly clean up runtime PM state. This ensures that the
autosuspend flag is cleared when the driver is unbound.

> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 19ec056b00afe..4dc3df6a41a4f 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1920,6 +1920,7 @@ static void i2c_imx_remove(struct platform_device *pdev)
>  
>  	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  }
>  
>  static int i2c_imx_runtime_suspend(struct device *dev)

[Severity: High]
This isn't a bug introduced by this patch, but does this runtime suspend
callback need to call synchronize_irq() before the clock is disabled?

Since the driver requests the IRQ with IRQF_SHARED, if another device on the
shared line triggers an interrupt while the I2C device is runtime suspended,
i2c_imx_isr() executes and reads the IMX_I2C_I2SR register:

drivers/i2c/busses/i2c-imx.c:i2c_imx_isr() {
    [ ... ]
    status = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
    [ ... ]
}

Could this unconditional read of hardware registers while the controller's
clock is disabled potentially cause a fatal external abort?

[Severity: High]
This isn't a bug introduced by this patch, but is there a potential DMA
channel resource leak in the probe function?

In i2c_imx_probe(), i2c_imx_dma_request() successfully requests TX and RX DMA
channels. If the subsequent call to i2c_add_numbered_adapter() fails:

drivers/i2c/busses/i2c-imx.c:i2c_imx_probe() {
    [ ... ]
    ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
    if (ret < 0)
        goto clk_notifier_unregister;
    [ ... ]
}

This jump appears to bypass calling i2c_imx_dma_free(). Does this leak the
channels permanently since they are not devm-managed?

[Severity: High]
This isn't a bug introduced by this patch, but is there a potential race
condition in i2c_imx_unreg_slave() that could lead to a use-after-free?

The function cancels the timer and clears the slave pointer without holding
the slave lock:

drivers/i2c/busses/i2c-imx.c:i2c_imx_unreg_slave() {
    [ ... ]
    hrtimer_cancel(&i2c_imx->slave_timer);
    i2c_imx->slave = NULL;
    [ ... ]
}

If a hardware interrupt executes concurrently between these operations, could
i2c_imx_isr() acquire the lock, see i2c_imx->slave as non-NULL, and invoke
i2c_imx_slave_handle() which restarts the timer via hrtimer_restart()? Since
the device is subsequently freed, would this leave a pending timer referencing
freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914091544.1667137-1-lgs201920130244@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: imx: disable autosuspend on remove
  2026-09-14  9:15 [PATCH] i2c: imx: disable autosuspend on remove Guangshuo Li
  2026-09-14  9:27 ` sashiko-bot
@ 2026-09-14 14:02 ` Frank Li
  2026-09-16  5:28 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-09-14 14:02 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Oleksij Rempel, Pengutronix Kernel Team, Andi Shyti, Frank Li,
	Sascha Hauer, Fabio Estevam, Uwe Kleine-König, Gao Pan,
	Fugang Duan, Wolfram Sang, linux-i2c, imx, linux-arm-kernel,
	linux-kernel, stable

On Mon, Sep 14, 2026 at 05:15:44PM +0800, Guangshuo Li wrote:
> i2c_imx_probe() enables runtime PM autosuspend with
> pm_runtime_use_autosuspend(). The probe error path correctly undoes
> this setting with pm_runtime_dont_use_autosuspend(), but the normal
> remove path only disables runtime PM.
>
> The runtime PM API requires pm_runtime_use_autosuspend() to be undone
> with pm_runtime_dont_use_autosuspend() at driver exit unless runtime PM
> was enabled with devm_pm_runtime_enable(). Leaving the autosuspend flag
> set therefore leaves the runtime PM state incompletely cleaned up after
> the driver is unbound.
>
> Add the missing pm_runtime_dont_use_autosuspend() call to the remove
> path.
>
> This issue was found by manual code inspection.
>
> Fixes: 588eb93ea49f ("i2c: imx: add runtime pm support to improve the performance")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/i2c/busses/i2c-imx.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 19ec056b00af..4dc3df6a41a4 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1920,6 +1920,7 @@ static void i2c_imx_remove(struct platform_device *pdev)
>
>  	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  }
>
>  static int i2c_imx_runtime_suspend(struct device *dev)
> --
> 2.43.0
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] i2c: imx: disable autosuspend on remove
  2026-09-14  9:15 [PATCH] i2c: imx: disable autosuspend on remove Guangshuo Li
  2026-09-14  9:27 ` sashiko-bot
  2026-09-14 14:02 ` Frank Li
@ 2026-09-16  5:28 ` Andi Shyti
  2 siblings, 0 replies; 4+ messages in thread
From: Andi Shyti @ 2026-09-16  5:28 UTC (permalink / raw)
  To: Guangshuo Li
  Cc: Oleksij Rempel, Pengutronix Kernel Team, Frank Li, Sascha Hauer,
	Fabio Estevam, Uwe Kleine-König, Gao Pan, Fugang Duan,
	Wolfram Sang, linux-i2c, imx, linux-arm-kernel, linux-kernel,
	stable

Hi Guangshuo,

On Mon, Sep 14, 2026 at 05:15:44PM +0800, Guangshuo Li wrote:
> i2c_imx_probe() enables runtime PM autosuspend with
> pm_runtime_use_autosuspend(). The probe error path correctly undoes
> this setting with pm_runtime_dont_use_autosuspend(), but the normal
> remove path only disables runtime PM.
> 
> The runtime PM API requires pm_runtime_use_autosuspend() to be undone
> with pm_runtime_dont_use_autosuspend() at driver exit unless runtime PM
> was enabled with devm_pm_runtime_enable(). Leaving the autosuspend flag
> set therefore leaves the runtime PM state incompletely cleaned up after
> the driver is unbound.
> 
> Add the missing pm_runtime_dont_use_autosuspend() call to the remove
> path.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 588eb93ea49f ("i2c: imx: add runtime pm support to improve the performance")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

merged to i2c/i2c-fixes.

I applied this patch only because someone spent some time to
review it.

Please, follow Krzysztof advice. group all the patches together,
version them and add a changelog.

Andi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-16  5:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  9:15 [PATCH] i2c: imx: disable autosuspend on remove Guangshuo Li
2026-09-14  9:27 ` sashiko-bot
2026-09-14 14:02 ` Frank Li
2026-09-16  5:28 ` Andi Shyti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.