The previous patch (as1197), to not reset the data toggle during driver
unbinding when an interface is already in altsetting 0, has introduced an
intermittent loss of packets due to data toggle mismatch. Specifically, the
ehci and ohci drivers do not use the device toggle field to actually track the
current toggle; they use hardware to track the device toggle. So avoiding the
call to usb_settoggle() in usb_endpoint_enable() isn't enough.
I think there is an easy way to fix this, and a hard way. The hard way (and
more correct way IMHO) is to make the data toggles HCD-internal, which is what
they really should be. The only interface that drivers or the USB core needs is
the ability to reset the data toggle after a set interface or a clear halt.
This would be a relatively large change, affecting all the HC drivers, the USB
core, and many of the USB device drivers.
The easy way is what the patch below does. Currently, dev->toggle has 2
different meanings:
-For uhci, it is the actual toggle value
-For ehci and ohci, it is a flag to indicate if the toggle should be reset
This patch changes it to have 3 meanings:
-For uhci, it is the actual toggle value
-For ehci and ohci while there is a qh or ed present for an endpoint, it is a
flag to indicate if the toggle should be reset
-For ehci and ohci when the endpoint is disabled, it stores the last toggle
value, which is restored when the endpoint is re-enabled and a new qh or ed is
created for the endpoint. If the toggle really should be reset, it happens
normally during the call to usb_enable_endpoint().
I don't know enough about the other HCD drivers to say if they use hardware
toggling or not. If they do, then they will need to similarly keep track of the
last toggle or there will occasionally be a lost packet due to toggle mismatch.
Signed-off-by: Dan Streetman <ddstreet@xxxxxxxx>
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 4725d15..7c8545f 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -927,6 +927,7 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
unsigned long flags;
struct ehci_qh *qh, *tmp;
+ int epnum, is_out, toggle;
/* ASSERT: any requests/urbs are being unlinked */
/* ASSERT: nobody can be submitting urbs for this any more */
@@ -937,6 +938,11 @@ rescan:
if (!qh)
goto done;
+ epnum = ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
+ is_out = !(ep->desc.bEndpointAddress & USB_DIR_IN);
+ toggle = (qh->hw_token & cpu_to_hc32 (ehci, QTD_TOGGLE)) != 0;
+ usb_settoggle(qh->dev, epnum, is_out, toggle);
+
/* endpoints can be iso streams. for now, we don't
* accelerate iso completions ... so spin a while.
*/
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 3712b92..e09bef3 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -819,6 +819,8 @@ done:
qh->qh_state = QH_STATE_IDLE;
qh->hw_info1 = cpu_to_hc32(ehci, info1);
qh->hw_info2 = cpu_to_hc32(ehci, info2);
+ if (usb_gettoggle (urb->dev, usb_pipeendpoint (urb->pipe), !is_input))
+ qh->hw_token |= cpu_to_hc32(ehci, QTD_TOGGLE);
usb_settoggle (urb->dev, usb_pipeendpoint (urb->pipe), !is_input, 1);
qh_refresh (ehci, qh);
return qh;
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 65a9609..a0cf489 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -314,6 +314,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
unsigned long flags;
struct ed *ed = ep->hcpriv;
unsigned limit = 1000;
+ int epnum, is_out, toggle;
/* ASSERT: any requests/urbs are being unlinked */
/* ASSERT: nobody can be submitting urbs for this any more */
@@ -332,6 +333,11 @@ sanitize:
finish_unlinks (ohci, 0);
}
+ epnum = ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
+ is_out = !(ep->desc.bEndpointAddress & USB_DIR_IN);
+ toggle = (ed->hwHeadP & cpu_to_hc32(ohci, ED_C)) != 0;
+ usb_settoggle(ed->dev, epnum, is_out, toggle);
+
switch (ed->state) {
case ED_UNLINK: /* wait for hw to finish? */
/* major IRQ delivery trouble loses INTR_SF too... */
diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
index 2f20d3d..d4ecacf 100644
--- a/drivers/usb/host/ohci-mem.c
+++ b/drivers/usb/host/ohci-mem.c
@@ -133,6 +133,8 @@ ed_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
static void
ed_free (struct ohci_hcd *hc, struct ed *ed)
{
+ if (ed->dev)
+ usb_put_dev (ed->dev);
dma_pool_free (hc->ed_cache, ed, ed->dma);
}
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index c2d80f8..201222b 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -393,7 +393,7 @@ static struct ed *ed_get (
if (!(ed = ep->hcpriv)) {
struct td *td;
- int is_out;
+ int epnum, is_out;
u32 info;
ed = ed_alloc (ohci, GFP_ATOMIC);
@@ -415,8 +415,13 @@ static struct ed *ed_get (
ed->hwHeadP = ed->hwTailP; /* ED_C, ED_H zeroed */
ed->state = ED_IDLE;
+ epnum = ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
is_out = !(ep->desc.bEndpointAddress & USB_DIR_IN);
+ if (usb_gettoggle(udev, epnum, is_out))
+ ed->hwHeadP |= cpu_to_hc32(ohci, ED_C);
+ usb_settoggle(udev, epnum, is_out, 1);
+
/* FIXME usbcore changes dev->devnum before SET_ADDRESS
* suceeds ... otherwise we wouldn't need "pipe".
*/
@@ -446,6 +451,8 @@ static struct ed *ed_get (
}
ed->hwINFO = cpu_to_hc32(ohci, info);
+ ed->dev = usb_get_dev(udev);
+
ep->hcpriv = ed;
}
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 222011f..16933a9 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -66,6 +66,9 @@ struct ed {
/* HC may see EDs on rm_list until next frame (frame_no == tick) */
u16 tick;
+
+ /* Access to USB device (for toggle) */
+ struct usb_device *dev;
} __attribute__ ((aligned(16)));
#define ED_MASK ((u32)~0x0f) /* strip hw status in low addr bits */
--
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
