Thanks Alan, I will add this to my for-usb-linus queue. Sarah On Wed, May 08, 2013 at 11:18:05AM -0400, Alan Stern wrote: > This patch shortens the logic in xhci_endpoint_init() by moving common > calculations involving max_packet and max_burst outside the switch > statement, rather than repeating the same code in multiple > case-specific statements. It also replaces two usages of max_packet > which were clearly intended to be max_burst all along. > > More importantly, it compensates for a common bug in high-speed bulk > endpoint descriptors. In many devices there is a bulk endpoint having > a wMaxPacketSize value smaller than 512, which is forbidden by the USB > spec. Some xHCI controllers can't handle this and refuse to accept > the endpoint. This patch changes the max_packet value to 512, which > allows the controller to use the endpoint properly. > > In practice the bogus maxpacket size doesn't matter, because none of > the transfers sent via these endpoints are longer than the maxpacket > value anyway. > > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Reported-and-tested-by: "Aurélien Leblond" <blablack@xxxxxxxxx> > CC: <stable@xxxxxxxxxxxxxxx> > > --- > > Question: Should this be handled in usbcore instead? The code that > parses endpoint descriptors already warns about high-speed bulk > endpoints with maxpacket different from 512. It could easily override > the bogus values. > > Also: This probably won't work in cases where the bogus maxpacket > value is _larger_ than 512. I vaguely recall seeing a device with a > bulk maxpacket size of 1024. But such an endpoint is unlikely to work > under an xHCI controller in any case, although it might work with > EHCI. > > > [as1678] > > drivers/usb/host/xhci-mem.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > Index: usb-3.9/drivers/usb/host/xhci-mem.c > =================================================================== > --- usb-3.9.orig/drivers/usb/host/xhci-mem.c > +++ usb-3.9/drivers/usb/host/xhci-mem.c > @@ -1423,15 +1423,17 @@ int xhci_endpoint_init(struct xhci_hcd * > ep_ctx->ep_info2 |= cpu_to_le32(xhci_get_endpoint_type(udev, ep)); > > /* Set the max packet size and max burst */ > + max_packet = GET_MAX_PACKET(usb_endpoint_maxp(&ep->desc)); > + max_burst = 0; > switch (udev->speed) { > case USB_SPEED_SUPER: > - max_packet = usb_endpoint_maxp(&ep->desc); > - ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet)); > /* dig out max burst from ep companion desc */ > - max_packet = ep->ss_ep_comp.bMaxBurst; > - ep_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(max_packet)); > + max_burst = ep->ss_ep_comp.bMaxBurst; > break; > case USB_SPEED_HIGH: > + /* Some devices get this wrong */ > + if (usb_endpoint_xfer_bulk(&ep->desc)) > + max_packet = 512; > /* bits 11:12 specify the number of additional transaction > * opportunities per microframe (USB 2.0, section 9.6.6) > */ > @@ -1439,17 +1441,16 @@ int xhci_endpoint_init(struct xhci_hcd * > usb_endpoint_xfer_int(&ep->desc)) { > max_burst = (usb_endpoint_maxp(&ep->desc) > & 0x1800) >> 11; > - ep_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(max_burst)); > } > - /* Fall through */ > + break; > case USB_SPEED_FULL: > case USB_SPEED_LOW: > - max_packet = GET_MAX_PACKET(usb_endpoint_maxp(&ep->desc)); > - ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet)); > break; > default: > BUG(); > } > + ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet) | > + MAX_BURST(max_burst)); > max_esit_payload = xhci_get_max_esit_payload(xhci, udev, ep); > ep_ctx->tx_info = cpu_to_le32(MAX_ESIT_PAYLOAD_FOR_EP(max_esit_payload)); > > > -- > 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 -- 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