On 2012年12月07日 23:22, Alan Stern wrote: > On Fri, 7 Dec 2012, Lan Tianyu wrote: > >>> Maybe you really need two flags. Do whatever is best; I'm sure you can >>> figure out a good scheme. >> Yeah. Two flags maybe good. In this situation, it should be call >> "power_is_on", right? power_is_on can be used to avoid to sending >> redundant PORT_POWER request. The other flag is dedicated to prevent >> device from being disconnected. Something like hub->busy_bits. We can >> can "child_busy", I am not very good at giving a name. So I'd like to >> see your opinion. :) > > How about "needs_debounce"? > > Alan Stern > Hi Alan: I write a patch based on the needs_debounce flag. The flag will be set when port has child device and power on successfully. Otherwise, I separate resume port and wait for connect in the usb_port_resume(). Device will not be disconnected when power_is_on is false or needs_debounce is set. Welcome comments. diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 86e595c..aa41d3a 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -26,6 +26,7 @@ #include <linux/mutex.h> #include <linux/freezer.h> #include <linux/random.h> +#include <linux/pm_qos.h> #include <asm/uaccess.h> #include <asm/byteorder.h> @@ -108,11 +109,14 @@ MODULE_PARM_DESC(use_both_schemes, DECLARE_RWSEM(ehci_cf_port_reset_rwsem); EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem); +#define HUB_PORT_RECONNECT_TRIES 20 + #define HUB_DEBOUNCE_TIMEOUT 1500 #define HUB_DEBOUNCE_STEP 25 #define HUB_DEBOUNCE_STABLE 100 static int usb_reset_and_verify_device(struct usb_device *udev); +static int hub_port_debounce(struct usb_hub *hub, int port1); static inline char *portspeed(struct usb_hub *hub, int portstatus) { @@ -718,13 +722,26 @@ int usb_hub_set_port_power(struct usb_device *hdev, int port1, bool set) { int ret; + struct usb_hub *hub = hdev_to_hub(hdev); + struct usb_port *port_dev + = hub->ports[port1 - 1]; + + if (set) { + if (port_dev->power_is_on) + return 0; - if (set) ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER); - else + } else { + if (!port_dev->power_is_on) + return 0; + ret = clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER); + } + + if (!ret) + port_dev->power_is_on = set; return ret; } @@ -804,7 +821,11 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay) dev_dbg(hub->intfdev, "trying to enable port power on " "non-switchable hub\n"); for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) - set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER); + if (hub->ports[port1 - 1]->power_is_on) + set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER); + else + clear_port_feature(hub->hdev, port1, + USB_PORT_FEAT_POWER); /* Wait at least 100 msec for power to become stable */ delay = max(pgood_delay, (unsigned) 100); @@ -2768,6 +2789,7 @@ EXPORT_SYMBOL_GPL(usb_enable_ltm); int usb_port_suspend(struct usb_device *udev, pm_message_t msg) { struct usb_hub *hub = hdev_to_hub(udev->parent); + struct usb_port *port_dev = hub->ports[udev->portnum - 1]; int port1 = udev->portnum; int status; @@ -2862,6 +2884,18 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) udev->port_is_suspended = 1; msleep(10); } + + /* + * Check whether current status is meeting requirement of + * usb port power off mechanism + */ + if (!udev->do_remote_wakeup + && (dev_pm_qos_flags(&port_dev->dev, + PM_QOS_FLAG_NO_POWER_OFF) != PM_QOS_FLAGS_ALL) + && udev->persist_enabled + && !status) + pm_runtime_put_sync(&port_dev->dev); + usb_mark_last_busy(hub->hdev); return status; } @@ -2945,6 +2979,25 @@ static int finish_port_resume(struct usb_device *udev) return status; } +static int usb_port_wait_for_connected(struct usb_hub *hub, int port1) +{ + int status, i; + + for (i = 0; i < HUB_PORT_RECONNECT_TRIES; i++) { + status = hub_port_debounce(hub, port1); + if (status & USB_PORT_STAT_CONNECTION) { + /* + * just clear enable-change feature since debounce + * has cleared connect-change feature. + */ + clear_port_feature(hub->hdev, port1, + USB_PORT_FEAT_C_ENABLE); + return 0; + } + } + return -ETIMEDOUT; +} + /* * usb_port_resume - re-activate a suspended usb device's upstream port * @udev: device to re-activate, not a root hub @@ -2982,10 +3035,39 @@ static int finish_port_resume(struct usb_device *udev) int usb_port_resume(struct usb_device *udev, pm_message_t msg) { struct usb_hub *hub = hdev_to_hub(udev->parent); + struct usb_port *port_dev = hub->ports[udev->portnum - 1]; int port1 = udev->portnum; int status; u16 portchange, portstatus; + + set_bit(port1, hub->busy_bits); + + if (!port_dev->power_is_on) { + status = pm_runtime_get_sync(&port_dev->dev); + if (status < 0) { + dev_dbg(&udev->dev, "can't resume usb port, status %d\n", + status); + clear_bit(port1, hub->busy_bits); + return status; + } + } + + + /* + * Wait for usb hub port to be reconnected in order to make + * the resume procedure successful. + */ + if (port_dev->needs_debounce) { + status = usb_port_wait_for_connected(hub, port1); + if (status < 0) { + dev_dbg(&udev->dev, "can't get reconnection after setting port power on, status %d\n", + status); + clear_bit(port1, hub->busy_bits); + return status; + } + } + /* Skip the initial Clear-Suspend step for a remote wakeup */ status = hub_port_status(hub, port1, &portstatus, &portchange); if (status == 0 && !port_is_suspended(hub, portstatus)) @@ -2993,8 +3075,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) // dev_dbg(hub->intfdev, "resume port %d\n", port1); - set_bit(port1, hub->busy_bits); - /* see 7.1.7.7; affects power usage, but not budgeting */ if (hub_is_superspeed(hub->hdev)) status = set_port_feature(hub->hdev, @@ -4126,6 +4206,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, struct usb_device *hdev = hub->hdev; struct device *hub_dev = hub->intfdev; struct usb_hcd *hcd = bus_to_hcd(hdev->bus); + struct usb_port *port_dev = hub->ports[port1 - 1]; unsigned wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics); struct usb_device *udev; @@ -4175,6 +4256,11 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, } } + if (!port_dev->power_is_on || port_dev->needs_debounce) { + clear_bit(port1, hub->change_bits); + return; + } + /* Disconnect any existing devices under this port */ if (udev) { if (hcd->phy && !hdev->parent && diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 8ea6bc8..e561ec2 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -80,6 +80,8 @@ struct usb_hub { * @port_owner: port's owner * @connect_type: port's connect type * @portnum: port index num based one + * @power_is_on: port's power state + * @needs_debounce: flag to ignore some port events after power on. */ struct usb_port { struct usb_device *child; @@ -87,6 +89,8 @@ struct usb_port { struct dev_state *port_owner; enum usb_port_connect_type connect_type; u8 portnum; + bool power_is_on; + bool needs_debounce; }; #define to_usb_port(_dev) \ diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index f5f8890..46a31fe 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -74,9 +74,14 @@ static int usb_port_runtime_resume(struct device *dev) struct usb_port *port_dev = to_usb_port(dev); struct usb_device *hdev = to_usb_device(dev->parent->parent); + int retval; - return usb_hub_set_port_power(hdev, port_dev->portnum, + retval = usb_hub_set_port_power(hdev, port_dev->portnum, true); + + if (port_dev->child && !retval) + port_dev->needs_debounce = true; + return retval; } static int usb_port_runtime_suspend(struct device *dev) @@ -120,6 +125,8 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) hub->ports[port1 - 1] = port_dev; port_dev->portnum = port1; + port_dev->power_is_on = true; + port_dev->needs_debounce = false; port_dev->dev.parent = hub->intfdev; port_dev->dev.groups = port_dev_group; port_dev->dev.type = &usb_port_device_type; -- Best regards Tianyu Lan -- 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