[RFC] i2c: rcar: reinit hardware on resume

[RFC] i2c: rcar: reinit hardware on resume

[Wolfram Sang]

On resume, reinit hardware to ensure state machine and clock settings
are proper. This code assumes:

* bus clock is a fixed value, so no need to recalculate timings after probe
* handling RuntimePM is not needed, because register content is kept
  when disabling the IP core clock

Tested with r8a7796 (M3-W) and r8a7790 (H2).

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

@Geert: do you agree to my assumptions?

 drivers/i2c/busses/i2c-rcar.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 26f2ff22e97e55..7010c77df11daa 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -907,10 +907,27 @@ static int rcar_i2c_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int rcar_i2c_resume(struct device *dev)
+{
+	struct rcar_i2c_priv *priv = dev_get_drvdata(dev);
+
+	rcar_i2c_init(priv);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(rcar_i2c_pm, NULL, rcar_i2c_resume);
+#define RCAR_I2C_PM	(&rcar_i2c_pm)
+#else
+#define RCAR_I2C_PM	NULL
+#endif
+
 static struct platform_driver rcar_i2c_driver = {
 	.driver	= {
 		.name	= "i2c-rcar",
 		.of_match_table = rcar_i2c_dt_ids,
+		.pm	= RCAR_I2C_PM,
 	},
 	.probe		= rcar_i2c_probe,
 	.remove		= rcar_i2c_remove,
-- 
2.11.0

Re: [RFC] i2c: rcar: reinit hardware on resume

[Kuninori Morimoto]

Hi Wolfram

> On resume, reinit hardware to ensure state machine and clock settings
> are proper. This code assumes:
> 
> * bus clock is a fixed value, so no need to recalculate timings after probe
> * handling RuntimePM is not needed, because register content is kept
>   when disabling the IP core clock
> 
> Tested with r8a7796 (M3-W) and r8a7790 (H2).
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
(snip)
> +static int rcar_i2c_resume(struct device *dev)
> +{
> +	struct rcar_i2c_priv *priv = dev_get_drvdata(dev);
> +
> +	rcar_i2c_init(priv);
> +
> +	return 0;
> +}

Current i2c-rcar driver probe is doing

static int rcar_i2c_probe(xxx)
{
	...
	pm_runtime_get_sync(dev);
	...
	rcar_i2c_init(priv);
	...
	pm_runtime_put(dev);
	...
}

Thus, just calling rcar_i2c_init() is not enough ?

Current driver is calling pm_runtime_get/put probe timing,
transfer timing, and more for register settings purpose.
But in my opinion, all register settings should be merged in one please
(= transfer function) and call pm_runtime_get/put there only.
R-Car I2C is not related, but I2C power domain might be
power OFF if SoC has power domain control feature (SH-Mobile has it).
In such case, register value which was set on probe timing doesn't
exist when transfer timing.
This can be increasing patch or something like that.

Best regards
---
Kuninori Morimoto

Re: [RFC] i2c: rcar: reinit hardware on resume

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 554 bytes --]


> Thus, just calling rcar_i2c_init() is not enough ?

Yes, where is my brown paper bag...

I noticed this yesterday as well before falling asleep... ;)

> But in my opinion, all register settings should be merged in one please
> (= transfer function) and call pm_runtime_get/put there only.
> R-Car I2C is not related, but I2C power domain might be
> power OFF if SoC has power domain control feature (SH-Mobile has it).

OK, so you want this feture even if R-Car is not affected? No problem,
just want to make sure. But yes, this is a seperate patch.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]