[BK PATCH] More USB fixes for 2.5.67

[BK PATCH] More USB fixes for 2.5.67

[Greg KH]

Hi,

Here are some USB bugfixes.  These changesets fix the oft-reported oops
when disconnecting a USB hub.  They also fix a number of bugs found by
the smatch tool.

Due to the reworking of the USB core to disconnect all urbs in flight
when a device is disconnected, if CONFIG_USB_DEBUG is enabled, some
drivers might report a lot of the following type of messages in the
kernel log:
usb 1-1.2: hcd_unlink_urb ed85e2c0 fail -22
usb 1-1.2: hcd_unlink_urb ed85e320 fail -22
usb 1-1.2: hcd_unlink_urb ed85e380 fail -22
usb 1-1.2: hcd_unlink_urb ed85e3e0 fail -22
usb 1-1.2: hcd_unlink_urb ed85e440 fail -22
usb 1-1.2: hcd_unlink_urb ed85e4a0 fail -22
usb 1-1.2: hcd_unlink_urb ed85e500 fail -22

If this happens during normal operation (open, read, write, close type
stuff), please let the people at linux-usb-devel know what device and
driver causes this, those drivers need to be fixed up.

It is normal to see a lot of these messages when a device is
disconnected, and after the drivers have been cleaned up, this message
will be suppressed in a few kernel versions.

Also, I'm seeing a oops when unloading the host controller drivers in
the hotplug call for the root hub device, these patches do not fix that
problem, and I'm actively trying to track that down.  But due to the fact
that it is a fairly rare thing to unload a USB controller driver,
compared to removing a USB hub, I think these patches should be applied
now.


Please pull from:  bk://kernel.bkbits.net/gregkh/linux/linus-2.5


Patches will be posted to linux-usb-devel and lkml as a follow-up thread
for those who want to see them.

thanks,

greg k-h


 Documentation/DocBook/usb.tmpl     |    6 +
 drivers/usb/core/hcd.c             |  130 +++++++++++++++++++++++++++++++++----
 drivers/usb/core/hcd.h             |   11 +--
 drivers/usb/core/urb.c             |   11 ---
 drivers/usb/core/usb.c             |   22 +++++-
 drivers/usb/host/ehci-hcd.c        |  101 ++++++++++------------------
 drivers/usb/host/ohci-hcd.c        |   85 ++++++++++--------------
 drivers/usb/host/ohci-pci.c        |    2 
 drivers/usb/host/ohci-sa1111.c     |    2 
 drivers/usb/host/uhci-hcd.c        |    1 
 drivers/usb/image/scanner.c        |   10 +-
 drivers/usb/misc/speedtch.c        |    9 +-
 drivers/usb/serial/io_edgeport.c   |    5 -
 drivers/usb/serial/keyspan.c       |    6 -
 drivers/usb/serial/kl5kusb105.c    |   10 +-
 drivers/usb/serial/kobil_sct.c     |    8 +-
 drivers/usb/storage/unusual_devs.h |    9 ++
 17 files changed, 265 insertions(+), 163 deletions(-)
-----

<arndt@lin02384n012.mc.schoenewald.de>:
  o USB: Patch against unusual_devs.h to enable Pontis SP600

David Brownell <david-b@pacbell.net>:
  o USB: EHCI disconnect cleanup
  o USB: DocBook/usb.tmpl patch
  o disconnect cleanup, new HCD callback
  o USB: disconnect cleanup, new HCD callback

Duncan Sands <baldrick@wanadoo.fr>:
  o USB speedtouch: discard packets for non-existant vcc's

Greg Kroah-Hartman <greg@kroah.com>:
  o USB: keyspan: fixed up might_sleep() problems on device close
  o USB: io_edgeport: stop unlinking a urb that we don't need to unlink
  o USB: kobil_sct fix up errors found by smatch
  o USB: kl5kusb105 fix up errors found by smatch
  o USB: add better check to prevent oops in hcd_unlink_urb()
  o USB: fix up spin_unlock_irqrestore() issues in previous patch

Henning Meier-Geinitz <henning@meier-geinitz.de>:
  o USB scanner.c endpoint detection fix

[PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1055, 2003/04/14 10:13:28-07:00, david-b@pacbell.net

[PATCH] USB: disconnect cleanup, new HCD callback

Here's a streamlined version:  doesn't require the unlink
cleanup, and expects the *hci-hcd updates later.  Smaller,
and sanity checked against all three major HCDs.

  - reverts that unlink() patch: disconnect() callbacks
    can continue to act like they always have.

  - adds new "disable that endpoint" support, necessary
    for safely changing configurations or altsettings
    as well as physical disconnect (which is almost the
    same as setting config to zero, except for ep0).

  - NEW BEHAVIOUR:  usbcore cleans up after drivers that
    return from disconnect() with urbs still linked, by
    using the new "disable that endpoint" support.

Because it doesn't have any *hci-hcd updates, the hardware
synch needed by EHCI and OHCI (not UHCI) still gets done
through the bus->deallocate(dev) call ... not where we've
ever needed it, but 2.3-compatible (and finally fixable).

That gets rid of some problematic disconnect scenarios, and
other fixes can be phased in over time:  the *hci-hcd updates
to relocate the hardware synch point so it always happens in
a task context, and disabling endpoints before their configs
change (invalidating all HC state).


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c	Wed Apr 16 10:49:39 2003
+++ b/drivers/usb/core/hcd.c	Wed Apr 16 10:49:39 2003
@@ -1068,6 +1068,28 @@
 
 /*-------------------------------------------------------------------------*/
 
+/* this makes the hcd giveback() the urb more quickly, by kicking it
+ * off hardware queues (which may take a while) and returning it as
+ * soon as practical.  we've already set up the urb's return status,
+ * but we can't know if the callback completed already.
+ */
+static void
+unlink1 (struct usb_hcd *hcd, struct urb *urb)
+{
+	if (urb == (struct urb *) hcd->rh_timer.data)
+		usb_rh_status_dequeue (hcd, urb);
+	else {
+		int		value;
+
+		/* failures "should" be harmless */
+		value = hcd->driver->urb_dequeue (hcd, urb);
+		if (value != 0)
+			dev_dbg (hcd->controller,
+				"dequeue %p --> %d\n",
+				urb, value);
+	}
+}
+
 struct completion_splice {		// modified urb context:
 	/* did we complete? */
 	struct completion	done;
@@ -1176,6 +1198,7 @@
 	spin_unlock (&hcd_data_lock);
 	spin_unlock_irqrestore (&urb->lock, flags);
 
+	// FIXME remove splicing, so this becomes unlink1 (hcd, urb);
 	if (urb == (struct urb *) hcd->rh_timer.data) {
 		usb_rh_status_dequeue (hcd, urb);
 		retval = 0;
@@ -1213,11 +1236,98 @@
 
 /*-------------------------------------------------------------------------*/
 
-/* called by khubd, rmmod, apmd, or other thread for hcd-private cleanup */
+/* disables the endpoint: cancels any pending urbs, then synchronizes with
+ * the hcd to make sure all endpoint state is gone from hardware. use for
+ * set_configuration, set_interface, driver removal, physical disconnect.
+ *
+ * example:  a qh stored in hcd_dev.ep[], holding state related to endpoint
+ * type, maxpacket size, toggle, halt status, and scheduling.
+ */
+static void hcd_endpoint_disable (struct usb_device *udev, int endpoint)
+{
+	unsigned long	flags;
+	struct hcd_dev	*dev;
+	struct usb_hcd	*hcd;
+	struct urb	*urb;
+	unsigned	epnum = endpoint & USB_ENDPOINT_NUMBER_MASK;
 
-// FIXME:  likely best to have explicit per-setting (config+alt)
-// setup primitives in the usbcore-to-hcd driver API, so nothing
-// is implicit.  kernel 2.5 needs a bunch of config cleanup...
+	dev = udev->hcpriv;
+	hcd = udev->bus->hcpriv;
+
+rescan:
+	/* (re)block new requests, as best we can */
+	if (endpoint & USB_DIR_IN) {
+		usb_endpoint_halt (udev, epnum, 0);
+		udev->epmaxpacketin [epnum] = 0;
+	} else {
+		usb_endpoint_halt (udev, epnum, 1);
+		udev->epmaxpacketout [epnum] = 0;
+	}
+
+	/* then kill any current requests */
+	spin_lock_irqsave (&hcd_data_lock, flags);
+	list_for_each_entry (urb, &dev->urb_list, urb_list) {
+		int	tmp = urb->pipe;
+
+		/* ignore urbs for other endpoints */
+		if (usb_pipeendpoint (tmp) != epnum)
+			continue;
+		if ((tmp ^ endpoint) & USB_DIR_IN)
+			continue;
+
+		/* another cpu may be in hcd, spinning on hcd_data_lock
+		 * to giveback() this urb.  the races here should be
+		 * small, but a full fix needs a new "can't submit"
+		 * urb state.
+		 */
+		if (urb->status != -EINPROGRESS)
+			continue;
+		usb_get_urb (urb);
+		spin_unlock (&hcd_data_lock);
+
+		spin_lock (&urb->lock);
+		tmp = urb->status;
+		if (tmp == -EINPROGRESS)
+			urb->status = -ESHUTDOWN;
+		spin_unlock (&urb->lock);
+
+		/* kick hcd unless it's already returning this */
+		if (tmp == -EINPROGRESS) {
+			tmp = urb->pipe;
+			unlink1 (hcd, urb);
+			dev_dbg (hcd->controller,
+				"shutdown urb %p pipe %08x ep%d%s%s\n",
+				urb, tmp, usb_pipeendpoint (tmp),
+				(tmp & USB_DIR_IN) ? "in" : "out",
+				({ char *s; \
+				 switch (usb_pipetype (tmp)) { \
+				 case PIPE_CONTROL:	s = ""; break; \
+				 case PIPE_BULK:	s = "-bulk"; break; \
+				 case PIPE_INTERRUPT:	s = "-intr"; break; \
+				 default: 		s = "-iso"; break; \
+				}; s;}));
+		}
+		usb_put_urb (urb);
+
+		/* list contents may have changed */
+		goto rescan;
+	}
+	spin_unlock_irqrestore (&hcd_data_lock, flags);
+
+	/* synchronize with the hardware, so old configuration state
+	 * clears out immediately (and will be freed).
+	 */
+	might_sleep ();
+	if (hcd->driver->endpoint_disable)
+		hcd->driver->endpoint_disable (hcd, dev, endpoint);
+}
+
+/*-------------------------------------------------------------------------*/
+
+/* called by khubd, rmmod, apmd, or other thread for hcd-private cleanup.
+ * we're guaranteed that the device is fully quiesced.  also, that each
+ * endpoint has been hcd_endpoint_disabled.
+ */
 
 static int hcd_free_dev (struct usb_device *udev)
 {
@@ -1270,6 +1380,7 @@
 	.deallocate =		hcd_free_dev,
 	.buffer_alloc =		hcd_buffer_alloc,
 	.buffer_free =		hcd_buffer_free,
+	.disable =		hcd_endpoint_disable,
 };
 EXPORT_SYMBOL (usb_hcd_operations);
 
diff -Nru a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
--- a/drivers/usb/core/hcd.h	Wed Apr 16 10:49:39 2003
+++ b/drivers/usb/core/hcd.h	Wed Apr 16 10:49:39 2003
@@ -153,6 +153,8 @@
 			dma_addr_t *dma);
 	void (*buffer_free)(struct usb_bus *bus, size_t size,
 			void *addr, dma_addr_t dma);
+
+	void (*disable)(struct usb_device *udev, int bEndpointAddress);
 };
 
 /* each driver provides one of these, and hardware init support */
@@ -198,6 +200,10 @@
 	// urb_enqueue, and not freed by urb_dequeue
 	void		(*free_config) (struct usb_hcd *hcd,
 				struct usb_device *dev);
+
+	/* hw synch, freeing endpoint resources that urb_dequeue can't */
+	void 	(*endpoint_disable)(struct usb_hcd *hcd,
+			struct hcd_dev *dev, int bEndpointAddress);
 
 	/* root hub support */
 	int		(*hub_status_data) (struct usb_hcd *hcd, char *buf);
diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
--- a/drivers/usb/core/urb.c	Wed Apr 16 10:49:39 2003
+++ b/drivers/usb/core/urb.c	Wed Apr 16 10:49:39 2003
@@ -381,16 +381,7 @@
  */
 int usb_unlink_urb(struct urb *urb)
 {
-	/* FIXME
-	 * We should not care about the state here, but the host controllers
-	 * die a horrible death if we unlink a urb for a device that has been
-	 * physically removed.
-	 */
-	if (urb &&
-	    urb->dev &&
-	    (urb->dev->state >= USB_STATE_DEFAULT) &&
-	    urb->dev->bus &&
-	    urb->dev->bus->op)
+	if (urb && urb->dev && urb->dev->bus && urb->dev->bus->op)
 		return urb->dev->bus->op->unlink_urb(urb);
 	else
 		return -ENODEV;
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c	Wed Apr 16 10:49:39 2003
+++ b/drivers/usb/core/usb.c	Wed Apr 16 10:49:39 2003
@@ -786,8 +786,12 @@
  */
 void usb_disconnect(struct usb_device **pdev)
 {
-	struct usb_device * dev = *pdev;
-	int i;
+	struct usb_device	*dev = *pdev;
+	struct usb_bus		*bus = dev->bus;
+	struct usb_operations	*ops = bus->op;
+	int			i;
+
+	might_sleep ();
 
 	if (!dev)
 		return;
@@ -808,13 +812,25 @@
 			usb_disconnect(child);
 	}
 
+	/* disconnect() drivers from interfaces (a key side effect) */
 	dev_dbg (&dev->dev, "unregistering interfaces\n");
 	if (dev->actconfig) {
 		for (i = 0; i < dev->actconfig->desc.bNumInterfaces; i++) {
-			struct usb_interface *interface = &dev->actconfig->interface[i];
+			struct usb_interface	*interface;
 
 			/* remove this interface */
+			interface = &dev->actconfig->interface[i];
 			device_unregister(&interface->dev);
+		}
+	}
+
+	/* deallocate hcd/hardware state */
+	if (ops->disable) {
+		void	(*disable)(struct usb_device *, int) = ops->disable;
+
+		for (i = 0; i < 15; i++) {
+			disable (dev, i);
+			disable (dev, USB_DIR_IN | i);
 		}
 	}
 

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1061, 2003/04/14 10:25:17-07:00, david-b@pacbell.net

[PATCH] USB: DocBook/usb.tmpl patch

remove duplicated word, fix an unclear implication.


diff -Nru a/Documentation/DocBook/usb.tmpl b/Documentation/DocBook/usb.tmpl
--- a/Documentation/DocBook/usb.tmpl	Wed Apr 16 10:49:01 2003
+++ b/Documentation/DocBook/usb.tmpl	Wed Apr 16 10:49:01 2003
@@ -294,7 +294,7 @@
 	<title>The USB Filesystem (usbfs)</title>
 
 	<para>This chapter presents the Linux <emphasis>usbfs</emphasis>.
-	You may prefer to avoid avoid writing new kernel code for your
+	You may prefer to avoid writing new kernel code for your
 	USB driver; that's the problem that usbfs set out to solve.
 	User mode device drivers are usually packaged as applications
 	or libraries, and may use usbfs through some programming library
@@ -355,7 +355,9 @@
 	    configuration files.</emphasis>
 	    Stable identifiers are available, for user mode applications
 	    that want to use them.  HID and networking devices expose
-	    these IDs.
+	    these stable IDs, so that for example you can be sure that
+	    you told the right UPS to power down its second server.
+	    "usbfs" doesn't (yet) expose those IDs.
 	    </para>
 
 	</sect1>

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1056, 2003/04/14 10:17:46-07:00, greg@kroah.com

[PATCH] USB: fix up spin_unlock_irqrestore() issues in previous patch


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c	Wed Apr 16 10:49:33 2003
+++ b/drivers/usb/core/hcd.c	Wed Apr 16 10:49:33 2003
@@ -1283,13 +1283,13 @@
 		if (urb->status != -EINPROGRESS)
 			continue;
 		usb_get_urb (urb);
-		spin_unlock (&hcd_data_lock);
+		spin_unlock_irqrestore (&hcd_data_lock, flags);
 
-		spin_lock (&urb->lock);
+		spin_lock_irqsave (&urb->lock, flags);
 		tmp = urb->status;
 		if (tmp == -EINPROGRESS)
 			urb->status = -ESHUTDOWN;
-		spin_unlock (&urb->lock);
+		spin_unlock_irqrestore (&urb->lock, flags);
 
 		/* kick hcd unless it's already returning this */
 		if (tmp == -EINPROGRESS) {

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1062, 2003/04/14 10:25:40-07:00, henning@meier-geinitz.de

[PATCH] USB scanner.c endpoint detection fix

This patch fixes the endpoint numbers. They were numbered from 1 to n
but that assumption is not correct in all cases.


diff -Nru a/drivers/usb/image/scanner.c b/drivers/usb/image/scanner.c
--- a/drivers/usb/image/scanner.c	Wed Apr 16 10:48:55 2003
+++ b/drivers/usb/image/scanner.c	Wed Apr 16 10:48:55 2003
@@ -355,6 +355,10 @@
  *      is closed and disconnected. Avoids crashes when writing to a 
  *      disconnected device. (Thanks to Greg KH).
  *
+ * 0.4.12  2003-04-11
+ *    - Fixed endpoint detection. The endpoints were numbered from 1 to n but
+ *      that assumption is not correct in all cases.
+ *
  * TODO
  *    - Performance
  *    - Select/poll methods
@@ -957,7 +961,7 @@
 				info ("probe_scanner: ignoring additional bulk_in_ep:%d", ep_cnt);
 				continue;
 			}
-			have_bulk_in = ep_cnt;
+			have_bulk_in = endpoint->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
 			dbg("probe_scanner: bulk_in_ep:%d", have_bulk_in);
 			continue;
 		}
@@ -968,7 +972,7 @@
 				info ("probe_scanner: ignoring additional bulk_out_ep:%d", ep_cnt);
 				continue;
 			}
