Hi,
On Mon, Jan 09, 2012 at 03:04:17PM +0530, Pavan Kondeti wrote:
> > /*
> > @@ -663,19 +704,58 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep, bool starting)
> > return;
> >
> > list_for_each_entry_safe(req, n, &dep->request_list, list) {
> > - trbs_left--;
> > + unsigned length;
> > + dma_addr_t dma;
> >
> > - if (!trbs_left)
> > - last_one = 1;
> > + if (req->request.num_mapped_sgs > 0) {
> > + struct usb_request *request = &req->request;
> > + struct scatterlist *sg = request->sg;
> > + struct scatterlist *s;
> > + int i;
> >
> > - /* Is this the last request? */
> > - if (list_empty(&dep->request_list))
> > - last_one = 1;
> > + for_each_sg(sg, s, request->num_mapped_sgs, i) {
> > + unsigned chain = true;
> >
> > - dwc3_prepare_one_trb(dep, req, last_one);
> > + length = sg_dma_len(s);
> > + dma = sg_dma_address(s);
> >
> > - if (last_one)
> > - break;
> > + if (i == (request->num_mapped_sgs - 1)
> > + || sg_is_last(s)) {
> > + last_one = true;
> > + chain = false;
> > + }
> > +
> > + trbs_left--;
> > + if (!trbs_left)
> > + last_one = true;
> > +
> > + if (last_one)
> > + chain = false;
> > +
> > + dwc3_prepare_one_trb(dep, req, dma, length,
> > + last_one, chain);
> > +
>
> We are setting LST bit to true for the last trb in the current SG list.
indeed.
> Ideally we should set LST bit true for the last trb in the current
> transfer. That means we should set last_one to true only if this req is
we do both, actually. See the first if (). The issue here is that if we
run out of TRBs, we cannot guarantee that we will be able to allocate
TRBs which will be physically subsequent to the last one we have. So,
the way the code is now, that transfer will be broken, indeed.
What we need to do, is to check from XferComplete whether we have
completed the entire transfer or not. In case we didn't, we start
another transfer for the remaining entries in the transfer. This means
we will have to keep track of completed entries from SG lists.
Patches fixing that are highly welcome, but I would like to see a
testcase which will trigger the issue above. It could be done on g_zero,
for instance.
--
balbi
Attachment:
signature.asc
Description: Digital signature
