[PATCH] i2c: imx-lpi2c: release clocks on remove

[PATCH] i2c: imx-lpi2c: release clocks on remove

[Guangshuo Li]

lpi2c_imx_probe() prepares and enables the controller clocks before
enabling runtime PM. After probe succeeds, clock management is handed
over to the runtime PM callbacks.

However, lpi2c_imx_remove() only disables runtime PM and autosuspend.
It does not ensure that the device is active before teardown or drop
the clock references acquired during probe.

For platforms which do not require clock prepare/unprepare in the
runtime PM callbacks, runtime suspend only disables the clocks, leaving
the prepare references held for the lifetime of the driver. Since the
remove path never unprepares them, these references are leaked when the
driver is unbound.

For platforms which prepare and unprepare clocks during runtime PM, an
unbind while the device is still runtime active can similarly leave
the clock prepare and enable references outstanding.

Resume the device before disabling runtime PM so that the clocks are in
a known enabled and prepared state. Then drop the temporary runtime PM
reference and disable and unprepare the clocks during final teardown.
Mark the device suspended after the hardware resources have been
released.

This issue was found by manual code inspection.

Fixes: 13d6eb20fc79 ("i2c: imx-lpi2c: add runtime pm support")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index e1a4338bc51e..481950004b2a 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -1604,8 +1604,13 @@ static void lpi2c_imx_remove(struct platform_device *pdev)
 
 	i2c_del_adapter(&lpi2c_imx->adapter);
 
+	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
+
+	clk_bulk_disable_unprepare(lpi2c_imx->num_clks, lpi2c_imx->clks);
+	pm_runtime_set_suspended(&pdev->dev);
 }
 
 static int __maybe_unused lpi2c_runtime_suspend(struct device *dev)
-- 
2.43.0

Re: [PATCH] i2c: imx-lpi2c: release clocks on remove

[sashiko-bot]

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

New issues:
- [High] Manually disabling hardware clocks while the devm-managed interrupt is still active creates a race condition leading to a system crash on spurious interrupts.
- [Medium] Ignored return value of pm_runtime_get_sync() leads to unbalanced clock disable and reference count underflow if device resume fails.

Pre-existing issues:
- [High] ISR unconditionally accesses hardware registers without ensuring the device is runtime active.
--

commit aa70bea937f841715508dc37222d676d4d04042f
Author: Guangshuo Li <lgs201920130244@gmail.com>
i2c: imx-lpi2c: release clocks on remove

This commit modifies lpi2c_imx_remove() to resume the device before disabling
runtime PM, ensuring the clocks are in a known enabled and prepared state. It
then drops the temporary runtime PM reference and explicitly disables and
unprepares the clocks during final teardown.

> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 49aabef7f115c..7f4bc98325853 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -1613,8 +1613,13 @@ static void lpi2c_imx_remove(struct platform_device *pdev)
>  
>  	i2c_del_adapter(&lpi2c_imx->adapter);
>  
> +	pm_runtime_get_sync(&pdev->dev);

[Severity: Medium]
If this fails and returns a negative error code, is it safe to proceed? The
clocks would remain disabled, which might cause an enable count underflow
when clk_bulk_disable_unprepare() is unconditionally called later in this
function.

>  	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
> +
> +	clk_bulk_disable_unprepare(lpi2c_imx->num_clks, lpi2c_imx->clks);

[Severity: High]
Does disabling the clocks here create a race condition with the devm managed
interrupt? Because the interrupt handler was registered with
devm_request_irq(), it is only automatically freed after lpi2c_imx_remove()
returns. If a spurious or delayed interrupt fires during this window,
lpi2c_imx_isr() will attempt to read hardware registers with clocks disabled,
which can cause a fatal system crash.

[Severity: High]
This is a pre-existing issue, but does lpi2c_imx_isr() also need to check if
the device is runtime active? The ISR unconditionally reads hardware registers:

drivers/i2c/busses/i2c-imx-lpi2c.c:lpi2c_imx_isr() {
    ...
    u32 scr = readl(lpi2c_imx->base + LPI2C_SCR);
    ...
}

If the device is suspended and a spurious interrupt fires, this could also
lead to a system crash. Should the ISR use pm_runtime_get_if_active()
before accessing hardware registers?

> +	pm_runtime_set_suspended(&pdev->dev);
>  }

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

Re: [PATCH] i2c: imx-lpi2c: release clocks on remove

[Krzysztof Kozlowski]

On 14/09/2026 11:07, Guangshuo Li wrote:
> lpi2c_imx_probe() prepares and enables the controller clocks before
> enabling runtime PM. After probe succeeds, clock management is handed
> over to the runtime PM callbacks.
> 
> However, lpi2c_imx_remove() only disables runtime PM and autosuspend.
> It does not ensure that the device is active before teardown or drop
> the clock references acquired during probe.
> 
> For platforms which do not require clock prepare/unprepare in the
> runtime PM callbacks, runtime suspend only disables the clocks, leaving
> the prepare references held for the lifetime of the driver. Since the
> remove path never unprepares them, these references are leaked when the
> driver is unbound.
> 
> For platforms which prepare and unprepare clocks during runtime PM, an
> unbind while the device is still runtime active can similarly leave
> the clock prepare and enable references outstanding.
> 
> Resume the device before disabling runtime PM so that the clocks are in
> a known enabled and prepared state. Then drop the temporary runtime PM
> reference and disable and unprepare the clocks during final teardown.
> Mark the device suspended after the hardware resources have been
> released.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 13d6eb20fc79 ("i2c: imx-lpi2c: add runtime pm support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>


Nothing improved, you again spammed with multiple independent patches so
maintainers would need to respond TO EACH one instead of to the thread.

Best regards,
Krzysztof