[RFC PATCH v5 16/16] usb, xhci: flush initial hub discovery to gate port power control

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

 



Until all root hubs have been discovered and tier mismatch identified,
port power control is unreliable.  When a USB3 port is paired with an
incorrect peer port there is chance a connected device will downgrade
its connection to its USB2 pins.  The downgrade occurs when the USB3
port is powered off while the USB2 peer port is powered.  Once the peer
ports are correctly assigned the kernel will prevent the disconnect
scenario.  So, wait for the peer ports to be correctly assigned before
allowing a USB3 port to power off.

Now that khubd is a workqueue, and provided that khubd arranges to
re-queue enumeration work until all hubs (connected at driver load) are
enumerated, a drain_workqueue() operation will wait for all initial
discovery to complete.  This requires the delayed hub->init_work to move
to khubd context.  Care is taken to maintain parallel waiting for all
root hubs power on delays.  However, since hub_quiesce() is called with
the device lock held it can no longer synchronously wait for init_work
to flush.

The workqueue subsystem enforces that no un-chained work (work queued
outside the workqueue context, e.g. hub_irq) may be queued for the
duration of the drain.  Add infrastructure to defer and replay
kick_khubd() requests.

Not Signed-off, pending resolution of the locking horror in hub_quiesce()
---
 drivers/usb/core/hcd.c       |    1 
 drivers/usb/core/hub.c       |   88 ++++++++++++++++++++++++++++++++++--------
 drivers/usb/core/hub.h       |    4 +-
 drivers/usb/core/port.c      |    4 ++
 drivers/usb/host/xhci-pci.c  |    5 ++
 drivers/usb/host/xhci-plat.c |    4 ++
 drivers/usb/host/xhci.c      |   10 +++++
 drivers/usb/host/xhci.h      |    5 ++
 include/linux/device.h       |    5 ++
 include/linux/usb.h          |    1 
 include/linux/usb/hcd.h      |    3 +
 11 files changed, 112 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index b3ecaf32f2aa..f79bd06edc5f 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2424,6 +2424,7 @@ struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver,
 		dev_dbg (dev, "hcd alloc failed\n");
 		return NULL;
 	}
+	INIT_LIST_HEAD(&hcd->khubd_defer);
 	if (primary_hcd == NULL) {
 		hcd->bandwidth_mutex = kmalloc(sizeof(*hcd->bandwidth_mutex),
 				GFP_KERNEL);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e0518af66af9..894d0e47b563 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -562,14 +562,20 @@ static void hub_release(struct kref *kref)
 	kfree(hub);
 }
 
-static void kick_khubd(struct usb_hub *hub)
+static int khubd_draining;
+
+static void kick_khubd_unlocked(struct usb_hub *hub)
 {
 	struct usb_interface *intf = to_usb_interface(hub->intfdev);
-	unsigned long flags;
+	struct usb_hcd *hcd = bus_to_hcd(hub->hdev->bus);
 
-	/* Suppress autosuspend until khubd runs */
-	spin_lock_irqsave(&hub_event_lock, flags);
-	if (!hub->disconnected) {
+	if (hub->disconnected)
+		return;
+
+	if (khubd_draining) {
+		list_move_tail(&hub->defer, &hcd->khubd_defer);
+	} else {
+		/* Suppress autosuspend until khubd runs */
 		usb_autopm_get_interface_no_resume(intf);
 		kref_get(&hub->kref);
 		if (!queue_work(khubd_wq, &hub->event_work)) {
@@ -577,6 +583,14 @@ static void kick_khubd(struct usb_hub *hub)
 			kref_put(&hub->kref, hub_release);
 		}
 	}
+}
+
+static void kick_khubd(struct usb_hub *hub)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&hub_event_lock, flags);
+	kick_khubd_unlocked(hub);
 	spin_unlock_irqrestore(&hub_event_lock, flags);
 }
 
@@ -588,6 +602,33 @@ void usb_kick_khubd(struct usb_device *hdev)
 		kick_khubd(hub);
 }
 
+void usb_drain_khubd(struct usb_hcd *hcd)
+{
+	static DEFINE_MUTEX(drain_mutex);
+	struct usb_hub *hub, *_h;
+
+	/* prevent concurrent drainers */
+	mutex_lock(&drain_mutex);
+
+	/* flag khubd to start deferring work */
+	spin_lock_irq(&hub_event_lock);
+	khubd_draining = 1;
+	spin_unlock_irq(&hub_event_lock);
+
+	drain_workqueue(khubd_wq);
+
+	spin_lock_irq(&hub_event_lock);
+	khubd_draining = 0;
+	list_for_each_entry_safe(hub, _h, &hcd->khubd_defer, defer) {
+		list_del_init(&hub->defer);
+		kick_khubd_unlocked(hub);
+	}
+	spin_unlock_irq(&hub_event_lock);
+
+	mutex_unlock(&drain_mutex);
+}
+EXPORT_SYMBOL_GPL(usb_drain_khubd);
+
 /*
  * 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.
@@ -1044,10 +1085,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 		 */
 		if (type == HUB_INIT) {
 			delay = hub_power_on(hub, false);
-			PREPARE_DELAYED_WORK(&hub->init_work, hub_init_func2);
-			queue_delayed_work(system_power_efficient_wq,
-					&hub->init_work,
-					msecs_to_jiffies(delay));
+			PREPARE_WORK(&hub->init_work, hub_init_func2);
+			hub->init_deadline = jiffies + msecs_to_jiffies(delay);
+			queue_work(khubd_wq, &hub->init_work);
 
 			/* Suppress autosuspend until init is done */
 			usb_autopm_get_interface_no_resume(
@@ -1199,10 +1239,9 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 
 		/* Don't do a long sleep inside a workqueue routine */
 		if (type == HUB_INIT2) {
-			PREPARE_DELAYED_WORK(&hub->init_work, hub_init_func3);
-			queue_delayed_work(system_power_efficient_wq,
-					&hub->init_work,
-					msecs_to_jiffies(delay));
+			PREPARE_WORK(&hub->init_work, hub_init_func3);
+			hub->init_deadline = jiffies + msecs_to_jiffies(delay);
+			queue_work(khubd_wq, &hub->init_work);
 			return;		/* Continues at init3: below */
 		} else {
 			msleep(delay);
@@ -1229,15 +1268,21 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 /* Implement the continuations for the delays above */
 static void hub_init_func2(struct work_struct *ws)
 {
-	struct usb_hub *hub = container_of(ws, struct usb_hub, init_work.work);
+	struct usb_hub *hub = container_of(ws, struct usb_hub, init_work);
+	unsigned long now = jiffies;
 
+	if (time_before(now, hub->init_deadline))
+		schedule_timeout(hub->init_deadline - now);
 	hub_activate(hub, HUB_INIT2);
 }
 
 static void hub_init_func3(struct work_struct *ws)
 {
-	struct usb_hub *hub = container_of(ws, struct usb_hub, init_work.work);
+	struct usb_hub *hub = container_of(ws, struct usb_hub, init_work);
+	unsigned long now = jiffies;
 
+	if (time_before(now, hub->init_deadline))
+		schedule_timeout(hub->init_deadline - now);
 	hub_activate(hub, HUB_INIT3);
 }
 
@@ -1250,7 +1295,14 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
 	struct usb_device *hdev = hub->hdev;
 	int i;
 
-	cancel_delayed_work_sync(&hub->init_work);
+	/* XXX: locking horror */
+	if (usb_device_is_locked(hdev)) {
+		usb_unlock_device(hdev);
+		cancel_work_sync(&hub->init_work);
+		usb_lock_device(hdev);
+	} else {
+		cancel_work_sync(&hub->init_work);
+	}
 
 	/* khubd and related activity won't re-trigger */
 	hub->quiescing = 1;
@@ -1604,6 +1656,7 @@ static void hub_disconnect(struct usb_interface *intf)
 	 */
 	hub->disconnected = 1;
 	spin_lock_irq(&hub_event_lock);
+	list_del_init(&hub->defer);
 	spin_unlock_irq(&hub_event_lock);
 
 	/* Disconnect all children and quiesce the hub */
@@ -1729,7 +1782,8 @@ descriptor_error:
 	hub->intfdev = &intf->dev;
 	hub->hdev = hdev;
 	INIT_DELAYED_WORK(&hub->leds, led_work);
-	INIT_DELAYED_WORK(&hub->init_work, NULL);
+	INIT_WORK(&hub->init_work, NULL);
+	INIT_LIST_HEAD(&hub->defer);
 	usb_get_intf(intf);
 
 	usb_set_intfdata (intf, hub);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 96a058c8d85f..898f3afb5e0a 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -37,6 +37,7 @@ struct usb_hub {
 		struct usb_port_status	port;
 	}			*status;	/* buffer for status reports */
 	struct mutex		status_mutex;	/* for the status buffer */
+	struct list_head	defer;
 
 	int			error;		/* last reported error */
 	int			nerrors;	/* track consecutive errors */
@@ -72,7 +73,8 @@ struct usb_hub {
 	unsigned		has_indicators:1;
 	u8			indicator[USB_MAXCHILDREN];
 	struct delayed_work	leds;
-	struct delayed_work	init_work;
+	struct work_struct	init_work;
+	unsigned long		init_deadline;
 	struct usb_port		**ports;
 };
 
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 0a3361eeb08a..c0b2493fdef7 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -125,6 +125,7 @@ static int usb_port_runtime_suspend(struct device *dev)
 	struct usb_port *port_dev = to_usb_port(dev);
 	struct usb_device *hdev = to_usb_device(dev->parent->parent);
 	struct usb_interface *intf = to_usb_interface(dev->parent);
+	struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 	struct usb_port *peer = port_dev->peer;
 	int port1 = port_dev->portnum;
@@ -133,6 +134,9 @@ static int usb_port_runtime_suspend(struct device *dev)
 	if (!hub)
 		return -EINVAL;
 
+	if (test_bit(HCD_FLAG_INIT_DISCO_BUSY, &hcd->flags))
+		return -EAGAIN;
+
 	if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF)
 			== PM_QOS_FLAGS_ALL)
 		return -EAGAIN;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 04f986d9234f..826d1179b26b 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -216,10 +216,14 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	 */
 	*((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;
 
+	set_bit(HCD_FLAG_INIT_DISCO_BUSY, &xhci->shared_hcd->flags);
 	retval = usb_add_hcd(xhci->shared_hcd, dev->irq,
 			IRQF_SHARED);
 	if (retval)
 		goto put_usb3_hcd;
+	async_schedule_domain(xhci_async_drain, xhci->shared_hcd,
+			      &xhci_async_probe_domain);
+
 	/* Roothub already marked as USB 3.0 speed */
 
 	/* We know the LPM timeout algorithms for this host, let the USB core
@@ -241,6 +245,7 @@ static void xhci_pci_remove(struct pci_dev *dev)
 {
 	struct xhci_hcd *xhci;
 
+	async_synchronize_full_domain(&xhci_async_probe_domain);
 	xhci = hcd_to_xhci(pci_get_drvdata(dev));
 	if (xhci->shared_hcd) {
 		usb_remove_hcd(xhci->shared_hcd);
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8affef910782..72ccce1c4d25 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -158,9 +158,12 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	 */
 	*((struct xhci_hcd **) xhci->shared_hcd->hcd_priv) = xhci;
 
+	set_bit(HCD_FLAG_INIT_DISCO_BUSY, &xhci->shared_hcd->flags);
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto put_usb3_hcd;
+	async_schedule_domain(xhci_async_drain, xhci->shared_hcd,
+			      &xhci_async_probe_domain);
 
 	return 0;
 
@@ -187,6 +190,7 @@ static int xhci_plat_remove(struct platform_device *dev)
 	struct usb_hcd	*hcd = platform_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 
+	async_synchronize_full_domain(&xhci_async_probe_domain);
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6fe577d46fa2..172ae31da502 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -44,6 +44,16 @@ static unsigned int quirks;
 module_param(quirks, uint, S_IRUGO);
 MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
 
+ASYNC_DOMAIN_EXCLUSIVE(xhci_async_probe_domain);
+void xhci_async_drain(void *_hcd, async_cookie_t cookie)
+{
+	struct usb_hcd *hcd = _hcd;
+
+	usb_drain_khubd(hcd);
+	clear_bit(HCD_FLAG_INIT_DISCO_BUSY, &hcd->flags);
+}
+
+
 /* TODO: copied from ehci-hcd.c - can this be refactored? */
 /*
  * xhci_handshake - spin reading hc until handshake completes or fails
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 58ed9d088e63..a2ade1bf5783 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -27,6 +27,7 @@
 #include <linux/timer.h>
 #include <linux/kernel.h>
 #include <linux/usb/hcd.h>
+#include <linux/async.h>
 
 /* Code sharing between pci-quirks and xhci hcd */
 #include	"xhci-ext-caps.h"
@@ -1636,6 +1637,10 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd *xhci)
 	return xhci->quirks & XHCI_LINK_TRB_QUIRK;
 }
 
+/* xHCI async probe drain */
+extern struct async_domain xhci_async_probe_domain;
+void xhci_async_drain(void *_hcd, async_cookie_t cookie);
+
 /* xHCI debugging */
 void xhci_print_ir_set(struct xhci_hcd *xhci, int set_num);
 void xhci_print_registers(struct xhci_hcd *xhci);
diff --git a/include/linux/device.h b/include/linux/device.h
index 952b01033c32..263c7a119fac 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -890,6 +890,11 @@ static inline void device_unlock(struct device *dev)
 	mutex_unlock(&dev->mutex);
 }
 
+static inline int device_is_locked(struct device *dev)
+{
+	return mutex_is_locked(&dev->mutex);
+}
+
 void driver_init(void);
 
 /*
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 7f6eb859873e..67a85d33e039 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -610,6 +610,7 @@ extern struct usb_device *usb_hub_find_child(struct usb_device *hdev,
 #define usb_lock_device(udev)		device_lock(&(udev)->dev)
 #define usb_unlock_device(udev)		device_unlock(&(udev)->dev)
 #define usb_trylock_device(udev)	device_trylock(&(udev)->dev)
+#define usb_device_is_locked(udev)	device_is_locked(&(udev)->dev)
 extern int usb_lock_device_for_reset(struct usb_device *udev,
 				     const struct usb_interface *iface);
 
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index efe8d8a7c7ad..62ecf9b936e4 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -119,6 +119,7 @@ struct usb_hcd {
 #define HCD_FLAG_WAKEUP_PENDING		4	/* root hub is resuming? */
 #define HCD_FLAG_RH_RUNNING		5	/* root hub is running? */
 #define HCD_FLAG_DEAD			6	/* controller has died? */
+#define HCD_FLAG_INIT_DISCO_BUSY	7	/* initial discovery busy */
 
 	/* The flags can be tested using these macros; they are likely to
 	 * be slightly faster than test_bit().
@@ -167,6 +168,7 @@ struct usb_hcd {
 	struct mutex		*bandwidth_mutex;
 	struct usb_hcd		*shared_hcd;
 	struct usb_hcd		*primary_hcd;
+	struct list_head	khubd_defer;
 
 
 #define HCD_BUFFER_POOLS	4
@@ -427,6 +429,7 @@ extern int usb_add_hcd(struct usb_hcd *hcd,
 		unsigned int irqnum, unsigned long irqflags);
 extern void usb_remove_hcd(struct usb_hcd *hcd);
 extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1);
+extern void usb_drain_khubd(struct usb_hcd *hcd);
 
 struct platform_device;
 extern void usb_hcd_platform_shutdown(struct platform_device *dev);

--
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