Implement port power recovery via queuing warm reset requests in khubd context. There are two new cases where we want to trigger a warm reset to a port: 1/ Buggy xHCs that do not propagate warm reset to child ports when reset-resuming the root hub. [1] [hub_activate] 2/ Unconditionally when recovering a powered down port from pm_runtime_suspend. [usb_port_runtime_resume] In case (1), where we are presumably resetting all ports on the root hub we do not want to wait serially wait for each of those resets to complete. We arrange for the ports to be recovered asynchronously. Waiting for port recovery occurs if hub_event() is triggered or if a port has a child device and that device is resumed. In case (2) we do not want khubd to ever interpret a pm runtime powered down port as disconnected. Prior to this patch the code was getting lucky to clear the connection change event on port-power off and mitigate hub_event() from processing a disconnect. This is now explicit with hub->poweroff_bits. The ->poweroff_bits also gate hub_activate() from requesting port power recovery if the port power policy is off. Recovery in that case is queued via usb_port_runtime_resume(). Later cleanups: 1/ Asynchronous operation is sub-optimal in this patch because a usb_port is not the device model parent of its child usb_device, even though it is a power-management parent of the child device. Rectifying this allows all port recovery actions to be queued prior to starting usb_port_resume() for each attached child. 2/ The ->power_is_on flag is superseded by poweroff_bits and can be removed. [1]: http://marc.info/?l=linux-usb&m=138683173421509&w=2 Cc: Julius Werner <jwerner@xxxxxxxxxxxx> Cc: Vikas Sajjan <vikas.sajjan@xxxxxxxxxx> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/usb/core/generic.c | 2 + drivers/usb/core/hub.c | 38 ++++++++++++++++++-- drivers/usb/core/hub.h | 19 ++-------- drivers/usb/core/port.c | 80 +++++++++++++++++++++++++++++------------- drivers/usb/core/usb.h | 2 + drivers/usb/host/xhci-pci.c | 1 + drivers/usb/host/xhci-plat.c | 1 + drivers/usb/host/xhci.c | 8 ++++ drivers/usb/host/xhci.h | 2 + include/linux/usb.h | 2 + include/linux/usb/hcd.h | 6 +++ 11 files changed, 117 insertions(+), 44 deletions(-) diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c index 2925db4e3859..3da29cd8ca54 100644 --- a/drivers/usb/core/generic.c +++ b/drivers/usb/core/generic.c @@ -217,7 +217,7 @@ static int generic_suspend(struct usb_device *udev, pm_message_t msg) } /* port resume runs in khubd context to coalesce port recovery - * actions and synchronize with hub_events + * actions and synchronize with hub_event */ void usb_port_resume_work(struct work_struct *w) { diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index e7b2efef537d..c4763f109113 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -590,6 +590,11 @@ void usb_kick_resume(struct usb_device *dev) queue_work(khubd_wq, &dev->resume_work); } +void usb_kick_port_resume(struct usb_port *port_dev) +{ + queue_work(khubd_wq, &port_dev->resume_work); +} + /* * Let the USB core know that a USB 3.0 device has sent a Function Wake Device * Notification, which indicates it had initiated remote wakeup. @@ -1083,6 +1088,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) struct usb_device *udev = hub->ports[port1 - 1]->child; u16 portstatus, portchange; + if (test_bit(port1, hub->poweroff_bits)) + continue; + portstatus = portchange = 0; status = hub_port_status(hub, port1, &portstatus, &portchange); if (udev || (portstatus & USB_PORT_STAT_CONNECTION)) @@ -1168,6 +1176,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) struct usb_port *port_dev = hub->ports[port1 - 1]; set_bit(port1, hub->resume_bits); + usb_kick_port_resume(port_dev); /* Don't set the change_bits when the device * was powered off. */ @@ -2712,6 +2721,21 @@ done: return status; } +int usb3_port_link_recover(struct usb_port *port_dev) +{ + int port1 = port_dev->portnum; + struct device *dev = &port_dev->dev; + struct usb_device *hdev = to_usb_device(dev->parent->parent); + struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + + if (!hub) + return -EIO; + + /* trigger a warm reset to recover from power session loss events */ + return hub_port_reset(hub, port1, NULL, HUB_LONG_RESET_TIME, true); +} +EXPORT_SYMBOL_GPL(usb3_port_link_recover); + /* Check if a port is power on */ static int port_is_power_on(struct usb_hub *hub, unsigned portstatus) { @@ -3186,6 +3210,8 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) } } + usb_port_sync_resume(); + /* 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)) @@ -3841,7 +3867,7 @@ EXPORT_SYMBOL_GPL(usb_enable_ltm); * every 25ms for transient disconnects. When the port status has been * unchanged for 100ms it returns the port status. */ -int hub_port_debounce(struct usb_hub *hub, int port1, bool must_be_connected) +int hub_port_debounce(struct usb_hub *hub, int port1) { int ret; int total_time, stable_time = 0; @@ -3855,8 +3881,7 @@ int hub_port_debounce(struct usb_hub *hub, int port1, bool must_be_connected) if (!(portchange & USB_PORT_STAT_C_CONNECTION) && (portstatus & USB_PORT_STAT_CONNECTION) == connection) { - if (!must_be_connected || - (connection == USB_PORT_STAT_CONNECTION)) + if (connection == USB_PORT_STAT_CONNECTION) stable_time += HUB_DEBOUNCE_STEP; if (stable_time >= HUB_DEBOUNCE_STABLE) break; @@ -4432,7 +4457,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, if (portchange & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE)) { - status = hub_port_debounce_be_stable(hub, port1); + status = hub_port_debounce(hub, port1); if (status < 0) { if (status != -ENODEV && printk_ratelimit()) dev_err(hub_dev, "connect-debounce failed, " @@ -4649,6 +4674,8 @@ static void hub_event(struct work_struct *w) u16 portstatus, portchange; u16 hubstatus, hubchange; + usb_port_sync_resume(); + do { dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n", hdev->state, hdev->maxchild, @@ -4714,6 +4741,9 @@ static void hub_event(struct work_struct *w) connect_change = 1; } + if (test_bit(i, hub->poweroff_bits)) + continue; + if (portchange & USB_PORT_STAT_C_ENABLE) { if (!connect_change) dev_dbg (hub_dev, diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h index 7547713d036a..2ff998ffbb3d 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -46,6 +46,7 @@ struct usb_hub { status change */ unsigned long busy_bits[1]; /* ports being reset or resumed */ + unsigned long poweroff_bits[1]; /* ports policy off */ unsigned long removed_bits[1]; /* ports with a "removed" device present */ unsigned long wakeup_bits[1]; /* ports that have signaled @@ -91,7 +92,9 @@ struct usb_port { struct device dev; struct dev_state *port_owner; enum usb_port_connect_type connect_type; + struct work_struct resume_work; u8 portnum; + unsigned resume_power:1; unsigned power_is_on:1; unsigned did_runtime_put:1; }; @@ -99,6 +102,7 @@ struct usb_port { #define to_usb_port(_dev) \ container_of(_dev, struct usb_port, dev) +extern void usb_port_sync_resume(void); extern int usb_hub_create_port_device(struct usb_hub *hub, int port1); extern void usb_hub_remove_port_device(struct usb_hub *hub, @@ -106,20 +110,5 @@ extern void usb_hub_remove_port_device(struct usb_hub *hub, extern int usb_hub_set_port_power(struct usb_device *hdev, struct usb_hub *hub, int port1, bool set); extern struct usb_hub *usb_hub_to_struct_hub(struct usb_device *hdev); -extern int hub_port_debounce(struct usb_hub *hub, int port1, - bool must_be_connected); extern int usb_clear_port_feature(struct usb_device *hdev, int port1, int feature); - -static inline int hub_port_debounce_be_connected(struct usb_hub *hub, - int port1) -{ - return hub_port_debounce(hub, port1, true); -} - -static inline int hub_port_debounce_be_stable(struct usb_hub *hub, - int port1) -{ - return hub_port_debounce(hub, port1, false); -} - diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 51542f852393..33a5b7910ff0 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -17,6 +17,7 @@ */ #include <linux/slab.h> +#include <linux/async.h> #include <linux/pm_qos.h> #include "hub.h" @@ -62,48 +63,76 @@ static const struct attribute_group *port_dev_group[] = { NULL, }; +static ASYNC_DOMAIN_EXCLUSIVE(usb_port_async_domain); + +void usb_port_sync_resume(void) +{ + async_synchronize_full_domain(&usb_port_async_domain); +} + static void usb_port_device_release(struct device *dev) { struct usb_port *port_dev = to_usb_port(dev); + usb_port_sync_resume(); kfree(port_dev); } #ifdef CONFIG_PM_RUNTIME -static int usb_port_runtime_resume(struct device *dev) +static void async_port_dev_resume(void *port, async_cookie_t cookie) { - struct usb_port *port_dev = to_usb_port(dev); + struct usb_port *port_dev = port; + struct device *dev = &port_dev->dev; struct usb_device *hdev = to_usb_device(dev->parent->parent); struct usb_interface *intf = to_usb_interface(dev->parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_hcd *hcd = bus_to_hcd(hdev->bus); int port1 = port_dev->portnum; - int retval; if (!hub) - return -EINVAL; + return; usb_autopm_get_interface(intf); - set_bit(port1, hub->busy_bits); - - retval = usb_hub_set_port_power(hdev, hub, port1, true); - if (port_dev->child && !retval) { - /* - * Attempt to wait for usb hub port to be reconnected in order - * to make the resume procedure successful. The device may have - * disconnected while the port was powered off, so ignore the - * return status. - */ - retval = hub_port_debounce_be_connected(hub, port1); - if (retval < 0) - dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", - retval); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); - retval = 0; + if (port_dev->resume_power) { + usb_hub_set_port_power(hdev, hub, port1, true); + + /* safe to de-ref ->child since we know hub_event() is idle */ + if (port_dev->child && + port_dev->child->persist_enabled) + set_bit(port1, hub->resume_bits); + clear_bit(port1, hub->poweroff_bits); } - clear_bit(port1, hub->busy_bits); + if (hcd->driver->port_resume) + hcd->driver->port_resume(hcd, port_dev, port_dev->resume_power); + port_dev->resume_power = 0; usb_autopm_put_interface(intf); - return retval; +} + +/* khubd flushes in-flight port_resume reactions, and new port recovery + * actions may not start while khubd is active. When a power session is + * resumed the device may appear disconnected so we want khubd to be + * held off until port recovery is complete. + */ +static void usb_port_dev_resume_work(struct work_struct *w) +{ + struct usb_port *port_dev; + + port_dev = container_of(w, typeof(*port_dev), resume_work); + async_schedule_domain(async_port_dev_resume, port_dev, + &usb_port_async_domain); +} + +/* child device resume synchronizes in-flight port recovery */ +static int usb_port_runtime_resume(struct device *dev) +{ + struct usb_port *port_dev = to_usb_port(dev); + + usb_port_sync_resume(); + port_dev->resume_power = 1; + usb_kick_port_resume(port_dev); + + return 0; } static int usb_port_runtime_suspend(struct device *dev) @@ -122,13 +151,15 @@ static int usb_port_runtime_suspend(struct device *dev) == PM_QOS_FLAGS_ALL) return -EAGAIN; + set_bit(port1, hub->poweroff_bits); usb_autopm_get_interface(intf); - set_bit(port1, hub->busy_bits); retval = usb_hub_set_port_power(hdev, hub, port1, false); + if (retval) + clear_bit(port1, hub->poweroff_bits); usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION); 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 @@ -164,6 +195,7 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1) port_dev->dev.groups = port_dev_group; port_dev->dev.type = &usb_port_device_type; dev_set_name(&port_dev->dev, "port%d", port1); + INIT_WORK(&port_dev->resume_work, usb_port_dev_resume_work); retval = device_register(&port_dev->dev); if (retval) diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index ea060cce6a0c..ce46fab04ef2 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -49,8 +49,10 @@ static inline unsigned usb_get_max_power(struct usb_device *udev, return c->desc.bMaxPower * mul; } +struct usb_port; extern void usb_kick_khubd(struct usb_device *dev); extern void usb_kick_resume(struct usb_device *dev); +extern void usb_kick_port_resume(struct usb_port *port_dev); extern int usb_match_one_id_intf(struct usb_device *dev, struct usb_host_interface *intf, const struct usb_device_id *id); diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 4221dee924b5..e58c9dcf643f 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -334,6 +334,7 @@ static const struct hc_driver xhci_pci_hc_driver = { .enable_device = xhci_enable_device, .update_hub_device = xhci_update_hub_device, .reset_device = xhci_discover_or_reset_device, + .port_resume = xhci_port_resume, /* * scheduling support diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index 8abda5c73ca1..ab8e4eb63031 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -72,6 +72,7 @@ static const struct hc_driver xhci_plat_xhci_driver = { .enable_device = xhci_enable_device, .update_hub_device = xhci_update_hub_device, .reset_device = xhci_discover_or_reset_device, + .port_resume = xhci_port_resume, /* * scheduling support diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d68ec1aa473d..662afcdd332c 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3542,6 +3542,14 @@ command_cleanup: return ret; } +void xhci_port_resume(struct usb_hcd *hcd, struct usb_port *port_dev, + bool runtime) +{ + /* unconditionally issue warm reset on pm_runtime resume */ + if (runtime) + usb3_port_link_recover(port_dev); +} + /* * At this point, the struct usb_device is about to go away, the device has * disconnected, and all traffic has been stopped and the endpoints have been diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 24344aab2107..7a724f73b7a8 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1782,6 +1782,8 @@ void xhci_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep); int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev); int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev); void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev); +void xhci_port_resume(struct usb_hcd *hcd, struct usb_port *port_dev, + bool runtime); /* xHCI ring, segment, TRB, and TD functions */ dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb); diff --git a/include/linux/usb.h b/include/linux/usb.h index fecd02495269..93f92b72909c 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -618,6 +618,8 @@ extern int usb_lock_device_for_reset(struct usb_device *udev, /* USB port reset for device reinitialization */ extern int usb_reset_device(struct usb_device *dev); extern void usb_queue_reset_device(struct usb_interface *dev); +struct usb_port; +extern int usb3_port_link_recover(struct usb_port *port_dev); #ifdef CONFIG_ACPI extern int usb_acpi_set_power_state(struct usb_device *hdev, int index, diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index b6781f4f0d9c..5a5df4eb2254 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -377,6 +377,12 @@ struct hc_driver { int (*disable_usb3_lpm_timeout)(struct usb_hcd *, struct usb_device *, enum usb3_link_state state); int (*find_raw_port_number)(struct usb_hcd *, int); + /* for xHC hcd's that fail to propagate a spec mandated + * warm reset when resuming the root hub. Or for the + * runtime pm case where a warm reset is required and + * will not be generated by power recovery + */ + void (*port_resume)(struct usb_hcd *, struct usb_port *, bool); }; static inline int hcd_giveback_urb_in_bh(struct usb_hcd *hcd) -- 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
