On 25 September 2017 at 01:17, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > The power management handling in dw_i2c_plat_probe() is somewhat > messy and it is rather hard to figure out the code intention for > the case when pm_disabled is set. In that case, the driver doesn't > enable runtime PM at all, but in addition to that it calls > pm_runtime_forbid() as though it wasn't sure if runtime PM might > be enabled for the device later by someone else. > > Although that concern doesn't seem to be actually valid, the > device is clearly still expected to be PM-capable even in the > pm_disabled set case, so a better approach would be to enable > runtime PM for it unconditionally and then prevent it from > being runtime-suspended by using pm_runtime_forbid(). This is nice cleanup! However I have one suggestion/comment. Using pm_runtime_forbid() to prevent the device from being runtime suspended may be a bit fragile, as userspace can then still change to "allow" it. Wouldn't it be better to bump the runtime PM usage count (pm_runtime_get_noresume()) instead? [...] Kind regards Uffe