Re: [PATCH] usb: dwc3: gadget: Correct ISOC DATA PIDs for short packets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 7/18/2017 4:27 PM, Felipe Balbi wrote:
> Hi,
>
> Manu Gautam <mgautam@xxxxxxxxxxxxxx> writes:
>>> Manu Gautam <mgautam@xxxxxxxxxxxxxx> writes:
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index aea9a5b..b81547d 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -854,8 +854,13 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, struct dwc3_trb *trb,
>>>>  			trb->ctrl = DWC3_TRBCTL_ISOCHRONOUS_FIRST;
>>>>  
>>>>  			if (speed == USB_SPEED_HIGH) {
>>>> -				struct usb_ep *ep = &dep->endpoint;
>>>> -				trb->size |= DWC3_TRB_SIZE_PCM1(ep->mult - 1);
>>>> +				unsigned int maxp = usb_endpoint_maxp(
>>>> +							dep->endpoint.desc);
>>>> +				unsigned int rem = length % maxp;
>>>> +				unsigned int mult = (length / maxp) & 0x3;
>>>> +
>>>> +				trb->size |= DWC3_TRB_SIZE_PCM1(
>>>> +						rem ? mult : mult - 1);
>>> Manu, It seems to me like we shouldn't be relying on req->length. Which
>>> gadget driver are you using to test this?
>> f_uvc function is used.
>> In bus analyzer logs there are DATA2, DATA1 PIDs even for a 2K byte TRB
>> (also last packet of the video frame are always less than maxpacket size).
> Understood, yeah it makes sense, although I think your patch can be
> simplified. Seems to me that it should be enough to set PCM1 to
> req->length / usb_endpoint_maxp(), no?

Still need to take care of two things:
1. Handle case if If req>length is more than 3K (buggy function driver)
2. We don't need to send extra packet for isoc if length is multiple of maxp.
Hence, remainder must be checked.

> Or, if we want to make use of ep->mult, we could:
>
> 	unsigned int mult = ep->mult - 1;
>
> 	if (req->length < (usb_endpoint_maxp() << 1))
> 		mult--;

I think it should be <=
E.g. for 2k size only two transfers should take place)


> 	if (req->length < usb_endpoint_maxp())
> 		mult--;
<=

> 	trb->size |= DWC3_TRB_SIZE_PCM1(mult);
>
> how about that?
>

This also looks fine and I can send the updated patch.
--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux