Re: [PATCH 3/4] usb: Add usb port system pm support
On 2013/3/29 4:47, Sarah Sharp wrote: On Thu, Mar 28, 2013 at 07:58:47AM +0800, Lan Tianyu wrote: On 2013/3/28 2:47, Alan Stern wrote: On Thu, 28 Mar 2013, Lan Tianyu wrote What happens if there's no device plugged in to the port, but the hub is enabled for remote wakeup? How will the hub be able to detect a plug-in event if the port isn't powered? Alan Stern Hi Alan: Great thanks for your review. The hub will not detect the new devices. From my opinion, this depends on the user space since the port only will be powered off when pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset the flag, losing plug-in event should have been took into account. So basically, you're saying that any new distro policy that turns off port power will need to re-enable it if the user wants hotplug events from a hub? I think that's a very important thing to document. I really think we need to add more description about the port power off mechanisms to Documentation/usb/power-management.txt. There's a little bit in Documentation/ABI/testing/sysfs-bus-usb, but since this mechanism is so new, I think we need to educate distro users on its effects and suggestions on how to use it. Can you take a first stab at an overview, and I'll let you know what's missing? That's good idea. Ok. I will do that. Sarah Sharp -- Best Regards Tianyu Lan linux kernel enabling team -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: Add usb port system pm support
On Thu, Mar 28, 2013 at 07:58:47AM +0800, Lan Tianyu wrote: > On 2013/3/28 2:47, Alan Stern wrote: > >On Thu, 28 Mar 2013, Lan Tianyu wrote > >What happens if there's no device plugged in to the port, but the hub > >is enabled for remote wakeup? How will the hub be able to detect a > >plug-in event if the port isn't powered? > > > >Alan Stern > > > Hi Alan: > Great thanks for your review. > The hub will not detect the new devices. From my opinion, this > depends on the user space since the port only will be powered off when > pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset > the flag, losing plug-in event should have been took into account. So basically, you're saying that any new distro policy that turns off port power will need to re-enable it if the user wants hotplug events from a hub? I think that's a very important thing to document. I really think we need to add more description about the port power off mechanisms to Documentation/usb/power-management.txt. There's a little bit in Documentation/ABI/testing/sysfs-bus-usb, but since this mechanism is so new, I think we need to educate distro users on its effects and suggestions on how to use it. Can you take a first stab at an overview, and I'll let you know what's missing? Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: Add usb port system pm support
On 2013/3/28 2:47, Alan Stern wrote: On Thu, 28 Mar 2013, Lan Tianyu wrote What happens if there's no device plugged in to the port, but the hub is enabled for remote wakeup? How will the hub be able to detect a plug-in event if the port isn't powered? Alan Stern Hi Alan: Great thanks for your review. The hub will not detect the new devices. From my opinion, this depends on the user space since the port only will be powered off when pm qos NO_POWER_OFF flag is unset(it is default to be set). If unset the flag, losing plug-in event should have been took into account. -- Best Regards Tianyu Lan linux kernel enabling team -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] usb: Add usb port system pm support
On Thu, 28 Mar 2013, Lan Tianyu wrote:
> This patch is to add usb port system pm support. Add
> usb port's system suspend/resume callbacks and call
> usb_port_runtime_resume/suspend() to power off these
> ports whose pm qos NO_POWER_OFF flag is not set, system
> wakeup is disabled and persist is enalbed.
>
> During system pm, usb port should be powered off after
> dev being suspended and powered on before dev being
> resumed. Since usb ports and devs enable async suspend,
> call device_pm_wait_for_dev() in the usb_port_system_suspend()
> and usb_port_resume() to keeping the order.
>
> If usb port was already powered off by runtime pm with
> port_dev->power_is_on being false, usb_port_system_suspend()
> returns directly.
>
> If usb port was not powered off during system suspend with
> port_dev->power_is_on being true, usb_port_system_resume()
> returns directly.
>
> Signed-off-by: Lan Tianyu
...
> +static int usb_port_system_suspend(struct device *dev)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> +
> + if (!port_dev->power_is_on)
> + return 0;
> +
> + if (port_dev->child) {
> + struct usb_device *udev = port_dev->child;
> +
> + /*
> + * usb port can't be powered off when dev's system
> + * wakeup is enabled or persist is disabled.
> + */
> + if (device_may_wakeup(&udev->dev)
> + || !udev->persist_enabled)
> + return 0;
> +
> + /*
> + * usb port should be powered off after usb dev
> + * being suspended.
> + */
> + device_pm_wait_for_dev(dev, &port_dev->child->dev);
> + }
> +
> + usb_port_runtime_suspend(dev);
> + return 0;
> +}
What happens if there's no device plugged in to the port, but the hub
is enabled for remote wakeup? How will the hub be able to detect a
plug-in event if the port isn't powered?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] usb: Add usb port system pm support
This patch is to add usb port system pm support. Add
usb port's system suspend/resume callbacks and call
usb_port_runtime_resume/suspend() to power off these
ports whose pm qos NO_POWER_OFF flag is not set, system
wakeup is disabled and persist is enalbed.
During system pm, usb port should be powered off after
dev being suspended and powered on before dev being
resumed. Since usb ports and devs enable async suspend,
call device_pm_wait_for_dev() in the usb_port_system_suspend()
and usb_port_resume() to keeping the order.
If usb port was already powered off by runtime pm with
port_dev->power_is_on being false, usb_port_system_suspend()
returns directly.
If usb port was not powered off during system suspend with
port_dev->power_is_on being true, usb_port_system_resume()
returns directly.
Signed-off-by: Lan Tianyu
---
drivers/usb/core/hub.c |4
drivers/usb/core/port.c | 49 ---
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c228e69..8c2562e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3166,6 +3166,10 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
int status;
u16 portchange, portstatus;
+ /* Wait for usb port system resume finishing */
+ if (!PMSG_IS_AUTO(msg))
+ device_pm_wait_for_dev(&udev->dev, &port_dev->dev);
+
if (port_dev->did_runtime_put) {
status = pm_runtime_get_sync(&port_dev->dev);
port_dev->did_runtime_put = false;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 797f9d5..f14fc72 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -71,7 +71,7 @@ static void usb_port_device_release(struct device *dev)
kfree(port_dev);
}
-#ifdef CONFIG_USB_SUSPEND
+#ifdef CONFIG_PM
static int usb_port_runtime_resume(struct device *dev)
{
struct usb_port *port_dev = to_usb_port(dev);
@@ -131,25 +131,68 @@ static int usb_port_runtime_suspend(struct device *dev)
set_bit(port1, hub->busy_bits);
retval = usb_hub_set_port_power(hdev, port1, false);
usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
- usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
+ usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
clear_bit(port1, hub->busy_bits);
usb_autopm_put_interface(intf);
return retval;
}
-#endif
+
+static int usb_port_system_suspend(struct device *dev)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+
+ if (!port_dev->power_is_on)
+ return 0;
+
+ if (port_dev->child) {
+ struct usb_device *udev = port_dev->child;
+
+ /*
+* usb port can't be powered off when dev's system
+* wakeup is enabled or persist is disabled.
+*/
+ if (device_may_wakeup(&udev->dev)
+ || !udev->persist_enabled)
+ return 0;
+
+ /*
+* usb port should be powered off after usb dev
+* being suspended.
+*/
+ device_pm_wait_for_dev(dev, &port_dev->child->dev);
+ }
+
+ usb_port_runtime_suspend(dev);
+ return 0;
+}
+
+static int usb_port_system_resume(struct device *dev)
+{
+ struct usb_port *port_dev = to_usb_port(dev);
+
+ if (port_dev->power_is_on)
+ return 0;
+
+ return usb_port_runtime_resume(dev);
+}
static const struct dev_pm_ops usb_port_pm_ops = {
+ .suspend = usb_port_system_suspend,
+ .resume = usb_port_system_resume,
#ifdef CONFIG_USB_SUSPEND
.runtime_suspend = usb_port_runtime_suspend,
.runtime_resume = usb_port_runtime_resume,
.runtime_idle = pm_generic_runtime_idle,
#endif
};
+#endif
struct device_type usb_port_device_type = {
.name = "usb_port",
.release = usb_port_device_release,
+#if CONFIG_PM
.pm = &usb_port_pm_ops,
+#endif
};
int usb_hub_create_port_device(struct usb_hub *hub, int port1)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
