Re: [PATCH 3/3] usb: dwc3: ep0: improve handling of unaligned OUT requests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7 April 2017 at 13:36, Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> wrote:
> Just like we did for all other endpoint types, let's rely on a chained
> TRB pointing to ep0_bounce_addr in order to align transfer size. This
> will make the code simpler.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> ---
>  drivers/usb/dwc3/core.h   |  6 +----
>  drivers/usb/dwc3/ep0.c    | 65 +++++++++++++----------------------------------
>  drivers/usb/dwc3/gadget.c | 50 ++++++++----------------------------
>  3 files changed, 29 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 2a2673767ccd..e1958f6237af 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -43,7 +43,7 @@
>  #define DWC3_PULL_UP_TIMEOUT   500     /* ms */
>  #define DWC3_ZLP_BUF_SIZE      1024    /* size of a superspeed bulk */
>  #define DWC3_BOUNCE_SIZE       1024    /* size of a superspeed bulk */
> -#define DWC3_EP0_BOUNCE_SIZE   512
> +#define DWC3_EP0_SETUP_SIZE    512
>  #define DWC3_ENDPOINTS_NUM     32
>  #define DWC3_XHCI_RESOURCES_NUM        2
>
> @@ -763,12 +763,10 @@ struct dwc3_scratchpad_array {
>   * struct dwc3 - representation of our controller
>   * @drd_work - workqueue used for role swapping
>   * @ep0_trb: trb which is used for the ctrl_req
> - * @ep0_bounce: bounce buffer for ep0
>   * @zlp_buf: used when request->zero is set
>   * @setup_buf: used while precessing STD USB requests
>   * @ep0_trb: dma address of ep0_trb
>   * @ep0_usb_req: dummy req used while handling STD USB requests
> - * @ep0_bounce_addr: dma address of ep0_bounce
>   * @scratch_addr: dma address of scratchbuf
>   * @ep0_in_setup: one control transfer is completed and enter setup phase
>   * @lock: for synchronizing
> @@ -866,13 +864,11 @@ struct dwc3 {
>         struct work_struct      drd_work;
>         struct dwc3_trb         *ep0_trb;
>         void                    *bounce;
> -       void                    *ep0_bounce;
>         void                    *zlp_buf;
>         void                    *scratchbuf;
>         u8                      *setup_buf;
>         dma_addr_t              ep0_trb_addr;
>         dma_addr_t              bounce_addr;
> -       dma_addr_t              ep0_bounce_addr;
>         dma_addr_t              scratch_addr;
>         struct dwc3_request     ep0_usb_req;
>         struct completion       ep0_in_setup;
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 3ba2309c837f..04249243e4d3 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -836,7 +836,6 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>         struct usb_request      *ur;
>         struct dwc3_trb         *trb;
>         struct dwc3_ep          *ep0;
> -       unsigned                transfer_size = 0;
>         unsigned                maxp;
>         unsigned                remaining_ur_length;
>         void                    *buf;
> @@ -849,9 +848,7 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>         ep0 = dwc->eps[0];
>
>         dwc->ep0_next_event = DWC3_EP0_NRDY_STATUS;
> -
>         trb = dwc->ep0_trb;
> -
>         trace_dwc3_complete_trb(ep0, trb);
>
>         r = next_request(&ep0->pending_list);
> @@ -872,39 +869,17 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
>         remaining_ur_length = ur->length;
>
>         length = trb->size & DWC3_TRB_SIZE_MASK;
> -
>         maxp = ep0->endpoint.maxpacket;
> +       transferred = ur->length - length;
> +       ur->actual += transferred;
>
>         if (dwc->ep0_bounced) {
> -               /*
> -                * Handle the first TRB before handling the bounce buffer if
> -                * the request length is greater than the bounce buffer size
> -                */
> -               if (ur->length > DWC3_EP0_BOUNCE_SIZE) {
> -                       transfer_size = ALIGN(ur->length - maxp, maxp);
> -                       transferred = transfer_size - length;
> -                       buf = (u8 *)buf + transferred;
> -                       ur->actual += transferred;
> -                       remaining_ur_length -= transferred;
> -
> -                       trb++;
> -                       length = trb->size & DWC3_TRB_SIZE_MASK;
> -
> -                       ep0->trb_enqueue = 0;
> -               }
> -
> -               transfer_size = roundup((ur->length - transfer_size),
> -                                       maxp);
> -
> -               transferred = min_t(u32, remaining_ur_length,
> -                                   transfer_size - length);
> -               memcpy(buf, dwc->ep0_bounce, transferred);
> -       } else {
> -               transferred = ur->length - length;
> +               trb++;
> +               trb->ctrl &= ~DWC3_TRB_CTRL_HWO;
> +               ep0->trb_enqueue = 0;
> +               dwc->ep0_bounced = false;
>         }
>
> -       ur->actual += transferred;
> -
>         if ((epnum & 1) && ur->actual < ur->length) {
>                 /* for some reason we did not get everything out */
>
> @@ -1006,8 +981,8 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>                 ret = dwc3_ep0_start_trans(dep);
>         } else if (!IS_ALIGNED(req->request.length, dep->endpoint.maxpacket)
>                         && (dep->number == 0)) {
> -               u32     transfer_size = 0;
>                 u32     maxpacket;
> +               u32     rem;
>
>                 ret = usb_gadget_map_request_by_dev(dwc->sysdev,
>                                 &req->request, dep->number);
> @@ -1015,23 +990,19 @@ static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
>                         return;
>
>                 maxpacket = dep->endpoint.maxpacket;
> -
> -               if (req->request.length > DWC3_EP0_BOUNCE_SIZE) {
> -                       transfer_size = ALIGN(req->request.length - maxpacket,
> -                                             maxpacket);
> -                       dwc3_ep0_prepare_one_trb(dep, req->request.dma,
> -                                                  transfer_size,
> -                                                  DWC3_TRBCTL_CONTROL_DATA,
> -                                                  true);
> -               }
> -
> -               transfer_size = roundup((req->request.length - transfer_size),
> -                                       maxpacket);
> -
> +               rem = req->request.length % maxpacket;
>                 dwc->ep0_bounced = true;
>
> -               dwc3_ep0_prepare_one_trb(dep, dwc->ep0_bounce_addr,
> -                                        transfer_size, DWC3_TRBCTL_CONTROL_DATA,
> +               /* prepare normal TRB */
> +               dwc3_ep0_prepare_one_trb(dep, req->request.dma,
> +                                        req->request.length,
> +                                        DWC3_TRBCTL_CONTROL_DATA,
> +                                        true);
> +
> +               /* Now prepare one extra TRB to align transfer size */
> +               dwc3_ep0_prepare_one_trb(dep, dwc->bounce_addr,
> +                                        maxpacket - rem,
> +                                        DWC3_TRBCTL_CONTROL_DATA,
>                                          false);
>                 ret = dwc3_ep0_start_trans(dep);
>         } else {
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 4787d6f449fa..4dc80729ae11 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -171,7 +171,6 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>                 int status)
>  {
>         struct dwc3                     *dwc = dep->dwc;
> -       unsigned int                    unmap_after_complete = false;
>
>         req->started = false;
>         list_del(&req->list);
> @@ -181,19 +180,8 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>         if (req->request.status == -EINPROGRESS)
>                 req->request.status = status;
>
> -       /*
> -        * NOTICE we don't want to unmap before calling ->complete() if we're
> -        * dealing with a bounced ep0 request. If we unmap it here, we would end
> -        * up overwritting the contents of req->buf and this could confuse the
> -        * gadget driver.
> -        */
> -       if (dwc->ep0_bounced && dep->number <= 1) {
> -               dwc->ep0_bounced = false;
> -               unmap_after_complete = true;
> -       } else {
> -               usb_gadget_unmap_request_by_dev(dwc->sysdev,
> -                               &req->request, req->direction);
> -       }
> +       usb_gadget_unmap_request_by_dev(dwc->sysdev,
> +                                       &req->request, req->direction);
>
>         trace_dwc3_gadget_giveback(req);
>
> @@ -201,10 +189,6 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
>         usb_gadget_giveback_request(&dep->endpoint, &req->request);
>         spin_lock(&dwc->lock);
>
> -       if (unmap_after_complete)
> -               usb_gadget_unmap_request_by_dev(dwc->sysdev,
> -                               &req->request, req->direction);
> -
>         if (dep->number > 1)
>                 pm_runtime_put(dwc->dev);
>  }
> @@ -3153,32 +3137,23 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>                 goto err0;
>         }
>
> -       dwc->setup_buf = kzalloc(DWC3_EP0_BOUNCE_SIZE, GFP_KERNEL);
> +       dwc->setup_buf = kzalloc(DWC3_EP0_SETUP_SIZE, GFP_KERNEL);
>         if (!dwc->setup_buf) {
>                 ret = -ENOMEM;
>                 goto err1;
>         }
>
> -       dwc->ep0_bounce = dma_alloc_coherent(dwc->sysdev,
> -                       DWC3_EP0_BOUNCE_SIZE, &dwc->ep0_bounce_addr,
> -                       GFP_KERNEL);
> -       if (!dwc->ep0_bounce) {
> -               dev_err(dwc->dev, "failed to allocate ep0 bounce buffer\n");
> -               ret = -ENOMEM;
> -               goto err2;
> -       }
> -
>         dwc->zlp_buf = kzalloc(DWC3_ZLP_BUF_SIZE, GFP_KERNEL);
>         if (!dwc->zlp_buf) {
>                 ret = -ENOMEM;
> -               goto err3;
> +               goto err2;
>         }
>
>         dwc->bounce = dma_alloc_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE,
>                         &dwc->bounce_addr, GFP_KERNEL);
>         if (!dwc->bounce) {
>                 ret = -ENOMEM;
> -               goto err4;
> +               goto err3;
>         }
>
>         init_completion(&dwc->ep0_in_setup);
> @@ -3218,7 +3193,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>
>         ret = dwc3_gadget_init_endpoints(dwc, dwc->num_eps);
>         if (ret)
> -               goto err5;
> +               goto err4;
>
>         ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
>         if (ret) {
> @@ -3228,16 +3203,14 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>
>         return 0;
>  err5:
> -       dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
> -                       dwc->bounce_addr);
> +       dwc3_gadget_free_endpoints(dwc);
>
>  err4:
> -       kfree(dwc->zlp_buf);
> +       dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
> +                       dwc->bounce_addr);
>
>  err3:
> -       dwc3_gadget_free_endpoints(dwc);
> -       dma_free_coherent(dwc->sysdev, DWC3_EP0_BOUNCE_SIZE,
> -                       dwc->ep0_bounce, dwc->ep0_bounce_addr);
> +       kfree(dwc->zlp_buf);
>
>  err2:
>         kfree(dwc->setup_buf);
> @@ -3260,9 +3233,6 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
>
>         dma_free_coherent(dwc->sysdev, DWC3_BOUNCE_SIZE, dwc->bounce,
>                         dwc->bounce_addr);
> -       dma_free_coherent(dwc->sysdev, DWC3_EP0_BOUNCE_SIZE,
> -                       dwc->ep0_bounce, dwc->ep0_bounce_addr);
> -
>         kfree(dwc->setup_buf);
>         kfree(dwc->zlp_buf);
>
> --
> 2.11.0.295.gd7dffce1ce
>

Thanks for patches.
Tested this series in my env where have problems with ep0 bounced
buffers (rndis and delayed dma unmap).
Works perfect.

BR
Janusz
--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux