Re: [PATCH] More USB fixes for 2.5.67

Greg KH (greg@kroah.com)
Wed, 16 Apr 2003 23:05:05 -0700


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

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