Hi Felipe, On 1/9/2018 12:43 AM, Felipe Balbi wrote: > > Hi, > > Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >> Hi, >> >> On 1/8/2018 4:06 AM, Felipe Balbi wrote: >>> >>> Hi, >>> >>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >>>> There are 2 control endpoint structures for DWC3. However, the driver >>>> only updates the OUT direction control endpoint structure during >>>> ConnectDone event. DWC3 driver needs to update the endpoint max packet >>>> size for control IN endpoint as well. If the max packet size is not >>>> properly set, then the driver will incorrectly calculate the data >>>> transfer size and fail to send ZLP for HS/FS 3-stage control read >>>> transfer. >>>> >>>> The fix is simply to update the max packet size for the ep0 IN direction >>>> during ConnectDone event. >>>> >>>> Signed-off-by: Thinh Nguyen <thinhn@xxxxxxxxxxxx> >>>> --- >>>> drivers/usb/dwc3/gadget.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>> index bdf2a2014ebd..6ae924d95cbc 100644 >>>> --- a/drivers/usb/dwc3/gadget.c >>>> +++ b/drivers/usb/dwc3/gadget.c >>>> @@ -2751,6 +2751,8 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc) >>>> break; >>>> } >>>> >>>> + dwc->eps[1]->endpoint.maxpacket = dwc->gadget.ep0->maxpacket; >>> >>> thanks :-) I've had that on my list for a while but never got to it >>> since it has no real effects other than reporting properly on >>> tracepoints :-) >>> >> >> Just to clarify, this is a bug. We found this issue when we test for >> HS/FS 3-stage control read transfer for ZLP. > > interesting... I have never seen such bug before. Got some tracepoints > of the problem? Also, if it's a bug, why wasn't it sent as a separate > patch with a Cc stable and a Fixes tag? > DWC3 gadget starts with max packet size (mps) 512 as default for ep0. That value should be updated after ConnectDone event when DWC3 checks for the connected speed. In HS/FS, the 3-stage control read transfer sends ZLP based on the mps for IN ep0, but that value is not updated. Here's a quick test with the 3-stage HS control read. Setup: * mass_storage driver * Modify driver string description to 128 bytes * Enumerate device in highspeed (mps = 64) Below is the snippet of trace with GetDescriptor(STRING) request for the driver string description. Device did not prepare and send ZLP packet. 41.787092: dwc3_ctrl_req: Get String Descriptor(Index = 4, Length = 255) 41.787093: dwc3_prepare_trb: ep0in: 0/0 trb 0000000070a29ad1 buf 00000000b8d67800 size 128 ctrl 00000c53 (HLcs:SC:data) 41.787094: dwc3_readl: addr 000000008008901e value 00002400 41.787095: dwc3_writel: addr 00000000831c6d00 value 00000000 41.787095: dwc3_writel: addr 000000003e4167b3 value 37820000 41.787095: dwc3_writel: addr 00000000f504c84e value 00000000 41.787095: dwc3_writel: addr 00000000ca003fac value 00000406 41.787095: dwc3_readl: addr 00000000ca003fac value 00000406 41.787098: dwc3_readl: addr 00000000ca003fac value 00010006 41.787100: dwc3_gadget_ep_cmd: ep0in: cmd 'Start Transfer' [1030] params 00000000 37820000 00000000 --> status: Successful 41.787101: dwc3_readl: addr 00000000ca003fac value 00010006 41.787103: dwc3_readl: addr 000000007bcb1278 value 80001000 41.787105: dwc3_writel: addr 000000007bcb1278 value 00001000 41.787107: dwc3_readl: addr 000000008eea6144 value 00000000 41.787216: dwc3_readl: addr 000000008eea6144 value 00000008 41.787218: dwc3_readl: addr 000000007bcb1278 value 00001000 41.787219: dwc3_writel: addr 000000007bcb1278 value 80001000 41.787220: dwc3_writel: addr 000000008eea6144 value 00000008 41.787220: dwc3_event: event (0000c042): ep0in: Transfer Complete [Data Phase] 41.787220: dwc3_complete_trb: ep0out: 0/1 trb 0000000070a29ad1 buf 00000000b8d67800 size 0 ctrl 00000c52 (hLcs:SC:data) 41.787221: dwc3_complete_trb: ep0out: 0/1 trb 00000000cba79c85 buf 0000000000000000 size 0 ctrl 00000000 (hlcs:sc:UNKNOWN) 41.787221: dwc3_gadget_giveback: ep0out: req 00000000fd996780 length 128/128 ZsI ==> 0 41.787221: dwc3_event: event (000010c2): ep0in: Transfer Not Ready (Not Active) [Data Phase] 41.787221: dwc3_readl: addr 000000007bcb1278 value 80001000 41.787223: dwc3_writel: addr 000000007bcb1278 value 00001000 41.787223: dwc3_readl: addr 000000008eea6144 value 00000000 46.294697: dwc3_readl: addr 000000008eea6144 value 00000004 46.294698: dwc3_readl: addr 000000007bcb1278 value 00001000 46.294699: dwc3_writel: addr 000000007bcb1278 value 80001000 46.294700: dwc3_writel: addr 000000008eea6144 value 00000004 Below is the snippet of the trace with the fix: 34.079018: dwc3_ctrl_req: Get String Descriptor(Index = 4, Length = 255) 34.079020: dwc3_prepare_trb: ep0in: 0/0 trb 000000006ece9b4d buf 00000000b8d67800 size 128 ctrl 00000455 (HlCs:Sc:data) 34.079020: dwc3_prepare_trb: ep0in: 0/0 trb 000000001b323efa buf 0000000037821000 size 0 ctrl 00000c53 (HLcs:SC:data) 34.079020: dwc3_readl: addr 00000000a90978f8 value 00002400 34.079021: dwc3_writel: addr 00000000c8f7ea9f value 00000000 34.079021: dwc3_writel: addr 000000000217f085 value 37820000 34.079021: dwc3_writel: addr 00000000b5593ec9 value 00000000 34.079021: dwc3_writel: addr 000000008b9e5492 value 00000406 34.079022: dwc3_readl: addr 000000008b9e5492 value 00000406 34.079024: dwc3_readl: addr 000000008b9e5492 value 00010006 34.079027: dwc3_gadget_ep_cmd: ep0in: cmd 'Start Transfer' [1030] params 00000000 37820000 00000000 --> status: Successful 34.079027: dwc3_readl: addr 000000008b9e5492 value 00010006 34.079029: dwc3_readl: addr 00000000e9897a1c value 80001000 34.079031: dwc3_writel: addr 00000000e9897a1c value 00001000 34.079031: dwc3_readl: addr 0000000034bddbfc value 00000000 34.079144: dwc3_readl: addr 0000000034bddbfc value 00000004 34.079146: dwc3_readl: addr 00000000e9897a1c value 00001000 34.079147: dwc3_writel: addr 00000000e9897a1c value 80001000 34.079148: dwc3_writel: addr 0000000034bddbfc value 00000004 34.079148: dwc3_event: event (0000c042): ep0in: Transfer Complete [Data Phase] 34.079148: dwc3_complete_trb: ep0out: 0/1 trb 000000006ece9b4d buf 00000000b8d67800 size 0 ctrl 00000454 (hlCs:Sc:data) 34.079149: dwc3_complete_trb: ep0out: 0/1 trb 000000001b323efa buf 0000000037821000 size 0 ctrl 00000c52 (hLcs:SC:data) 34.079149: dwc3_gadget_giveback: ep0out: req 00000000bbc62204 length 128/128 ZsI ==> 0 34.079149: dwc3_readl: addr 00000000e9897a1c value 80001000 34.079150: dwc3_writel: addr 00000000e9897a1c value 00001000 I'll "Cc" stable for this patch. Also, I'll separate this patch and "[PATH 11/15] usb: dwc3: ep0: Reset TRB counter for ep0 IN" from this patch series then. BR, Thinh
<<attachment: tracepoints.zip>>
