diff mbox series

i2c: i2c-s3c2410: fix runtime PM cleanup

Message ID tencent_692077742B9CE600137EC92B0FEA258A8C09@qq.com
State Under Review
Delegated to: Andi Shyti
Headers show
Series i2c: i2c-s3c2410: fix runtime PM cleanup | expand

Commit Message

Jiawen Liu Aug. 18, 2026, 1:11 p.m. UTC
Keep clock enabled until adapter teardown.

The remove path disables the controller clock before unregistering the
I2C adapter, so adapter teardown can run after the hardware clock has
already been turned off.

Move I2C teardown before clock release (pm_runtime_disable,
clk_unprepare) in s3c24xx_i2c_remove.

Signed-off-by: jiawen <1298662399@qq.com>
---

Comments

Andi Shyti Aug. 18, 2026, 2:52 p.m. UTC | #1
Hi Jiawen,

On Tue, Aug 18, 2026 at 05:11:57PM +0400, Jiawen Liu wrote:
> Keep clock enabled until adapter teardown.
> 
> The remove path disables the controller clock before unregistering the
> I2C adapter, so adapter teardown can run after the hardware clock has
> already been turned off.
> 
> Move I2C teardown before clock release (pm_runtime_disable,
> clk_unprepare) in s3c24xx_i2c_remove.

This last paragraph is actually useless. Perhaps you can write:

"
The remove path disables the controller clock before
unregistering the I2C adapter, so adapter teardown can run after
the hardware clock has been turned off.

Fix the ordering in s3c24xx_i2c_remove() by calling
i2c_del_adapter() before pm_runtime_disable(), keeping the clock
enabled until adapter teardown is complete.
"

This way we also mention that the ordering between PM runtime and
i2c_del_adapter() has also changed.

Does it work for you?

> Signed-off-by: jiawen <1298662399@qq.com>

Please, next time write your full name, not "jiawen", but "Jiawen
Liu".

Besides this can also be considered as a fix and needs to add:

Fixes: 34e81ad5f0b6 ("i2c: s3c2410: fix ABBA deadlock by keeping clock prepared")
Cc: <stable@vger.kernel.org> # v3.19+

No need to resend, unless there are other reviews. Please just
let me know whether the changs I am suggesting work for you.

Thanks,
Andi

> ---
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -1136,11 +1136,9 @@
>  {
>  	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
>  
> +	i2c_del_adapter(&i2c->adap);
> +	pm_runtime_disable(&pdev->dev);
>  	clk_unprepare(i2c->clk);
> -
> -	pm_runtime_disable(&pdev->dev);
> -
> -	i2c_del_adapter(&i2c->adap);
>  }
>  
>  static int s3c24xx_i2c_suspend_noirq(struct device *dev)
>
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1136,11 +1136,9 @@ 
 {
 	struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
 
+	i2c_del_adapter(&i2c->adap);
+	pm_runtime_disable(&pdev->dev);
 	clk_unprepare(i2c->clk);
-
-	pm_runtime_disable(&pdev->dev);
-
-	i2c_del_adapter(&i2c->adap);
 }
 
 static int s3c24xx_i2c_suspend_noirq(struct device *dev)