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

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

[Wolfram Sang]

Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
So, we can remove these calls.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 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] i2c: s3c2410: remove superfluous runtime PM calls

[Sylwester Nawrocki]

On 15/12/15 19:14, Wolfram Sang wrote:
> Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> So, we can remove these calls.

Won't this break i2c client devices that use runtime PM? Not sure
if any cases of such client exist now, I'll try to find some time
to test this change.

IIRC client's pm_runtime* calls return error if their parent device's
runtime PM is not enabled. Also enabling runtime PM on the i2c adapter
allows propagating runtime PM calls up to its parent - the i2c
controller platform device.

So perhaps in your patch we should only remove pm_runtime_get_sync,
pm_runtime_put calls, leaving runtime PM on the adapter enabled.
For any future cases where the Exynos I2C controller is not in an
always on power domain the runtime PM handling in this driver would
need to be further reworked anyway.

Perhaps we could just enable/disable adapter's runtime PM in i2c core.

--
Thanks,
Sylwester

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

[Charles Keepax]

On Wed, Dec 16, 2015 at 02:53:07PM +0100, Sylwester Nawrocki wrote:
> On 15/12/15 19:14, Wolfram Sang wrote:
> > Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> > pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> > So, we can remove these calls.
> 
> Won't this break i2c client devices that use runtime PM? Not sure
> if any cases of such client exist now, I'll try to find some time
> to test this change.
> 

Our CODECs can be controlled over I2C and use runtime PM.
However, this change doesn't seem to have any adverse effect on
them that I can find.

Tested-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

> IIRC client's pm_runtime* calls return error if their parent device's
> runtime PM is not enabled. Also enabling runtime PM on the i2c adapter
> allows propagating runtime PM calls up to its parent - the i2c
> controller platform device.
> 
> So perhaps in your patch we should only remove pm_runtime_get_sync,
> pm_runtime_put calls, leaving runtime PM on the adapter enabled.
> For any future cases where the Exynos I2C controller is not in an
> always on power domain the runtime PM handling in this driver would
> need to be further reworked anyway.
> 
> Perhaps we could just enable/disable adapter's runtime PM in i2c core.
> 

Afraid not sure I can comment on this part.

Thanks,
Charles

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

[Wolfram Sang]

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

On Thu, Dec 17, 2015 at 09:46:55AM +0000, Charles Keepax wrote:
> On Wed, Dec 16, 2015 at 02:53:07PM +0100, Sylwester Nawrocki wrote:
> > On 15/12/15 19:14, Wolfram Sang wrote:
> > > Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> > > pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> > > So, we can remove these calls.
> > 
> > Won't this break i2c client devices that use runtime PM? Not sure
> > if any cases of such client exist now, I'll try to find some time
> > to test this change.
> > 
> 
> Our CODECs can be controlled over I2C and use runtime PM.
> However, this change doesn't seem to have any adverse effect on
> them that I can find.
> 
> Tested-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks for testing!


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

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

[Wolfram Sang]

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

Hi,

> > Since commit 6ada5c1e1b077a ("i2c: Mark adapter devices with
> > pm_runtime_no_callbacks"), runtime PM on adapters turned into a no-op.
> > So, we can remove these calls.
> 
> Won't this break i2c client devices that use runtime PM? Not sure
> if any cases of such client exist now, I'll try to find some time
> to test this change.

Thanks, much appreciated.

> IIRC client's pm_runtime* calls return error if their parent device's
> runtime PM is not enabled. Also enabling runtime PM on the i2c adapter
> allows propagating runtime PM calls up to its parent - the i2c
> controller platform device.

I was thinking the PM core would take care of that. Grepping for
other users of pm_runtime_no_callbacks(), I see that most but not all
use pm_runtime_enable() on the device, too.

Asking linux-pm for help here: If we want to support RuntimePM for I2C
clients, do we need to enable RuntimePM on the logical I2C adapter
device (the bus master) which is already marked using
pm_runtime_no_callbacks?

> Perhaps we could just enable/disable adapter's runtime PM in i2c core.

That would be the proper place to do it, as I see it.

Thanks,

   Wolfram


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

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

[Alan Stern]

On Sat, 19 Dec 2015, Wolfram Sang wrote:

> Asking linux-pm for help here: If we want to support RuntimePM for I2C
> clients, do we need to enable RuntimePM on the logical I2C adapter
> device (the bus master) which is already marked using
> pm_runtime_no_callbacks?

In theory you don't need to.  But there are some advantages if you do: 
You get automatic runtime PM time accounting for the adapter device 
(how much time active and how much suspended), and suspend events will 
propagate from the I2C clients all the way up to the adapter's parent.

Alan Stern

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

[Wolfram Sang]

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


> > Asking linux-pm for help here: If we want to support RuntimePM for I2C
> > clients, do we need to enable RuntimePM on the logical I2C adapter
> > device (the bus master) which is already marked using
> > pm_runtime_no_callbacks?
> 
> In theory you don't need to.  But there are some advantages if you do: 
> You get automatic runtime PM time accounting for the adapter device 
> (how much time active and how much suspended), and suspend events will 
> propagate from the I2C clients all the way up to the adapter's parent.

That's exactly what I want. Thank you very much!


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