Hi,
John Youn <John.Youn@xxxxxxxxxxxx> writes:
> Hi Felipe,
>
> On 8/15/2016 4:02 AM, Felipe Balbi wrote:
>> Upon transfer completion after a full ring, let's
>> add more TRBs to our ring in order to complete our
>> request successfully.
>>
>> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
>> ---
>> drivers/usb/dwc3/gadget.c | 36 ++++++++++++++++++++++++------------
>> 1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 90b3d7965136..6483991c8013 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -888,14 +888,13 @@ static u32 dwc3_calc_trbs_left(struct dwc3_ep *dep)
>> static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
>> struct dwc3_request *req, unsigned int trbs_left)
>> {
>> - struct usb_request *request = &req->request;
>> - struct scatterlist *sg = request->sg;
>> + struct scatterlist *sg = req->sg;
>> struct scatterlist *s;
>> unsigned int length;
>> dma_addr_t dma;
>> int i;
>>
>> - for_each_sg(sg, s, request->num_mapped_sgs, i) {
>> + for_each_sg(sg, s, req->num_pending_sgs, i) {
>> unsigned chain = true;
>>
>> length = sg_dma_len(s);
>> @@ -945,7 +944,7 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
>> return;
>>
>> list_for_each_entry_safe(req, n, &dep->pending_list, list) {
>> - if (req->request.num_mapped_sgs > 0)
>> + if (req->num_pending_sgs > 0)
>> dwc3_prepare_one_trb_sg(dep, req, trbs_left--);
>> else
>> dwc3_prepare_one_trb_linear(dep, req, trbs_left--);
>> @@ -1071,6 +1070,9 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req)
>> if (ret)
>> return ret;
>>
>> + req->sg = req->request.sg;
>> + req->num_pending_sgs = req->request.num_mapped_sgs;
>> +
>> list_add_tail(&req->list, &dep->pending_list);
>>
>> if (usb_endpoint_xfer_isoc(dep->endpoint.desc) &&
>> @@ -1935,22 +1937,30 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, struct dwc3_ep *dep,
>> int ret;
>>
>> list_for_each_entry_safe(req, n, &dep->started_list, list) {
>> -
>> + unsigned length;
>> + unsigned actual;
>> int chain;
>>
>> - chain = req->request.num_mapped_sgs > 0;
>> + length = req->request.length;
>> + chain = req->num_pending_sgs > 0;
>> if (chain) {
>> - struct scatterlist *sg = req->request.sg;
>> + struct scatterlist *sg = req->sg;
>> struct scatterlist *s;
>> + unsigned int pending = req->num_pending_sgs;
>> unsigned int i;
>>
>> - for_each_sg(sg, s, req->request.num_mapped_sgs, i) {
>> + for_each_sg(sg, s, pending, i) {
>> trb = &dep->trb_pool[dep->trb_dequeue];
>> count += trb->size & DWC3_TRB_SIZE_MASK;
>> dwc3_ep_inc_deq(dep);
>>
>> + req->sg = sg_next(s);
>> + req->num_pending_sgs--;
>> +
>> ret = __dwc3_cleanup_done_trbs(dwc, dep, req, trb,
>> event, status, chain);
>> + if (ret)
>> + break;
>> }
>> } else {
>> trb = &dep->trb_pool[dep->trb_dequeue];
>> @@ -1968,11 +1978,13 @@ static int dwc3_cleanup_done_reqs(struct dwc3 *dwc, struct dwc3_ep *dep,
>> * should receive and we simply bounce the request back to the
>> * gadget driver for further processing.
>> */
>> - req->request.actual += req->request.length - count;
>> - dwc3_gadget_giveback(dep, req, status);
>> + actual = length - req->request.actual;
>> + req->request.actual = actual;
>>
>> - if (ret)
>> - break;
>> + if (ret && chain && (actual < length) && req->num_pending_sgs)
>> + return __dwc3_gadget_kick_transfer(dep, 0);
>> +
>> + dwc3_gadget_giveback(dep, req, status);
>> }
>>
>> /*
>>
>
> On your testing/next, I see considerable slow down and file
> corruption in mass storage.
>
> After bisecting, this patch seems to be the first one that shows
> problems. The device fails even enumeration here.
>
> I haven't looked in detail at the changes but, do you have any ideas?
>
> I have attached driver logs.
g_mass_storage doesn't use sglists, so I find this to be unlikely. I'll
test again but I didn't notice any problems on my side.
--
balbi
Attachment:
signature.asc
Description: PGP signature
