Re: [PATCH 0/2] Synchronize PM runtime enable state with parent

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 29 Sep 2022 16:46:16 +0200
Mårten Lindahl <marten.lindahl@axis.com> wrote:

> Hi!
> 
> A device that creates a child character device with cdev_device_add by
> default creates a PM sysfs group with power attributes for userspace
> control. This means that the power attributes monitors the child device
> only and thus does not reflect the parent device PM runtime behavior.
> 
> But as the PM runtime framework (rpm_suspend/rpm_resume) operates not
> only on a single device that has been enabled for runtime PM, but also
> on its parent, it should be possible to synchronize the child and the
> parent so that the power attribute monitoring reflects the child and
> the parent as one.
> 
> As an example, if an i2c_client device registers an iio_device, the
> iio_device will create sysfs power/attribute nodes for userspace
> control. But if the dev_pm_ops with resume/suspend callbacks is
> attached to the struct i2c_driver.driver.pm, the PM runtime needs to be
> enabled for the i2c_client device and not for the child iio_device.
> 
> In this case PM runtime can be enabled for the i2c_client device and
> suspend/resume callbacks will be triggered, but the child sysfs power
> attributes will be visible but marked as 'unsupported' and can not be
> used for control or monitoring. This can be confusing as the sysfs
> device node presents the i2c_client and the iio_device as one device.
> 
> A solution to this is to enable runtime PM not only for the i2c_device
> but for the iio_device child also. As there already exists a link from
> the child to its parent and both are enabled, all sysfs
> control/monitoring now reflects both devices, which from a userspace
> perspective makes more sense.
> 
> These patches [1] adds a function to the PM runtime framework to
> synchronize the PM runtime enable state of a device with its parent,
> and [2] applies it to an iio device, where the described issue exists.

Hi Mårten,

This seems sensible to me. With hindsight we should have probably
disabled the pm interface for IIO devices. It's a fun question on 
whether we could do so now without 'breaking' some user.  Obviously
the users wouldn't work today, for the reasons you point out, but
that doesn't mean some program isn't relying on the files being there.

Anyhow, I'm looking forward to input from others on this.

Jonathan


 
> 
> Kind regards
> Mårten Lindahl
> 
> Mårten Lindahl (2):
>   PM: runtime: Synchronize PM runtime enable state with parent
>   iio: light: vcnl4000: Incorporate iio_device with PM runtime
> 
>  drivers/base/power/runtime.c | 18 ++++++++++++++++++
>  drivers/iio/light/vcnl4000.c |  1 +
>  include/linux/pm_runtime.h   |  2 ++
>  3 files changed, 21 insertions(+)
>