[PATCH 1/2] usb: Fix usb_xfer_time() to work when called in 16bit mode.

[Kevin O'Connor]

Make sure to wrap accesses to the usb_pipe struct with GET_LOWFLAT so
that it works in 16bit mode.  This bug impacts both ehci and uhci usb
controllers (it should not impact ohci and xhci as those never call
the function in 16bit mode).

Signed-off-by: Kevin O'Connor 
---
 src/hw/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hw/usb.c b/src/hw/usb.c
index 46b46be..930b5d9 100644
--- a/src/hw/usb.c
+++ b/src/hw/usb.c
@@ -195,7 +195,7 @@ usb_xfer_time(struct usb_pipe *pipe, int datalen)
     // set_address commands where we don't want to stall the boot if
     // the device doesn't actually exist.  Add 100ms to account for
     // any controller delays.
-    if (!pipe->devaddr)
+    if (!GET_LOWFLAT(pipe->devaddr))
         return USB_TIME_STATUS + 100;
     return USB_TIME_COMMAND + 100;
 }

[PATCH 2/2] xhci: Call usb_desc2pipe() on xhci_update_pipe().

[Kevin O'Connor]

Make sure to call usb_desc2pipe() when updating a pipe settings.  This
ensures that pipe->devaddr is properly updated.

Signed-off-by: Kevin O'Connor 
---
 src/hw/usb-xhci.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c
index e3052a5..bbf51c2 100644
--- a/src/hw/usb-xhci.c
+++ b/src/hw/usb-xhci.c
@@ -995,29 +995,32 @@ xhci_update_pipe(struct usbdevice_s *usbdev, struct usb_pipe *upipe
     if (!CONFIG_USB_XHCI)
         return NULL;
     u8 eptype = epdesc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
+    int oldmaxpacket = upipe->maxpacket;
+    usb_desc2pipe(upipe, usbdev, epdesc);
     struct xhci_pipe *pipe = container_of(upipe, struct xhci_pipe, pipe);
     struct usb_xhci_s *xhci = container_of(
         pipe->pipe.cntl, struct usb_xhci_s, usb);
     dprintf(3, "%s: usbdev %p, ring %p, slotid %d, epid %d\n", __func__,
             usbdev, &pipe->reqs, pipe->slotid, pipe->epid);
-    if (eptype == USB_ENDPOINT_XFER_CONTROL &&
-        pipe->pipe.maxpacket != epdesc->wMaxPacketSize) {
-        dprintf(1, "%s: reconf ctl endpoint pkt size: %d -> %d\n",
-                __func__, pipe->pipe.maxpacket, epdesc->wMaxPacketSize);
-        pipe->pipe.maxpacket = epdesc->wMaxPacketSize;
-        struct xhci_inctx *in = xhci_alloc_inctx(usbdev, 1);
-        if (!in)
-            return upipe;
-        in->add = (1 << 1);
-        struct xhci_epctx *ep = (void*)&in[2 << xhci->context64];
-        ep->ctx[1] |= (pipe->pipe.maxpacket << 16);
-        int cc = xhci_cmd_evaluate_context(xhci, pipe->slotid, in);
-        if (cc != CC_SUCCESS) {
-            dprintf(1, "%s: reconf ctl endpoint: failed (cc %d)\n",
-                    __func__, cc);
-        }
-        free(in);
+    if (eptype != USB_ENDPOINT_XFER_CONTROL || upipe->maxpacket == oldmaxpacket)
+        return upipe;
+
+    // maxpacket has changed on control endpoint - update controller.
+    dprintf(1, "%s: reconf ctl endpoint pkt size: %d -> %d\n",
+            __func__, oldmaxpacket, pipe->pipe.maxpacket);
+    struct xhci_inctx *in = xhci_alloc_inctx(usbdev, 1);
+    if (!in)
+        return upipe;
+    in->add = (1 << 1);
+    struct xhci_epctx *ep = (void*)&in[2 << xhci->context64];
+    ep->ctx[1] |= (pipe->pipe.maxpacket << 16);
+    int cc = xhci_cmd_evaluate_context(xhci, pipe->slotid, in);
+    if (cc != CC_SUCCESS) {
+        dprintf(1, "%s: reconf ctl endpoint: failed (cc %d)\n",
+                __func__, cc);
     }
+    free(in);
+
     return upipe;
 }

[PATCH 2/2] xhci: Call usb_desc2pipe() on xhci_update_pipe().

[Matt DeVillier]

+1 on both patches (or the two together at least)

On 9/9/2014 6:35 PM, Kevin O'Connor wrote:
> Make sure to call usb_desc2pipe() when updating a pipe settings.  This
> ensures that pipe->devaddr is properly updated.
> Signed-off-by: Kevin O'Connor
>
> src/hw/usb-xhci.c | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c
> index e3052a5..bbf51c2 100644
> --- a/src/hw/usb-xhci.c
> +++ b/src/hw/usb-xhci.c
> @@ -995,29 +995,32 @@ xhci_update_pipe(struct usbdevice_s *usbdev, struct usb_pipe *upipe
>      if (!CONFIG_USB_XHCI)
>          return NULL;
>      u8 eptype = epdesc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> int oldmaxpacket = upipe->maxpacket;
> usb_desc2pipe(upipe, usbdev, epdesc);
> struct xhci_pipe *pipe = container_of(upipe, struct xhci_pipe, pipe);
> struct usb_xhci_s *xhci = container_of(
>     pipe->pipe.cntl, struct usb_xhci_s, usb);
> dprintf(3, "%s: usbdev %p, ring %p, slotid %d, epid %d\n", __func__,
>         usbdev, &pipe->reqs, pipe->slotid, pipe->epid);
> if (eptype == USB_ENDPOINT_XFER_CONTROL &&
>    pipe->pipe.maxpacket != epdesc->wMaxPacketSize) {
>    dprintf(1, "%s: reconf ctl endpoint pkt size: %d -> %d\n",
>            __func__, pipe->pipe.maxpacket, epdesc->wMaxPacketSize);
>    pipe->pipe.maxpacket = epdesc->wMaxPacketSize;
>    struct xhci_inctx *in = xhci_alloc_inctx(usbdev, 1);
>    if (!in)
>        return upipe;
>    in->add = (1 << 1);
>    struct xhci_epctx *ep = (void*)&in[2 << xhci->context64];
>    ep->ctx[1] |= (pipe->pipe.maxpacket << 16);
>    int cc = xhci_cmd_evaluate_context(xhci, pipe->slotid, in);
>    if (cc != CC_SUCCESS) {
>        dprintf(1, "%s: reconf ctl endpoint: failed (cc %d)\n",
>                __func__, cc);
>    }
>    free(in);
> if (eptype != USB_ENDPOINT_XFER_CONTROL || upipe->maxpacket == oldmaxpacket)
>    return upipe;
> // maxpacket has changed on control endpoint - update controller.
> dprintf(1, "%s: reconf ctl endpoint pkt size: %d -> %d\n",
>        __func__, oldmaxpacket, pipe->pipe.maxpacket);
> struct xhci_inctx *in = xhci_alloc_inctx(usbdev, 1);
> if (!in)
>    return upipe;
> in->add = (1 << 1);
> struct xhci_epctx *ep = (void*)&in[2 << xhci->context64];
> ep->ctx[1] |= (pipe->pipe.maxpacket << 16);
> int cc = xhci_cmd_evaluate_context(xhci, pipe->slotid, in);
> if (cc != CC_SUCCESS) {
>    dprintf(1, "%s: reconf ctl endpoint: failed (cc %d)\n",
>            __func__, cc);
> }
> free(in);
> return upipe;
>
> }