Thread (2 messages) 2 messages, 2 authors, 2015-12-15

Re: [RFC] i2c: Prevent runtime suspend of parent during adapter registration

From: Alan Stern <stern@rowland.harvard.edu>
Date: 2015-12-15 15:21:06
Also in: linux-i2c

On Tue, 15 Dec 2015, Jarkko Nikula wrote:
If runtime PM of parent device is enabled before it registers the I2C
adapter there can be unnecessary runtime suspend during adapter device
registration. This can happen when adapter is registered from parent's
probe and if parent device is initially active platform device.

In that case power.usage_count of parent device is zero and
pm_runtime_get()/pm_runtime_put() cycle during probe could put the
parent into runtime suspend. This happens when the i2c_register_adapter()
calls the device_register():

i2c_register_adapter
  device_register
    device_add
      bus_probe_device
        device_initial_probe
          __device_attach
            if (dev->parent) pm_runtime_get_sync(dev->parent)
            ...
            if (dev->parent) pm_runtime_put(dev->parent)

After that i2c_register_adapter() continues registering I2C slave
devices. In case slave device probe does I2C transfers the parent will
resume again and thus get a needless runtime suspend/resume cycle during
adapter registration.

Prevent this while retaining the runtime PM status of parent by only
incrementing/decrementing parent device power usage count during I2C
adapter registration. That makes sure there won't be spurious runtime PM
status changes for parent's probe and lets the driver core to idle the
device after probe finishes.

Signed-off-by: Jarkko Nikula <redacted>
---
I noticed this with i2c-designware-platdrv.c which started to do so
after commit 36d48fb5766a ("i2c: designware-platdrv: enable RuntimePM
before registering to the core"). I guess the same could happen also
with a few other I2C drivers that enable runtime PM similar way. For
instance i2c-at91.c, i2c-cadence.c, i2c-hix5hd2.c and i2c-qup.c.
That made me thinking if issue might be best to handle in i2c-core.c.

Device core drivers/base/dd.c: driver_probe_device() or
drivers/base/platform.c: platform_drv_probe() could be other
alternatives but that would cause a regression to a driver that
purposively tries to suspend the device in its probe().
This isn't how the problem is handled in other places.  In all the 
cases I know about, the parent's subsystem or driver makes sure that 
the parent will not go into runtime suspend while it is being probed.  
At the very least, it should insure that the parent will not go into 
runtime suspend while it is registering a child device (which 
typically happens within a probe routine).

In general, it's bad form to do pm_runtime_get/put calls on your parent 
-- you should make changes only to the device you're responsible for.

Alan Stern
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help