[ adding Rafael ]
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 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.
> +
> + 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.
> diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
> index 483632087788..a63ad55dbf6f 100644
> --- a/Documentation/power/pm_qos_interface.txt
> +++ b/Documentation/power/pm_qos_interface.txt
> @@ -94,8 +94,8 @@ flags. Values are updated in response to changes of the request list.
>
> Target latency value is simply the minimum of the request values held in the
> parameter list elements. The PM QoS flags aggregate value is a gather (bitwise
> -OR) of all list elements' values. Two device PM QoS flags are defined currently:
> -PM_QOS_FLAG_NO_POWER_OFF and PM_QOS_FLAG_REMOTE_WAKEUP.
> +OR) of all list elements' values. Three device PM QoS flags are defined currently:
> +PM_QOS_FLAG_NO_POWER_OFF, PM_QOS_FLAG_REMOTE_WAKEUP, PM_QOS_FLAG_NO_NOTIFY.
>
> Note: the aggregated target value is implemented as an atomic variable so that
> reading the aggregated value does not require any locking mechanism.
> @@ -148,13 +148,14 @@ from the device's power directory.
>
> int dev_pm_qos_expose_flags(device, value)
> Add a request to the device's PM QoS list of flags and create sysfs attributes
> -pm_qos_no_power_off and pm_qos_remote_wakeup under the device's power directory
> -allowing user space to change these flags' value.
> +pm_qos_no_power_off, pm_qos_remote_wakeup, and pm_qos_no_notify_flags under the
> +device's power directory allowing user space to change these flags' value.
>
> void dev_pm_qos_hide_flags(device)
> -Drop the request added by dev_pm_qos_expose_flags() from the device's PM QoS list
> -of flags and remove sysfs attributes pm_qos_no_power_off and pm_qos_remote_wakeup
> -under the device's power directory.
> +Drop the request added by dev_pm_qos_expose_flags() from the device's PM QoS
> +list of flags and remove sysfs attributes pm_qos_no_power_off,
> +pm_qos_remote_wakeup, and pm_qos_no_notify_flags under the device's power
> +directory.
>
> Notification mechanisms:
> The per-device PM QoS framework has 2 different and distinct notification trees:
> diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
> index 5c1361a9e5dd..b8ec64e8ee66 100644
> --- a/drivers/base/power/qos.c
> +++ b/drivers/base/power/qos.c
> @@ -737,12 +737,12 @@ void dev_pm_qos_hide_flags(struct device *dev)
> EXPORT_SYMBOL_GPL(dev_pm_qos_hide_flags);
>
> /**
> - * dev_pm_qos_update_flags - Update PM QoS flags request owned by user space.
> - * @dev: Device to update the PM QoS flags request for.
> - * @mask: Flags to set/clear.
> - * @set: Whether to set or clear the flags (true means set).
> + * dev_pm_qos_update_flags_nonotify - update flags without notifying a driver
> + *
> + * This routine is only meant to be used by drivers that are forwarding
> + * userspace flag change requests between known peers
> */
> -int dev_pm_qos_update_flags(struct device *dev, s32 mask, bool set)
> +int dev_pm_qos_update_flags_nonotify(struct device *dev, s32 mask, bool set)
> {
> s32 value;
> int ret;
> @@ -768,6 +768,44 @@ int dev_pm_qos_update_flags(struct device *dev, s32 mask, bool set)
> pm_runtime_put(dev);
> return ret;
> }
> +EXPORT_SYMBOL_GPL(dev_pm_qos_update_flags_nonotify);
> +
> +/**
> + * dev_pm_qos_update_flags - Update PM QoS flags request owned by user space.
> + * @dev: Device to update the PM QoS flags request for.
> + * @mask: Flags to set/clear.
> + * @set: Whether to set or clear the flags (true means set).
> + */
> +int dev_pm_qos_update_flags(struct device *dev, s32 mask, bool set)
> +{
> + bool (*callback)(struct device *, s32, bool);
> + enum pm_qos_flags_status stat;
> + bool done = false;
> +
> + if (dev->pm_domain)
> + callback = dev->pm_domain->ops.notify_flags;
> + else if (dev->type && dev->type->pm)
> + callback = dev->type->pm->notify_flags;
> + else if (dev->class && dev->class->pm)
> + callback = dev->class->pm->notify_flags;
> + else if (dev->bus && dev->bus->pm)
> + callback = dev->bus->pm->notify_flags;
> + else
> + callback = NULL;
> +
> + if (!callback && dev->driver && dev->driver->pm)
> + callback = dev->driver->pm->notify_flags;
> +
> + stat = dev_pm_qos_flags(dev, PM_QOS_FLAG_NO_NOTIFY);
> + if (callback && stat == PM_QOS_FLAGS_NONE)
> + done = callback(dev, mask, set);
> +
> + /* callback consumed the event */
> + if (done)
> + return 0;
> +
> + return dev_pm_qos_update_flags_nonotify(dev, mask, set);
> +}
> #else /* !CONFIG_PM_RUNTIME */
> static void __dev_pm_qos_hide_latency_limit(struct device *dev) {}
> static void __dev_pm_qos_hide_flags(struct device *dev) {}
> diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
> index 03e089ade5ce..a1e4be468b7f 100644
> --- a/drivers/base/power/sysfs.c
> +++ b/drivers/base/power/sysfs.c
> @@ -252,9 +252,7 @@ static ssize_t pm_qos_no_power_off_show(struct device *dev,
> & PM_QOS_FLAG_NO_POWER_OFF));
> }
>
> -static ssize_t pm_qos_no_power_off_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t n)
> +static ssize_t flags_store(struct device *dev, s32 mask, const char *buf, size_t n)
> {
> int ret;
>
> @@ -264,13 +262,40 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
> if (ret != 0 && ret != 1)
> return -EINVAL;
>
> - ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
> + ret = dev_pm_qos_update_flags(dev, mask, ret);
> return ret < 0 ? ret : n;
> }
>
> +static ssize_t pm_qos_no_power_off_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return flags_store(dev, PM_QOS_FLAG_NO_POWER_OFF, buf, n);
> +}
> +
> static DEVICE_ATTR(pm_qos_no_power_off, 0644,
> pm_qos_no_power_off_show, pm_qos_no_power_off_store);
>
> +static ssize_t pm_qos_no_notify_flags_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%d\n", !!(dev_pm_qos_requested_flags(dev)
> + & PM_QOS_FLAG_NO_POWER_OFF));
> +}
> +
> +
> +static ssize_t pm_qos_no_notify_flags_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + return flags_store(dev, PM_QOS_FLAG_NO_NOTIFY, buf, n);
> +}
> +
> +static DEVICE_ATTR(pm_qos_no_notify_flags, 0644,
> + pm_qos_no_notify_flags_show, pm_qos_no_notify_flags_store);
> +
> +
> static ssize_t pm_qos_remote_wakeup_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -283,16 +308,7 @@ static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t n)
> {
> - int ret;
> -
> - if (kstrtoint(buf, 0, &ret))
> - return -EINVAL;
> -
> - if (ret != 0 && ret != 1)
> - return -EINVAL;
> -
> - ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
> - return ret < 0 ? ret : n;
> + return flags_store(dev, PM_QOS_FLAG_REMOTE_WAKEUP, buf, n);
> }
>
> static DEVICE_ATTR(pm_qos_remote_wakeup, 0644,
> @@ -633,6 +649,7 @@ static struct attribute *pm_qos_flags_attrs[] = {
> #ifdef CONFIG_PM_RUNTIME
> &dev_attr_pm_qos_no_power_off.attr,
> &dev_attr_pm_qos_remote_wakeup.attr,
> + &dev_attr_pm_qos_no_notify_flags.attr,
> #endif /* CONFIG_PM_RUNTIME */
> NULL,
> };
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 51542f852393..237b91bb2079 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -20,6 +20,7 @@
> #include <linux/pm_qos.h>
>
> #include "hub.h"
> +#include "usb-platform.h"
>
> static const struct attribute_group *port_dev_group[];
>
> @@ -131,12 +132,18 @@ static int usb_port_runtime_suspend(struct device *dev)
> usb_autopm_put_interface(intf);
> return retval;
> }
> +
> +static bool usb_port_notify_flags(struct device *dev, s32 mask, bool set)
> +{
> + return usb_connector_notify_flags(to_usb_port(dev), mask, set);
> +}
> #endif
>
> static const struct dev_pm_ops usb_port_pm_ops = {
> #ifdef CONFIG_PM_RUNTIME
> .runtime_suspend = usb_port_runtime_suspend,
> .runtime_resume = usb_port_runtime_resume,
> + .notify_flags = usb_port_notify_flags,
> #endif
> };
>
> diff --git a/drivers/usb/core/usb-platform.c b/drivers/usb/core/usb-platform.c
> index 6b39d1fde8e4..8bec79131bb1 100644
> --- a/drivers/usb/core/usb-platform.c
> +++ b/drivers/usb/core/usb-platform.c
> @@ -8,6 +8,7 @@
> * Software Foundation, version 2.
> *
> */
> +#include <linux/pm_qos.h>
> #include <linux/device.h>
> #include "usb-platform.h"
>
> @@ -53,6 +54,43 @@ struct usb_domain *usb_create_domain(void)
> }
> EXPORT_SYMBOL_GPL(usb_create_domain);
>
> +bool usb_connector_notify_flags(struct usb_port *port_dev, s32 mask, bool set)
> +{
> + LIST_HEAD(tmp);
> + struct usb_port *p;
> + struct usb_domain *udom;
> + struct usb_connector *uconn = port_dev->connector;
> +
> + if (!uconn || mask != PM_QOS_FLAG_NO_POWER_OFF)
> + return false;
> +
> + udom = uconn->domain;
> + mutex_lock(&udom->lock);
> +
> + /* resume the connector to set the flags */
> + list_for_each_entry(port_dev, &uconn->ports, node) {
> + pm_runtime_get_sync(&port_dev->dev);
> + dev_pm_qos_update_flags_nonotify(&port_dev->dev, mask, set);
> + }
> +
> + /* order the resulting suspensions disconnected ports first */
> + list_for_each_entry_safe(port_dev, p, &uconn->ports, node) {
> + if (port_dev->child)
> + list_move(&port_dev->node, &tmp);
> + else
> + pm_runtime_put(&port_dev->dev);
> + }
> +
> + list_for_each_entry_safe(port_dev, p, &tmp, node) {
> + list_move(&port_dev->node, &uconn->ports);
> + pm_runtime_put(&port_dev->dev);
> + }
> + mutex_unlock(&udom->lock);
> +
> + /* notify pm core that we completed the request ourself */
> + return true;
> +}
> +
> static struct usb_connector *create_connector(struct usb_domain *udom,
> struct usb_port *port_dev,
> size_t pair_data)
> diff --git a/drivers/usb/core/usb-platform.h b/drivers/usb/core/usb-platform.h
> index 768bc1d4c831..1938479dc7ad 100644
> --- a/drivers/usb/core/usb-platform.h
> +++ b/drivers/usb/core/usb-platform.h
> @@ -42,3 +42,4 @@ struct usb_connector {
> int usb_domain_pair_port(struct usb_domain *udom, struct usb_port *port_dev,
> void *data, size_t size);
> void usb_domain_remove_port(struct usb_domain *udom, struct usb_port *port_dev);
> +bool usb_connector_notify_flags(struct usb_port *port_dev, s32 mask, bool set);
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index a224c7f5c377..12e2d5b549fd 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -268,6 +268,10 @@ typedef struct pm_message {
> * these conditions and handle the device as appropriate, possibly queueing
> * a suspend request for it. The return value is ignored by the PM core.
> *
> + * @notify_flags: Device wants to know when the flags are being changed
> + * as it may have implications on a peer device. For examle USB
> + * peer port power management.
> + *
> * Refer to Documentation/power/runtime_pm.txt for more information about the
> * role of the above callbacks in device runtime power management.
> *
> @@ -297,6 +301,7 @@ struct dev_pm_ops {
> int (*runtime_suspend)(struct device *dev);
> int (*runtime_resume)(struct device *dev);
> int (*runtime_idle)(struct device *dev);
> + bool (*notify_flags)(struct device *dev, s32 mask, bool set);
> };
>
> #ifdef CONFIG_PM_SLEEP
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 5a95013905c8..c4bdf5d21a6b 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -36,6 +36,7 @@ enum pm_qos_flags_status {
>
> #define PM_QOS_FLAG_NO_POWER_OFF (1 << 0)
> #define PM_QOS_FLAG_REMOTE_WAKEUP (1 << 1)
> +#define PM_QOS_FLAG_NO_NOTIFY (1 << 2)
>
> struct pm_qos_request {
> struct plist_node node;
> @@ -205,6 +206,7 @@ static inline s32 dev_pm_qos_requested_flags(struct device *dev)
> {
> return dev->power.qos->flags_req->data.flr.flags;
> }
> +int dev_pm_qos_update_flags_nonotify(struct device *dev, s32 mask, bool set);
> #else
> static inline int dev_pm_qos_expose_latency_limit(struct device *dev, s32 value)
> { return 0; }
> @@ -217,6 +219,9 @@ static inline int dev_pm_qos_update_flags(struct device *dev, s32 m, bool set)
>
> static inline s32 dev_pm_qos_requested_latency(struct device *dev) { return 0; }
> static inline s32 dev_pm_qos_requested_flags(struct device *dev) { return 0; }
> +static inline int dev_pm_qos_update_flags_nonotify(struct device *dev, s32 mask,
> + bool set)
> + { return 0; }
> #endif
>
> #endif
>
--
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
