Hi Felipe,
On Wed, May 17, 2017 at 01:31:12PM +0300, Felipe Balbi wrote:
> Some functions might want to have very, very long request queues. We
> can't make any assumptions about how many requests we *are* able to
> map, so instead of mapping requests early, let's map them late. This
> way, functions can queue as many requests as they'd like but we won't
> take DMA resources until they are needed.
>
> Also, we can now stop processing requests when we run out of DMA
> resources but still keep requests in the queue for late processing.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> ---
>
> Tested with g_mass_storage using 1024 requests. Note that we have
> space for 255 TRBs in our ring. TRB #256 is our link TRB back to
> the head of the ring.
>
> drivers/usb/dwc3/gadget.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 7113a9f53ca9..750364eb11e1 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1099,6 +1099,17 @@ static void dwc3_prepare_trbs(struct dwc3_ep *dep)
> }
>
> list_for_each_entry_safe(req, n, &dep->pending_list, list) {
> + struct dwc3 *dwc = dep->dwc;
> + int ret;
> +
> + ret = usb_gadget_map_request_by_dev(dwc->sysdev, &req->request,
> + dep->direction);
> + if (ret)
> + return;
> +
> + req->sg = req->request.sg;
> + req->num_pending_sgs = req->request.num_mapped_sgs;
> +
Consider the case when the request is dequeued or ep_disable() is called
before its TRB was prepared (such as when there aren't any TRBs
available yet), and therefore dma_map_{single,sg}() hasn't been called
yet. Wouldn't then dwc3_gadget_giveback() end up unconditionally calling
dma_unmap_{single,sg}() with an invalid request->dma (or sg) handle?
Depending on the platform's DMA implementation the unmap operation could
result in a panic.
I'm wondering if we should try to fix this more generally, that is allow
usb_gadget_unmap_request_by_dev() to know not to call dma_unmap_*()
unless it was peviously mapped. The host core does this already by way
of flags such as URB_DMA_MAP_SINGLE and URB_DMA_MAP_SG which are applied
to urb->transfer_flags and used in the urb map/unmap routines in
hcd.c. Perhaps we can add something similar to udc/core.c?
Thanks,
Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
