Hi, Felipe Balbi <balbi@xxxxxxxxxx> writes: > Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> 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
Attachment:
signature.asc
Description: PGP signature
