Hi,
On Wed, Jul 19, 2017 at 7:37 PM, Manu Gautam <mgautam@xxxxxxxxxxxxxx> wrote:
>
> 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 <mgautam@xxxxxxxxxxxxxx>
> ---
> 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
>
Sorry for late response. The change looks fine to me.
BTW, I never get high speed/high bandwidth mode work with Qualcom8953
platform. But I didn't see any issue related with
less length packet. :).
Regards
Yin, Fengwei
--
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
