* Felipe Balbi | 2011-11-22 11:42:06 [+0200]:
>diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>index c04a0d3..c9bdada 100644
>--- a/drivers/usb/dwc3/gadget.c
>+++ b/drivers/usb/dwc3/gadget.c
>@@ -539,6 +539,83 @@ static void dwc3_gadget_ep_free_request(struct usb_ep *ep,
> kfree(req);
> }
>
>+static int dwc3_gadget_ep_sg_prepare(struct usb_ep *ep,
>+ struct usb_request_sg *rsg, struct scatterlist *sg,
>+ unsigned int num_entries, unsigned int length,
>+ gfp_t gfp_flags)
>+{
>+ struct dwc3_ep *dep = to_dwc3_ep(ep);
>+ int ret = -ENOMEM;
>+ int i;
>+
>+ if (!rsg || !sg || num_entries == 0)
>+ return -EINVAL;
>+
>+ rsg->requests = kcalloc(num_entries, sizeof(*rsg->requests), gfp_flags);
>+ if (!rsg->requests)
>+ goto err0;
>+
>+ for_each_sg(sg, sg, rsg->num_entries, i) {
>+ struct usb_request *request;
>+ unsigned len;
>+
>+ request = usb_ep_alloc_request(ep, gfp_flags);
>+ if (!request) {
>+ rsg->num_entries = i;
>+ goto err1;
>+ }
>+
>+ len = length;
>+ if (len == 0) {
>+ struct scatterlist *sg2;
>+ int j;
>+
>+ for_each_sg(sg, sg2, num_entries, j)
>+ len += sg2->length;
>+ }
Why do iterate twice in case length is zero? And we do we have multiple
requests with the same size then?
What imagine for sg supportis th follwing: if the udc supports sg we can
queue sg-based requests which have buf = NULL. A request which we
receive could look like: req->length = 8192 and req->num_sgs = 3.
And the sgs are:
sg 1 starts at mem_page_offset + 2048 len 2048
sg 2 starts at mem_page_offset + 0 len 4096
sg 3 starts at mem_page_offset + 0 len 2048
Those three transfers should be part of _one_ request. Therefore you
don't need this no_interrupt field or anything else like that because
you call ->complete() once the whole thing is through. With one struct
usb_request only.
If you queue 2 usb_request struct and the gadget can wait with its
notification after the second is compled, this is another story.
>+
>+ request->no_interrupt = true;
>+ request->buf = NULL;
>+ request->num_sgs = num_entries;
>+
>+ request->length = len;
>+
>+ if (usb_endpoint_dir_in(dep->desc))
>+ request->short_not_ok = true;
Why is this for you to decice? Shouldn't the gadget take care of this?
>+ rsg->requests[i] = request;
>+
>+ /* FIXME allocate a TRB, prepare TRB, set CHN bit, etc */
We have 16 or 32 TRBs waiting. All we need to do prepare then and set
the CHN bit.
>+ }
>+
>+ /* we want interrupt on the last request */
>+ rsg->requests[--i]->no_interrupt = false;
>+
>+ rsg->num_entries = num_entries;
>+ rsg->status = 0;
>+ rsg->actual = 0;
>+
>+ return 0;
>+
>+err1:
>+ kfree(rsg->requests);
>+
>+err0:
>+ return ret;
>+}
>+
Sebastian
--
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