-			have_bulk_out = ep_cnt;
+			have_bulk_out = endpoint->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
 			dbg("probe_scanner: bulk_out_ep:%d", have_bulk_out);
 			continue;
 		}
@@ -979,7 +983,7 @@
 				info ("probe_scanner: ignoring additional intr_ep:%d", ep_cnt);
 				continue;
 			}
-			have_intr = ep_cnt;
+			have_intr = endpoint->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
 			dbg("probe_scanner: intr_ep:%d", have_intr);
 			continue;
 		}

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1059, 2003/04/14 10:21:36-07:00, greg@kroah.com

[PATCH] USB: kl5kusb105 fix up errors found by smatch


diff -Nru a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
--- a/drivers/usb/serial/kl5kusb105.c	Wed Apr 16 10:49:14 2003
+++ b/drivers/usb/serial/kl5kusb105.c	Wed Apr 16 10:49:14 2003
@@ -964,8 +964,9 @@
 	     if (retval)
 			 return(retval);
 
-	     kernel_termios_to_user_termios((struct termios *)arg,  
-					    &priv->termios);
+	     if (kernel_termios_to_user_termios((struct termios *)arg,  
+						&priv->termios))
+		     return -EFAULT;
 	     return(0);
 	     }
 	case TCSETS: {
@@ -980,8 +981,9 @@
 		if (retval)
 			    return(retval);
 
-		user_termios_to_kernel_termios(&priv->termios,
-					       (struct termios *)arg);
+		if (user_termios_to_kernel_termios(&priv->termios,
+						  (struct termios *)arg))
+			return -EFAULT;
 		klsi_105_set_termios(port, &priv->termios);
 		return(0);
 	     }

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1060, 2003/04/14 10:22:36-07:00, greg@kroah.com

