diff mbox

i2c: rcar: fix resume by always initializing registers before transfer

Message ID 87o9vtl19p.wl%kuninori.morimoto.gx@renesas.com
State RFC
Headers show

Commit Message

Kuninori Morimoto April 18, 2017, 11:59 p.m. UTC 
Hi Wolfram

> Resume failed because of uninitialized registers. Instead of adding a
> resume callback, we simply initialize registers before every transfer.
> This lightweight change is more robust and will keep us safe if we ever
> need support for power domains or dynamic frequency changes.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Morimoto-san: you convinced me that putting all register settings to one place
> is a nice thing to do. Here is the patch for that. However, having
> pm_runtime_get/put in one place won't work, I am afraid. For I2C slave or
> multi-master support, we need permanent-on handling. So, I think the current
> pm_runtime handling is good in that regard. Can you agree?

Thanks.
I understand about I2C slave / multi-master case of pm_runtime_get/put.
But in probe case (after rcar_i2c_init() move), we can't do like below ?
this is more clear code for me.

-------------
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Wolfram Sang April 19, 2017, 6:59 a.m. UTC | #1 
Morimoto-san,

> I understand about I2C slave / multi-master case of pm_runtime_get/put.
> But in probe case (after rcar_i2c_init() move), we can't do like below ?
> this is more clear code for me.
> 
> -------------
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 26f2ff2..db7f4d1 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -855,7 +855,6 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>  	priv->dma_rx = priv->dma_tx = ERR_PTR(-EPROBE_DEFER);
>  
>  	pm_runtime_enable(dev);
> -	pm_runtime_get_sync(dev);
>  	ret = rcar_i2c_clock_calculate(priv, &i2c_t);

rcar_i2c_clock_calculate() uses clk_get_rate, so clk must be active.

Regards,

   Wolfram
Kuninori Morimoto April 19, 2017, 7:29 a.m. UTC | #2 
Hi Wolfram

> > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> > index 26f2ff2..db7f4d1 100644
> > --- a/drivers/i2c/busses/i2c-rcar.c
> > +++ b/drivers/i2c/busses/i2c-rcar.c
> > @@ -855,7 +855,6 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> >  	priv->dma_rx = priv->dma_tx = ERR_PTR(-EPROBE_DEFER);
> >  
> >  	pm_runtime_enable(dev);
> > -	pm_runtime_get_sync(dev);
> >  	ret = rcar_i2c_clock_calculate(priv, &i2c_t);
> 
> rcar_i2c_clock_calculate() uses clk_get_rate, so clk must be active.

Ahh, OK, I understand.
Then,

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 26f2ff2..db7f4d1 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -855,7 +855,6 @@  static int rcar_i2c_probe(struct platform_device *pdev)
 	priv->dma_rx = priv->dma_tx = ERR_PTR(-EPROBE_DEFER);
 
 	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
 	ret = rcar_i2c_clock_calculate(priv, &i2c_t);
 	if (ret < 0)
 		goto out_pm_put;
@@ -863,11 +862,10 @@  static int rcar_i2c_probe(struct platform_device *pdev)
 	rcar_i2c_init(priv);
 
 	/* Don't suspend when multi-master to keep arbitration working */
-	if (of_property_read_bool(dev->of_node, "multi-master"))
+	if (of_property_read_bool(dev->of_node, "multi-master")) {
 		priv->flags |= ID_P_PM_BLOCKED;
-	else
-		pm_runtime_put(dev);
-
+		pm_runtime_get_sync(dev);
+	}
 
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);