Hi,

John Youn <john.y...@synopsys.com> 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.ba...@linux.intel.com>
>> ---
>>  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

Reply via email to