diff mbox series

[v2,2/2] i2c: imx: avoid taking clk_prepare mutex in PM callbacks

Message ID 20180308132518.2161-2-l.stach@pengutronix.de
State Accepted
Headers show
Series None | expand

Commit Message

Lucas Stach March 8, 2018, 1:25 p.m. UTC
This is unsafe, as the runtime PM callbacks are called from the PM
workqueue, so this may deadlock when handling an i2c attached clock,
which may already hold the clk_prepare mutex from another context.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/i2c/busses/i2c-imx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Philipp Zabel March 27, 2018, 9:18 a.m. UTC | #1
On Thu, 2018-03-08 at 14:25 +0100, Lucas Stach wrote:
> This is unsafe, as the runtime PM callbacks are called from the PM
> workqueue, so this may deadlock when handling an i2c attached clock,
> which may already hold the clk_prepare mutex from another context.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

> ---
>  drivers/i2c/busses/i2c-imx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index c9632b2f0eaa..d7267dd9c7bf 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1226,7 +1226,7 @@ static int i2c_imx_runtime_suspend(struct device *dev)
>  {
>  	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
>  
> -	clk_disable_unprepare(i2c_imx->clk);
> +	clk_disable(i2c_imx->clk);
>  
>  	return 0;
>  }
> @@ -1236,7 +1236,7 @@ static int i2c_imx_runtime_resume(struct device *dev)
>  	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
>  	int ret;
>  
> -	ret = clk_prepare_enable(i2c_imx->clk);
> +	ret = clk_enable(i2c_imx->clk);
>  	if (ret)
>  		dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);
>
Wolfram Sang April 3, 2018, 1:29 p.m. UTC | #2
On Thu, Mar 08, 2018 at 02:25:18PM +0100, Lucas Stach wrote:
> This is unsafe, as the runtime PM callbacks are called from the PM
> workqueue, so this may deadlock when handling an i2c attached clock,
> which may already hold the clk_prepare mutex from another context.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Applied to for-next, thanks!

stable tag?
Lucas Stach April 10, 2018, 3:42 p.m. UTC | #3
Hi Wolfram,

Am Dienstag, den 03.04.2018, 15:29 +0200 schrieb Wolfram Sang:
> On Thu, Mar 08, 2018 at 02:25:18PM +0100, Lucas Stach wrote:
> > This is unsafe, as the runtime PM callbacks are called from the PM
> > workqueue, so this may deadlock when handling an i2c attached
> > clock,
> > which may already hold the clk_prepare mutex from another context.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> 
> Applied to for-next, thanks!
> 
> stable tag?

Yes, even though the number of affected systems is low it might make
sense:

Fixes: 588eb93ea49f (i2c: imx: add runtime pm support to improve the
performance)

Regards,
Lucas
Wolfram Sang April 10, 2018, 3:55 p.m. UTC | #4
> > stable tag?
> 
> Yes, even though the number of affected systems is low it might make
> sense:
> 
> Fixes: 588eb93ea49f (i2c: imx: add runtime pm support to improve the
> performance)

Thanks, but it is in Linus' tree already. So, whoever wants it
backported needs to notify stable manually.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index c9632b2f0eaa..d7267dd9c7bf 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -1226,7 +1226,7 @@  static int i2c_imx_runtime_suspend(struct device *dev)
 {
 	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
 
-	clk_disable_unprepare(i2c_imx->clk);
+	clk_disable(i2c_imx->clk);
 
 	return 0;
 }
@@ -1236,7 +1236,7 @@  static int i2c_imx_runtime_resume(struct device *dev)
 	struct imx_i2c_struct *i2c_imx = dev_get_drvdata(dev);
 	int ret;
 
-	ret = clk_prepare_enable(i2c_imx->clk);
+	ret = clk_enable(i2c_imx->clk);
 	if (ret)
 		dev_err(dev, "can't enable I2C clock, ret=%d\n", ret);