| Message ID | 1450203249-4873-1-git-send-email-wsa@the-dreams.de |
|---|---|
| State | Superseded |
| Headers | show |
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 -- 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
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 -- 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
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
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!
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 -- 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
> > 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!
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);
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(-)