| Message ID | 20170815143445.26167-1-jarkko.nikula@linux.intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Aug 15, 2017 at 05:34:44PM +0300, Jarkko Nikula wrote: > I guess pm_runtime_put_noidle() call in i2c_dw_probe_slave() was copied > by accident from similar master mode adapter registration code. It is > unbalanced due missing pm_runtime_get_noresume() but harmless since it > doesn't decrease dev->power.usage_count below zero. > > In theory we can hit similar needless runtime suspend/resume cycle > during slave mode adapter registration that was happening when > registering the master mode adapter. See commit cd998ded5c12 ("i2c: > designware: Prevent runtime suspend during adapter registration"). > > However, since we are slave, we can consider it as a wrong configuration > if we have other slaves attached under this adapter and can omit the > pm_runtime_get_noresume()/pm_runtime_put_noidle() calls for simplicity. > > Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Applied to for-current, thanks! Not really a bugfix, but 2/2 is, so, well... BTW, Luis, are you still there? Holiday season?
On 17-Aug-17 16:57, Wolfram Sang wrote: > On Tue, Aug 15, 2017 at 05:34:44PM +0300, Jarkko Nikula wrote: >> I guess pm_runtime_put_noidle() call in i2c_dw_probe_slave() was copied >> by accident from similar master mode adapter registration code. It is >> unbalanced due missing pm_runtime_get_noresume() but harmless since it >> doesn't decrease dev->power.usage_count below zero. >> >> In theory we can hit similar needless runtime suspend/resume cycle >> during slave mode adapter registration that was happening when >> registering the master mode adapter. See commit cd998ded5c12 ("i2c: >> designware: Prevent runtime suspend during adapter registration"). >> >> However, since we are slave, we can consider it as a wrong configuration >> if we have other slaves attached under this adapter and can omit the >> pm_runtime_get_noresume()/pm_runtime_put_noidle() calls for simplicity. >> >> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > > Applied to for-current, thanks! Not really a bugfix, but 2/2 is, so, > well... BTW, Luis, are you still there? Holiday season? > Hi all, Sorry I was on holidays yes. I will get my setup ready to test this ASAP. But it makes sense what Jarkko said in 2/2. If it helps, for what I can remember I also used i2c-rcar slave mode as inspiration for this implementation. Thank you, Luis
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c index 64c7c5e337b9..bb8f738cab14 100644 --- a/drivers/i2c/busses/i2c-designware-slave.c +++ b/drivers/i2c/busses/i2c-designware-slave.c @@ -382,7 +382,6 @@ int i2c_dw_probe_slave(struct dw_i2c_dev *dev) ret = i2c_add_numbered_adapter(adap); if (ret) dev_err(dev->dev, "failure adding adapter: %d\n", ret); - pm_runtime_put_noidle(dev->dev); return ret; }
I guess pm_runtime_put_noidle() call in i2c_dw_probe_slave() was copied by accident from similar master mode adapter registration code. It is unbalanced due missing pm_runtime_get_noresume() but harmless since it doesn't decrease dev->power.usage_count below zero. In theory we can hit similar needless runtime suspend/resume cycle during slave mode adapter registration that was happening when registering the master mode adapter. See commit cd998ded5c12 ("i2c: designware: Prevent runtime suspend during adapter registration"). However, since we are slave, we can consider it as a wrong configuration if we have other slaves attached under this adapter and can omit the pm_runtime_get_noresume()/pm_runtime_put_noidle() calls for simplicity. Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> --- drivers/i2c/busses/i2c-designware-slave.c | 1 - 1 file changed, 1 deletion(-)