Hi, Felipe Balbi <[email protected]> writes: > Thinh Nguyen <[email protected]> writes: > >> Hi Felipe, >> >> On 9/7/2017 12:16 AM, Felipe Balbi wrote: >>>>>> drivers/usb/dwc3/gadget.c | 8 +++++--- >>>>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>>>> index 9e41605a..6b299c7 100644 >>>>>> --- a/drivers/usb/dwc3/gadget.c >>>>>> +++ b/drivers/usb/dwc3/gadget.c >>>>>> @@ -191,14 +191,16 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, >>>>>> struct dwc3_request *req, >>>>>> >>>>>> req->started = false; >>>>>> list_del(&req->list); >>>>>> - req->trb = NULL; >>>>>> req->remaining = 0; >>>>>> >>>>>> if (req->request.status == -EINPROGRESS) >>>>>> req->request.status = status; >>>>>> >>>>>> - usb_gadget_unmap_request_by_dev(dwc->sysdev, >>>>>> - &req->request, req->direction); >>>>>> + if (req->trb) >>>>> This check does not account for control data transfer. TRBs for ep0 are >>>>> not set to its req->trb. ep0out request needs to be unmapped, otherwise >>>>> device will receive bogus data. >>>>> >>>>> Our internal test showed that the device failed to interpret control >>>>> data from host. I bisected to this patch. >>> >>> what was the testing? How can I reproduce it? >> >> This is a regression. The internal test found the issue when it does a >> 3-stage Control Write Transfer. You can reproduce this issue with just 1 >> out data packet of size > 0. Read and check the data on control request >> completion. > > okay, is this enough to fix the problem for you? > > modified drivers/usb/dwc3/ep0.c > @@ -48,6 +48,9 @@ static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep, > dwc = dep->dwc; > trb = &dwc->ep0_trb[dep->trb_enqueue]; > > + if (!req->trb) > + req->trb = trb; > + > if (chain) > dep->trb_enqueue++;
sorry, no. this is totally wrong :-) Here's a better version:
modified drivers/usb/dwc3/ep0.c
@@ -990,6 +990,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
DWC3_TRBCTL_CONTROL_DATA,
true);
+ req->trb = &dwc->ep0_trb[dep->trb_enqueue - 1];
+
/* Now prepare one extra TRB to align transfer size */
dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
maxpacket - rem,
@@ -1015,6 +1017,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
DWC3_TRBCTL_CONTROL_DATA,
true);
+ req->trb = &dwc->ep0_trb[dep->trb_enqueue - 1];
+
/* Now prepare one extra TRB to align transfer size */
dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
0, DWC3_TRBCTL_CONTROL_DATA,
@@ -1029,6 +1033,9 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
dwc3_ep0_prepare_one_trb(dep, req->request.dma,
req->request.length, DWC3_TRBCTL_CONTROL_DATA,
false);
+
+ req->trb = &dwc->ep0_trb[dep->trb_enqueue];
+
ret = dwc3_ep0_start_trans(dep);
}
(didn't even compile test)
--
balbi
signature.asc
Description: PGP signature
