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 <st...@rowland.harvard.edu>
> Reported-and-tested-by: "Aurélien Leblond" <blabl...@gmail.com>
> CC: <sta...@vger.kernel.org>
>
> ---
>
> 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 majord...@vger.kernel.org
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
