[PATCH 0/2] i2c: proper RuntimePM for the adapter device

[PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Wolfram Sang]

RuntimePM for the logical adapter device should be handled in a central place
by the core, and not by drivers. This series does exactly that.

This is the first step of harmonizing RuntimePM handling in I2C.


Wolfram Sang (2):
  i2c: always enable RuntimePM for the adapter device
  i2c: s3c2410: remove superfluous runtime PM calls

 drivers/i2c/busses/i2c-s3c2410.c | 6 ------
 drivers/i2c/i2c-core.c           | 3 +++
 2 files changed, 3 insertions(+), 6 deletions(-)

-- 
2.1.4

[PATCH 1/2] i2c: always enable RuntimePM for the adapter device

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The adapter device is a logical device. Because of that, it already uses
pm_runtime_no_callbacks() in the core. To ensure proper propagation from
the children (i2c devices) to the parent of the adapter (the HW device),
make sure RuntimePM is enabled in any case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e94d2ca2aab4aa..55159e8e53283a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1610,6 +1610,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	dev_dbg(&adap->dev, "adapter [%s] registered\n", adap->name);
 
 	pm_runtime_no_callbacks(&adap->dev);
+	pm_runtime_enable(&adap->dev);
 
 #ifdef CONFIG_I2C_COMPAT
 	res = class_compat_create_link(i2c_adapter_compat_class, &adap->dev,
@@ -1864,6 +1865,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 	/* device name is gone after device_unregister */
 	dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
 
+	pm_runtime_disable(&adap->dev);
+
 	/* wait until all references to the device are gone
 	 *
 	 * FIXME: This is old code and should ideally be replaced by an
-- 
2.1.4

Re: [PATCH 1/2] i2c: always enable RuntimePM for the adapter device

[Wolfram Sang]

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

On Wed, Dec 23, 2015 at 06:19:28PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The adapter device is a logical device. Because of that, it already uses
> pm_runtime_no_callbacks() in the core. To ensure proper propagation from
> the children (i2c devices) to the parent of the adapter (the HW device),
> make sure RuntimePM is enabled in any case.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

[PATCH 2/2] i2c: s3c2410: remove superfluous runtime PM calls

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

RuntimePM of the adapter device is now taken care of by the core. So, we
can remove these calls.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-s3c2410.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 5df819610d5280..362a6de548336b 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -784,7 +784,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 	int retry;
 	int ret;
 
-	pm_runtime_get_sync(&adap->dev);
 	ret = clk_enable(i2c->clk);
 	if (ret)
 		return ret;
@@ -795,7 +794,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 
 		if (ret != -EAGAIN) {
 			clk_disable(i2c->clk);
-			pm_runtime_put(&adap->dev);
 			return ret;
 		}
 
@@ -805,7 +803,6 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 	}
 
 	clk_disable(i2c->clk);
-	pm_runtime_put(&adap->dev);
 	return -EREMOTEIO;
 }
 
@@ -1256,8 +1253,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	pm_runtime_enable(&i2c->adap.dev);
-
 	dev_info(&pdev->dev, "%s: S3C I2C adapter\n", dev_name(&i2c->adap.dev));
 	return 0;
 }
@@ -1273,7 +1268,6 @@ static int s3c24xx_i2c_remove(struct platform_device *pdev)
 
 	clk_unprepare(i2c->clk);
 
-	pm_runtime_disable(&i2c->adap.dev);
 	pm_runtime_disable(&pdev->dev);
 
 	s3c24xx_i2c_deregister_cpufreq(i2c);
-- 
2.1.4

Re: [PATCH 2/2] i2c: s3c2410: remove superfluous runtime PM calls

[Wolfram Sang]

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

On Wed, Dec 23, 2015 at 06:19:29PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> RuntimePM of the adapter device is now taken care of by the core. So, we
> can remove these calls.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Shubhrajyoti Datta]

On Wed, Dec 23, 2015 at 10:49 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> RuntimePM for the logical adapter device should be handled in a central place
> by the core, and not by drivers. This series does exactly that.

This is good idea.

>
> This is the first step of harmonizing RuntimePM handling in I2C.
>

Also maybe we can add a flag that the driver enables.
that way for a short period till the driver removes the call
the double call is not there.

not an objection to the patch though
>
> Wolfram Sang (2):
>   i2c: always enable RuntimePM for the adapter device
>   i2c: s3c2410: remove superfluous runtime PM calls
>
>  drivers/i2c/busses/i2c-s3c2410.c | 6 ------
>  drivers/i2c/i2c-core.c           | 3 +++
>  2 files changed, 3 insertions(+), 6 deletions(-)
>
> --
> 2.1.4
>
> --
> 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

Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Wolfram Sang]

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


> > RuntimePM for the logical adapter device should be handled in a central place
> > by the core, and not by drivers. This series does exactly that.
> 
> This is good idea.

Thanks!

> Also maybe we can add a flag that the driver enables.
> that way for a short period till the driver removes the call
> the double call is not there.

The only driver I found was the s3c2410 and I fixed it. Is there another
one? Then, I'd update my series because I want a consistent state in one
go.


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

Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Shubhrajyoti Datta]

>
>> Also maybe we can add a flag that the driver enables.
>> that way for a short period till the driver removes the call
>> the double call is not there.
>
> The only driver I found was the s3c2410 and I fixed it. Is there another
> one? Then, I'd update my series because I want a consistent state in one
> go.
>
I think the below drivers

drivers/i2c/busses/i2c-at91.c
drivers/i2c/busses/i2c-designware-core.c
drivers/i2c/busses/i2c-designware-platdrv.c
drivers/i2c/busses/i2c-rcar.c
drivers/i2c/busses/i2c-sh_mobile.c
drivers/i2c/busses/i2c-hix5hd2.c
drivers/i2c/busses/i2c-nomadik.c
drivers/i2c/busses/i2c-omap.c
drivers/i2c/busses/i2c-qup.c
drivers/i2c/busses/i2c-s3c2410.c

Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Wolfram Sang]

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

> I think the below drivers
> 
> drivers/i2c/busses/i2c-at91.c
> drivers/i2c/busses/i2c-designware-core.c
> drivers/i2c/busses/i2c-designware-platdrv.c
> drivers/i2c/busses/i2c-rcar.c
> drivers/i2c/busses/i2c-sh_mobile.c
> drivers/i2c/busses/i2c-hix5hd2.c
> drivers/i2c/busses/i2c-nomadik.c
> drivers/i2c/busses/i2c-omap.c
> drivers/i2c/busses/i2c-qup.c
> drivers/i2c/busses/i2c-s3c2410.c

They use RuntimePM on their platform device, not on the adapter device
AFAICS.


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

Re: [PATCH 0/2] i2c: proper RuntimePM for the adapter device

[Shubhrajyoti Datta]

On Fri, Dec 25, 2015 at 3:55 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> I think the below drivers
>>
>> drivers/i2c/busses/i2c-at91.c
>> drivers/i2c/busses/i2c-designware-core.c
>> drivers/i2c/busses/i2c-designware-platdrv.c
>> drivers/i2c/busses/i2c-rcar.c
>> drivers/i2c/busses/i2c-sh_mobile.c
>> drivers/i2c/busses/i2c-hix5hd2.c
>> drivers/i2c/busses/i2c-nomadik.c
>> drivers/i2c/busses/i2c-omap.c
>> drivers/i2c/busses/i2c-qup.c
>> drivers/i2c/busses/i2c-s3c2410.c
>
> They use RuntimePM on their platform device, not on the adapter device
> AFAICS.
>
Indeed my bad.