On 1/9/2012 3:12 PM, Felipe Balbi wrote:
> 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.
>
Thanks for your reply.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