[PATCH] USB: kobil_sct fix up errors found by smatch


diff -Nru a/drivers/usb/serial/kobil_sct.c b/drivers/usb/serial/kobil_sct.c
--- a/drivers/usb/serial/kobil_sct.c	Wed Apr 16 10:49:08 2003
+++ b/drivers/usb/serial/kobil_sct.c	Wed Apr 16 10:49:08 2003
@@ -514,7 +514,9 @@
 			dbg("%s - port %d Error in verify_area", __FUNCTION__, port->number);
 			return(result);
 		}
-		kernel_termios_to_user_termios((struct termios *)arg, &priv->internal_termios);
+		if (kernel_termios_to_user_termios((struct termios *)arg,
+						   &priv->internal_termios))
+			return -EFAULT;
 		return 0;
 
 	case TCSETS:   // 0x5402
@@ -527,7 +529,9 @@
 			dbg("%s - port %d Error in verify_area", __FUNCTION__, port->number);
 			return result;
 		}
-		user_termios_to_kernel_termios( &priv->internal_termios, (struct termios *)arg);
+		if (user_termios_to_kernel_termios(&priv->internal_termios,
+						   (struct termios *)arg))
+			return -EFAULT;
 		
 		settings = (unsigned char *) kmalloc(50, GFP_KERNEL);  
 		if (! settings) {

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1057, 2003/04/14 10:18:56-07:00, greg@kroah.com

[PATCH] USB: add better check to prevent oops in hcd_unlink_urb()


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c	Wed Apr 16 10:49:27 2003
+++ b/drivers/usb/core/hcd.c	Wed Apr 16 10:49:27 2003
@@ -1229,7 +1229,7 @@
 	spin_unlock (&hcd_data_lock);
 	spin_unlock_irqrestore (&urb->lock, flags);
 bye:
-	if (retval && sys)
+	if (retval && sys && sys->driver)
 		dev_dbg (sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
 	return retval;
 }

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1058, 2003/04/14 10:20:28-07:00, david-b@pacbell.net

[PATCH] disconnect cleanup, new HCD callback

Attached, find a patch that "ought to" teach OHCI how to do that
cleanup, by implementing the new callback.  (And the first half
is the patch you applied, with that irqsave tweak -- so you should
already have it.)


diff -Nru a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
--- a/drivers/usb/host/ohci-hcd.c	Wed Apr 16 10:49:20 2003
+++ b/drivers/usb/host/ohci-hcd.c	Wed Apr 16 10:49:20 2003
@@ -313,65 +313,56 @@
  */
 
 static void
-ohci_free_config (struct usb_hcd *hcd, struct usb_device *udev)
+ohci_endpoint_disable (struct usb_hcd *hcd, struct hcd_dev *dev, int ep)
 {
 	struct ohci_hcd		*ohci = hcd_to_ohci (hcd);
-	struct hcd_dev		*dev = (struct hcd_dev *) udev->hcpriv;
-	int			i;
+	int			epnum = ep & USB_ENDPOINT_NUMBER_MASK;
 	unsigned long		flags;
+	struct ed		*ed;
 
-rescan:
-	/* free any eds, and dummy tds, still hanging around */
-	spin_lock_irqsave (&ohci->lock, flags);
-	for (i = 0; i < 32; i++) {
-		struct ed	*ed = dev->ep [i];
+	/* ASSERT:  any requests/urbs are being unlinked */
+	/* ASSERT:  nobody can be submitting urbs for this any more */
 
-		if (!ed)
-			continue;
+	ohci_dbg (ohci, "ep %02x disable\n", ep);
+	epnum <<= 1;
+	if (epnum != 0 && !(ep & USB_DIR_IN))
+		epnum |= 1;
 
-		if (ohci->disabled && ed->state != ED_IDLE)
-			ed->state = ED_IDLE;
-		switch (ed->state) {
-		case ED_UNLINK:		/* wait a frame? */
-			goto do_rescan;
-		case ED_IDLE:		/* fully unlinked */
+rescan:
+	spin_lock_irqsave (&ohci->lock, flags);
+	ed = dev->ep [epnum];
+	if (!ed)
+		goto done;
+
+	if (!HCD_IS_RUNNING (ohci->hcd.state) || ohci->disabled)
+		ed->state = ED_IDLE;
+	switch (ed->state) {
+	case ED_UNLINK:		/* wait for hw to finish? */
+		spin_unlock_irqrestore (&ohci->lock, flags);
+		set_current_state (TASK_UNINTERRUPTIBLE);
+		schedule_timeout (1);
+		goto rescan;
+	case ED_IDLE:		/* fully unlinked */
+		if (list_empty (&ed->td_list)) {
 			td_free (ohci, ed->dummy);
+			ed_free (ohci, ed);
 			break;
-		default:
-			ohci_err (ohci,
-				"dev %s ep%d-%s linked; disconnect() bug?\n",
-				udev->devpath,
-				(i >> 1) & 0x0f, (i & 1) ? "out" : "in");
-
-			/* ED_OPER: some driver disconnect() is broken,
-			 * it didn't even start its unlinks much less wait
-			 * for their completions.
-			 * OTHERWISE:  hcd bug, ed is garbage
-			 *
-			 * ... we can't recycle this memory in either case,
-			 * so just leak it to avoid oopsing.
-			 */
-			continue;
 		}
-		ed_free (ohci, ed);
+		/* else FALL THROUGH */
+	default:
+		/* caller was supposed to have unlinked any requests;
+		 * that's not our job.  can't recover; must leak ed.
+		 */
+		ohci_err (ohci, "ed %p (#%d) state %d%s\n",
+			ed, epnum, ed->state,
+			list_empty (&ed->td_list) ? "" : "(has tds)");
+		td_free (ohci, ed->dummy);
+		break;
 	}
+	dev->ep [epnum] = 0;
+done:
 	spin_unlock_irqrestore (&ohci->lock, flags);
 	return;
-
-do_rescan:
-#ifdef DEBUG
-	/* a driver->disconnect() returned before its unlinks completed? */
-	if (in_interrupt ()) {
-		ohci_warn (ohci,
-			"driver disconnect() bug %s ep%d-%s\n", 
-			udev->devpath,
-			(i >> 1) & 0x0f, (i & 1) ? "out" : "in");
-	}
-#endif
-
-	spin_unlock_irqrestore (&ohci->lock, flags);
-	wait_ms (1);
-	goto rescan;
 }
 
 static int ohci_get_frame (struct usb_hcd *hcd)
diff -Nru a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
--- a/drivers/usb/host/ohci-pci.c	Wed Apr 16 10:49:20 2003
+++ b/drivers/usb/host/ohci-pci.c	Wed Apr 16 10:49:20 2003
@@ -334,7 +334,7 @@
 	 */
 	.urb_enqueue =		ohci_urb_enqueue,
 	.urb_dequeue =		ohci_urb_dequeue,
-	.free_config =		ohci_free_config,
+	.endpoint_disable =	ohci_endpoint_disable,
 
 	/*
 	 * scheduling support
diff -Nru a/drivers/usb/host/ohci-sa1111.c b/drivers/usb/host/ohci-sa1111.c
--- a/drivers/usb/host/ohci-sa1111.c	Wed Apr 16 10:49:20 2003
+++ b/drivers/usb/host/ohci-sa1111.c	Wed Apr 16 10:49:20 2003
@@ -333,7 +333,7 @@
 	 */
 	.urb_enqueue =		ohci_urb_enqueue,
 	.urb_dequeue =		ohci_urb_dequeue,
-	.free_config =		ohci_free_config,
+	.endpoint_disable =	ohci_endpoint_disable,
 
 	/*
 	 * scheduling support
diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c	Wed Apr 16 10:49:20 2003
+++ b/drivers/usb/host/uhci-hcd.c	Wed Apr 16 10:49:20 2003
@@ -2390,7 +2390,6 @@
 
 	.urb_enqueue =		uhci_urb_enqueue,
 	.urb_dequeue =		uhci_urb_dequeue,
-	.free_config =		NULL,
 
 	.get_frame_number =	uhci_hcd_get_frame_number,
 

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1066, 2003/04/14 21:24:10-07:00, greg@kroah.com

[PATCH] USB: io_edgeport: stop unlinking a urb that we don't need to unlink.


diff -Nru a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
--- a/drivers/usb/serial/io_edgeport.c	Wed Apr 16 10:48:29 2003
+++ b/drivers/usb/serial/io_edgeport.c	Wed Apr 16 10:48:29 2003
@@ -962,9 +962,8 @@
 		kfree(urb->transfer_buffer);
 	}
 
-	// Free the command urb
-	usb_unlink_urb (urb);
-	usb_free_urb   (urb);
+	/* Free the command urb */
+	usb_free_urb (urb);
 
 	if (port_paranoia_check (edge_port->port, __FUNCTION__)) {
 		return;

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1063, 2003/04/14 10:26:02-07:00, david-b@pacbell.net

[PATCH] USB: EHCI disconnect cleanup

So here's the EHCI implementation of that new callback,
morphed/simplified from the old "free_config" one (which
is now gone).  It looks almost identical to the OHCI
version, except the dummy TDs work a bit differently.

Again, drivers that clean themselves up in disconnect()
shouldn't notice this change.  I didn't re-test this;
I don't have devices with the other kind of driver.  You
should do so with one of yours (high speed hub and TT).

There are still about half a dozen places in usbcore
and the HCDs that would benefit from using this new
callback, FWIW.  I'd call them non-critical bugfixes
that should wait a bit, while this batch shakes out.


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c	Wed Apr 16 10:48:49 2003
+++ b/drivers/usb/core/hcd.c	Wed Apr 16 10:48:49 2003
@@ -1353,9 +1353,6 @@
 		return -EINVAL;
 	}
 
-	if (hcd->driver->free_config)
-		hcd->driver->free_config (hcd, udev);
-
 	spin_lock_irqsave (&hcd_data_lock, flags);
 	list_del (&dev->dev_list);
 	udev->hcpriv = NULL;
diff -Nru a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
--- a/drivers/usb/core/hcd.h	Wed Apr 16 10:48:48 2003
+++ b/drivers/usb/core/hcd.h	Wed Apr 16 10:48:48 2003
@@ -196,11 +196,6 @@
 					int mem_flags);
 	int	(*urb_dequeue) (struct usb_hcd *hcd, struct urb *urb);
 
-	// frees configuration resources -- allocated as needed during
-	// urb_enqueue, and not freed by urb_dequeue
-	void		(*free_config) (struct usb_hcd *hcd,
-				struct usb_device *dev);
-
 	/* hw synch, freeing endpoint resources that urb_dequeue can't */
 	void 	(*endpoint_disable)(struct usb_hcd *hcd,
 			struct hcd_dev *dev, int bEndpointAddress);
diff -Nru a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
--- a/drivers/usb/host/ehci-hcd.c	Wed Apr 16 10:48:49 2003
+++ b/drivers/usb/host/ehci-hcd.c	Wed Apr 16 10:48:49 2003
@@ -870,80 +870,55 @@
 
 // bulk qh holds the data toggle
 
-static void ehci_free_config (struct usb_hcd *hcd, struct usb_device *udev)
+static void
+ehci_endpoint_disable (struct usb_hcd *hcd, struct hcd_dev *dev, int ep)
 {
-	struct hcd_dev		*dev = (struct hcd_dev *)udev->hcpriv;
 	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
-	int			i;
+	int			epnum;
 	unsigned long		flags;
+	struct ehci_qh		*qh;
 
-	/* ASSERT:  no requests/urbs are still linked (so no TDs) */
+	/* ASSERT:  any requests/urbs are being unlinked */
 	/* ASSERT:  nobody can be submitting urbs for this any more */
 
-	ehci_dbg (ehci, "free_config %s devnum %d\n",
-			udev->devpath, udev->devnum);
+	ehci_dbg (ehci, "ep %02x disable\n", ep);
+	epnum = ep & USB_ENDPOINT_NUMBER_MASK;
+	if (epnum != 0 && (ep & USB_DIR_IN))
+		epnum |= 0x10;
 
+rescan:
 	spin_lock_irqsave (&ehci->lock, flags);
-	for (i = 0; i < 32; i++) {
-		if (dev->ep [i]) {
-			struct ehci_qh		*qh;
-			char			*why;
-
-			/* dev->ep never has ITDs or SITDs */
-			qh = (struct ehci_qh *) dev->ep [i];
-
-			/* detect/report non-recoverable errors */
-			if (in_interrupt ()) 
-				why = "disconnect() didn't";
-			else if ((qh->hw_info2 & cpu_to_le32 (0xffff)) != 0
-					&& qh->qh_state != QH_STATE_IDLE)
-				why = "(active periodic)";
-			else
-				why = 0;
-			if (why) {
-				err ("dev %s-%s ep %d-%s error: %s",
-					hcd_to_bus (hcd)->bus_name,
-					udev->devpath,
-					i & 0xf, (i & 0x10) ? "IN" : "OUT",
-					why);
-				BUG ();
-			}
-
-			dev->ep [i] = 0;
-			if (qh->qh_state == QH_STATE_IDLE)
-				goto idle;
-			ehci_dbg (ehci, "free_config, async ep 0x%02x qh %p",
-					i, qh);
-
-			/* scan_async() empties the ring as it does its work,
-			 * using IAA, but doesn't (yet?) turn it off.  if it
-			 * doesn't empty this qh, likely it's the last entry.
-			 */
-			while (qh->qh_state == QH_STATE_LINKED
-					&& ehci->reclaim
-					&& HCD_IS_RUNNING (ehci->hcd.state)
-					) {
-				spin_unlock_irqrestore (&ehci->lock, flags);
-				/* wait_ms() won't spin, we're a thread;
-				 * and we know IRQ/timer/... can progress
-				 */
-				wait_ms (1);
-				spin_lock_irqsave (&ehci->lock, flags);
-			}
-			if (qh->qh_state == QH_STATE_LINKED)
-				start_unlink_async (ehci, qh);
-			while (qh->qh_state != QH_STATE_IDLE
-					&& ehci->hcd.state != USB_STATE_HALT) {
-				spin_unlock_irqrestore (&ehci->lock, flags);
-				wait_ms (1);
-				spin_lock_irqsave (&ehci->lock, flags);
-			}
-idle:
+	qh = (struct ehci_qh *) dev->ep [epnum];
+	if (!qh)
+		goto done;
+
+	if (!HCD_IS_RUNNING (ehci->hcd.state))
+		qh->qh_state = QH_STATE_IDLE;
+	switch (qh->qh_state) {
+	case QH_STATE_UNLINK:		/* wait for hw to finish? */
+		spin_unlock_irqrestore (&ehci->lock, flags);
+		set_current_state (TASK_UNINTERRUPTIBLE);
+		schedule_timeout (1);
+		goto rescan;
+	case QH_STATE_IDLE:		/* fully unlinked */
+		if (list_empty (&qh->qtd_list)) {
 			qh_put (ehci, qh);
+			break;
 		}
+		/* else FALL THROUGH */
+	default:
+		/* caller was supposed to have unlinked any requests;
+		 * that's not our job.  just leak this memory.
+		 */
+		ehci_err (ehci, "qh %p (#%d) state %d%s\n",
+			qh, epnum, qh->qh_state,
+			list_empty (&qh->qtd_list) ? "" : "(has tds)");
+		break;
 	}
-
+	dev->ep [epnum] = 0;
+done:
 	spin_unlock_irqrestore (&ehci->lock, flags);
+	return;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -978,7 +953,7 @@
 	 */
 	.urb_enqueue =		ehci_urb_enqueue,
 	.urb_dequeue =		ehci_urb_dequeue,
-	.free_config =		ehci_free_config,
+	.endpoint_disable =	ehci_endpoint_disable,
 
 	/*
 	 * scheduling support

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1064, 2003/04/14 10:28:01-07:00, arndt@lin02384n012.mc.schoenewald.de

[PATCH] USB: Patch against unusual_devs.h to enable Pontis SP600


diff -Nru a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
--- a/drivers/usb/storage/unusual_devs.h	Wed Apr 16 10:48:42 2003
+++ b/drivers/usb/storage/unusual_devs.h	Wed Apr 16 10:48:42 2003
@@ -299,6 +299,15 @@
                 US_SC_8070, US_PR_CBI, NULL,
                 US_FL_FIX_INQUIRY ),
 
+/* Enable USB storage access to the MMC/SD and CompactFlash cards inside the
+ * Pontis SP600 MP3 player (entry found on http://www.pontis.de/).
+ */
+UNUSUAL_DEV(  0x09bc, 0x0003, 0x0000, 0x9999,
+		"PONTIS",
+		"SP600",
+		US_SC_SCSI, US_PR_BULK, NULL,
+		US_FL_START_STOP ),
+
 #ifdef CONFIG_USB_STORAGE_ISD200
 UNUSUAL_DEV(  0x05ab, 0x0031, 0x0100, 0x0110,
 		"In-System",

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1067, 2003/04/14 22:12:51-07:00, greg@kroah.com

[PATCH] USB: keyspan: fixed up might_sleep() problems on device close.


diff -Nru a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
--- a/drivers/usb/serial/keyspan.c	Wed Apr 16 10:48:23 2003
+++ b/drivers/usb/serial/keyspan.c	Wed Apr 16 10:48:23 2003
@@ -1534,7 +1534,7 @@
 	this_urb->transfer_buffer_length = sizeof(msg);
 
 	this_urb->dev = serial->dev;
-	if ((err = usb_submit_urb(this_urb, GFP_KERNEL)) != 0) {
+	if ((err = usb_submit_urb(this_urb, GFP_ATOMIC)) != 0) {
 		dbg("%s - usb_submit_urb(setup) failed (%d)", __FUNCTION__, err);
 	}
 #if 0
@@ -1659,7 +1659,7 @@
 	this_urb->transfer_buffer_length = sizeof(msg);
 
 	this_urb->dev = serial->dev;
-	if ((err = usb_submit_urb(this_urb, GFP_KERNEL)) != 0) {
+	if ((err = usb_submit_urb(this_urb, GFP_ATOMIC)) != 0) {
 		dbg("%s - usb_submit_urb(setup) failed", __FUNCTION__);
 	}
 #if 0
@@ -1824,7 +1824,7 @@
 	this_urb->transfer_buffer_length = sizeof(msg);
 
 	this_urb->dev = serial->dev;
-	if ((err = usb_submit_urb(this_urb, GFP_KERNEL)) != 0) {
+	if ((err = usb_submit_urb(this_urb, GFP_ATOMIC)) != 0) {
 		dbg("%s - usb_submit_urb(setup) failed (%d)", __FUNCTION__, err);
 	}
 #if 0

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

ChangeSet 1.1065, 2003/04/14 10:31:35-07:00, baldrick@wanadoo.fr

[PATCH] USB speedtouch: discard packets for non-existant vcc's

I broke part of the udsl_decode_rawcell logic in a previous patch, leading to
possible hangs on startup/shutdown.  I've attached the 2.4 and 2.5 versions.
Thanks to Subodh Srivastava and Ted Phelps for their bug reports.  Here is the
2.5 patch included inline for reference:


diff -Nru a/drivers/usb/misc/speedtch.c b/drivers/usb/misc/speedtch.c
--- a/drivers/usb/misc/speedtch.c	Wed Apr 16 10:48:36 2003
+++ b/drivers/usb/misc/speedtch.c	Wed Apr 16 10:48:36 2003
@@ -278,9 +278,10 @@
 
 		/* here should the header CRC check be... */
 
-		if (!(vcc = udsl_find_vcc (instance, vpi, vci)))
+		if (!(vcc = udsl_find_vcc (instance, vpi, vci))) {
 			dbg ("udsl_decode_rawcell: no vcc found for packet on vpi %d, vci %d", vpi, vci);
-		else {
+			__skb_pull (skb, min (skb->len, (unsigned) 53));
+		} else {
 			dbg ("udsl_decode_rawcell found vcc %p for packet on vpi %d, vci %d", vcc, vpi, vci);
 
 			if (skb->len >= 53) {
@@ -323,8 +324,8 @@
 				skb_pull (skb, 53);
 			} else {
 				/* If data is corrupt and skb doesn't hold a whole cell, flush the lot */
-				if (skb_pull (skb, 53) == NULL)
-					return NULL;
+				__skb_pull (skb, skb->len);
+				return NULL;
 			}
 		}
 	}

Re: [PATCH] More USB fixes for 2.5.67

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 1698 bytes --]

Umm... this patch is only needed on 2.4.x -- 2.5.x doesn't send START_STOP
on it's own anymore, and limiting that command is the only thing this entry
seems to do.

I don't think this should be applied.

Matt

On Wed, Apr 16, 2003 at 11:05:05PM -0700, Greg KH wrote:
> ChangeSet 1.1064, 2003/04/14 10:28:01-07:00, arndt@lin02384n012.mc.schoenewald.de
> 
> [PATCH] USB: Patch against unusual_devs.h to enable Pontis SP600
> 
> 
> diff -Nru a/drivers/usb/storage/unusual_devs.h b/drivers/usb/storage/unusual_devs.h
> --- a/drivers/usb/storage/unusual_devs.h	Wed Apr 16 10:48:42 2003
> +++ b/drivers/usb/storage/unusual_devs.h	Wed Apr 16 10:48:42 2003
> @@ -299,6 +299,15 @@
>                  US_SC_8070, US_PR_CBI, NULL,
>                  US_FL_FIX_INQUIRY ),
>  
> +/* Enable USB storage access to the MMC/SD and CompactFlash cards inside the
> + * Pontis SP600 MP3 player (entry found on http://www.pontis.de/).
> + */
> +UNUSUAL_DEV(  0x09bc, 0x0003, 0x0000, 0x9999,
> +		"PONTIS",
> +		"SP600",
> +		US_SC_SCSI, US_PR_BULK, NULL,
> +		US_FL_START_STOP ),
> +
>  #ifdef CONFIG_USB_STORAGE_ISD200
>  UNUSUAL_DEV(  0x05ab, 0x0031, 0x0100, 0x0110,
>  		"In-System",
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

You should try to see the techs say "three piece suit".
					-- The Chief
User Friendly, 11/23/1997

[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]

Re: [PATCH] More USB fixes for 2.5.67

[Greg KH]

On Thu, Apr 17, 2003 at 08:18:00AM -0700, Matthew Dharm wrote:
> Umm... this patch is only needed on 2.4.x -- 2.5.x doesn't send START_STOP
> on it's own anymore, and limiting that command is the only thing this entry
> seems to do.
> 
> I don't think this should be applied.

Ah, ok, I'll revert it.

Thanks for letting me know.

greg k-h