Re: [RFC PATCH 04/15] PM / Runtime: Allow drivers to intercept pm qos flag changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday, October 24, 2013 12:42:36 AM Dan Williams wrote:
> [ adding Rafael ]

First, this is not the right address to Cc (please check MAINTAINERS in
3.12-rc6), but it still kind of works yet.

Second, quite frankly, I don't like this change.

> On Thu, Oct 24, 2013 at 12:35 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> > ...because struct dev_pm_ops is too clean and uniform.
> >
> > USB port power management needs to manage ports that are colocated in a
> > given connector.  If we power down a connector we need to arrange for
> > the peer port in the connector to also be suspended.  If the peer
> > connector stays active it may cause the device to disconnect on the initial
> > (suspended) port and reconnect on the other.
> >
> > Notification of when userspace disables pm_qos_no_power_off allows the
> > driver to arrange for the peer port to be suspended before the primary
> > port suspends.
> >
> > Alternatives considered:
> > 1/ force power down the peer port: denies userspace control over power
> >    policy
> >
> > 2/ try to synchronize port power management within the USB core:
> >    proved difficult to find a non-racy way to synchronize peer
> >    connections against power management changes.
> >
> > 3/ create infrastructure to expose the concept of peer devices in the
> >    pm_runtime system... maybe defer until other use cases outside of USB pop
> >    up.  For now, punting the notification to the driver is sufficient.
> >
> > As stated this notification allows the kernel to keep power-policy
> > consistent across a connector.  If userspace wants to regain per-port
> > control (at the risk of unexpected disconnects) it can set the
> > pm_qos_no_notify flag to prevent ports from coordinating as a connector.
> > pm_qos_no_notify blocks flags synchronization, restoring the traditional
> > behavior of one pm_qos_no_power_off manipulation only affecting one
> > device.
> >
> > Cc: Rafael J. Wysocki <rjw@xxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > Cc: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > ---
> >  Documentation/ABI/testing/sysfs-devices-power |   21 +++++++++++
> >  Documentation/power/pm_qos_interface.txt      |   15 ++++----
> >  drivers/base/power/qos.c                      |   48 ++++++++++++++++++++++---
> >  drivers/base/power/sysfs.c                    |   45 ++++++++++++++++-------
> >  drivers/usb/core/port.c                       |    7 ++++
> >  drivers/usb/core/usb-platform.c               |   38 ++++++++++++++++++++
> >  drivers/usb/core/usb-platform.h               |    1 +
> >  include/linux/pm.h                            |    5 +++
> >  include/linux/pm_qos.h                        |    5 +++
> >  9 files changed, 159 insertions(+), 26 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> > index efe449bdf811..36d1603ab3bb 100644
> > --- a/Documentation/ABI/testing/sysfs-devices-power
> > +++ b/Documentation/ABI/testing/sysfs-devices-power
> > @@ -235,3 +235,24 @@ Description:
> >
> >                 This attribute has no effect on system-wide suspend/resume and
> >                 hibernation.
> > +
> > +What:          /sys/devices/.../power/pm_qos_no_notify_flags
> > +Date:          October 2013
> > +Contact:       Dan Williams <dan.j.williams@xxxxxxxxx>
> > +Description:
> > +               The /sys/devices/.../power/pm_qos_no_notify_flags attribute is
> > +               used for determining whether the kernel is permitted to forward
> > +               changes to the the PM QoS flags (no_power_off, remote_wakeup) to
> > +               other devices it deems to be related in the system.  When this
> > +               flag is set userspace is indicating that it wants exclusive
> > +               control

This is aganist the way the PM QoS flags were designed.  There is no exclusive
control over them and user space cannot expect specific behavior when one of
them is unset in sysfs.  Specifically, "no power off" unset doesn't mean "power
off is required".  It merely means "I have no objections against powering off
if that's what you decide to do".

> > of the flags and takes responsibility for situations
> > +               where a driver would want the power control setting unified
> > +               amongst a set of devices.  In the case of USB a relationship is
> > +               indicating by a "peer:<hub -deviceB>-port<num>" link in the
> > +               <hub-deviceA>/port<num> directory.

I'm not sure what you're trying to achieve with this, but almost certainly this
isn't the right way to go.  Admittedly, I'm a bit tired now, so it's better if I
have a fresh look at this tomorrow.

> > +               Not all drivers support this attribute.  If it isn't supported,
> > +               it is not present.
> > +
> > +               This attribute has no effect on system-wide suspend/resume and
> > +               hibernation.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux