[RFC] Delay EHCI initialization until khubd is idle

[Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>]

Everybody:

Frans Pop has been bothered by the error messages that appear in the 
system log when EHCI initialization interrupts the initialization of 
devices attached to the companion controllers.

The patch below provides a way to prevent these messages from being
generated, most of the time.  It forces EHCI initialization to wait
until khubd is idle, which won't happen until all devices attached to a
companion controller have been initialized and enumerated, though not
necessarily probed.

Now this is cosmetic at best.  Devices will still suffer a short
disconnection when EHCI starts up; the difference is that now the
errors will affect the drivers bound to those devices rather than
affecting khubd.  Since drivers tend to produce relatively few error
messages when a disconnect occurs (unlike khubd which tends to complain
a lot), the log remains cleaner.

In theory this might even be detrimental.  For example, it's possible
that the delay might allow time for a USB flash drive to be mounted
before it is disconnected, instead of being disconnected before
usb-storage can bind to it.

The question is whether the patch should be merged.  Opinions?

Alan Stern



Index: usb-2.6/drivers/usb/core/hcd.h
===================================================================
--- usb-2.6.orig/drivers/usb/core/hcd.h
+++ usb-2.6/drivers/usb/core/hcd.h
@@ -486,4 +486,7 @@ static inline void usbmon_urb_complete(s
  */
 extern struct rw_semaphore ehci_cf_port_reset_rwsem;
 
+/* This is also for use by khubd and ehci-hcd. */
+extern void usb_wait_for_khubd_idle(void);
+
 #endif /* __KERNEL__ */
Index: usb-2.6/drivers/usb/core/hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hub.c
+++ usb-2.6/drivers/usb/core/hub.c
@@ -3097,6 +3097,16 @@ loop:
         } /* end while (1) */
 }
 
+/* Let ehci-hcd know when khubd is idle */
+static int khubd_active;
+static DECLARE_WAIT_QUEUE_HEAD(khubd_wqh);
+
+void usb_wait_for_khubd_idle(void)
+{
+	wait_event(khubd_wqh, !khubd_active);
+}
+EXPORT_SYMBOL_GPL(usb_wait_for_khubd_idle);
+
 static int hub_thread(void *__unused)
 {
 	/* khubd needs to be freezable to avoid intefering with USB-PERSIST
@@ -3107,7 +3117,11 @@ static int hub_thread(void *__unused)
 	set_freezable();
 
 	do {
+		khubd_active = 1;
 		hub_events();
+		khubd_active = 0;
+		wake_up_all(&khubd_wqh);
+
 		wait_event_freezable(khubd_wait,
 				!list_empty(&hub_event_list) ||
 				kthread_should_stop());
Index: usb-2.6/drivers/usb/host/ehci-hcd.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/ehci-hcd.c
+++ usb-2.6/drivers/usb/host/ehci-hcd.c
@@ -604,7 +604,13 @@ static int ehci_run (struct usb_hcd *hcd
 	 * guarantees that no resets are in progress.  After we set CF,
 	 * a short delay lets the hardware catch up; new resets shouldn't
 	 * be started before the port switching actions could complete.
+	 *
+	 * In an attempt to avoid unwanted initialization error messages
+	 * for devices attached to companion controllers, we will wait
+	 * until khubd is idle before beginning.  This isn't truly a
+	 * general solution but it should help during system bootup.
 	 */
+	usb_wait_for_khubd_idle();
 	down_write(&ehci_cf_port_reset_rwsem);
 	hcd->state = HC_STATE_RUNNING;
 	ehci_writel(ehci, FLAG_CF, &ehci->regs->configured_flag);

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