The PIDs for Isochronous data transfers are incorrect for high bandwidth IN endpoints when the request length is less than EP wMaxPacketSize. As per spec correct PIDs for ISOC data transfers are: ->For request length <= wMaxPacketSize - DATA0, ->For wMaxPacketSize < length <= (2 * wMaxPacketSize) - DATA1,DATA0 ->For (2 * wMaxPacketSize) < length <= (3 * wMaxPacketSize) - DATA2, DATA1, DATA0.
But driver always sets PCM fields based on wMaxPacketSize due to which DATA2 happens even for small requests. Fix this by setting the PCM field of trb->size depending on request length rather than fixing it to the value depending on wMaxPacketSize. Ideally it shouldn't give any issues as dwc3 will send 0-length packet for next IN token if host sends (even after receiving a short packet). Windows seems to ignore this but with MacOS frame loss observed when using f_uvc. Signed-off-by: Manu Gautam <mgau...@codeaurora.org> --- Changes for v2: - Simplify and add comment as per Felipe's suggestion diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 9e41605a..c0d6c54 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -894,9 +894,41 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb, if (!node) { trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST; + /* + * USB Specification 2.0 Section 5.9.2 states that: "If + * there is only a single transaction in the microframe, + * only a DATA0 data packet PID is used. If there are + * two transactions per microframe, DATA1 is used for + * the first transaction data packet and DATA0 is used + * for the second transaction data packet. If there are + * three transactions per microframe, DATA2 is used for + * the first transaction data packet, DATA1 is used for + * the second, and DATA0 is used for the third." + * + * IOW, we should satisfy the following cases: + * + * 1) length <= wMaxPacketSize + * - DATA0 + * + * 2) wMaxPacketSize < length <= (2 * wMaxPacketSize) + * - DATA1, DATA0 + * + * 3) (2*wMaxPacketSize) < length <= (3*wMaxPacketSize) + * - DATA2, DATA1, DATA0 + */ + if (speed == USB_SPEED_HIGH) { - struct usb_ep *ep = &dep->endpoint; - trb->size |= DWC3_TRB_SIZE_PCM1(ep->mult - 1); + unsigned int mult = dep->endpoint.mult - 1; + unsigned int maxp = usb_endpoint_maxp( + dep->endpoint.desc); + + if (length <= (maxp << 1)) + mult--; + + if (length <= maxp) + mult--; + + trb->size |= DWC3_TRB_SIZE_PCM1(mult); } } else { trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project -- 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