linux-kernel - Re: [PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMz4kuKE9MaCLFfu9FdBHEfDOGXaXz4ABmOsCoKdqV-BYimtNw@mail.gmail.com>
Date:   Fri, 30 Sep 2016 13:50:23 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Felipe Balbi <balbi@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>
Cc:     Mark Brown <broonie@...nel.org>, USB <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Baolin Wang <baolin.wang@...aro.org>
Subject: Re: [PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when
 stopping gadget

Hi Felipe,

On 19 September 2016 at 19:52, Baolin Wang <baolin.wang@...aro.org> wrote:
> When we change the USB function with configfs dynamically, we possibly met this
> situation: one core is doing the control transfer, another core is trying to
> unregister the USB gadget from userspace, we must wait for completing this
> control tranfer, or it will hang the controller to set the DEVCTRLHLT flag.
>

Any comments about this new version patchset? Thanks.

> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
> ---
>  drivers/usb/dwc3/core.h   |    2 ++
>  drivers/usb/dwc3/ep0.c    |    2 ++
>  drivers/usb/dwc3/gadget.c |   23 +++++++++++++++++++++++
>  3 files changed, 27 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index b2317e7..01a6fbd 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -745,6 +745,7 @@ struct dwc3_scratchpad_array {
>   * @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 tranfer is completed and enter setup phase
>   * @lock: for synchronizing
>   * @dev: pointer to our struct device
>   * @xhci: pointer to our xHCI child
> @@ -843,6 +844,7 @@ struct dwc3 {
>         dma_addr_t              ep0_bounce_addr;
>         dma_addr_t              scratch_addr;
>         struct dwc3_request     ep0_usb_req;
> +       struct completion       ep0_in_setup;
>
>         /* device lock */
>         spinlock_t              lock;
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index fe79d77..06c167a 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -311,6 +311,8 @@ void dwc3_ep0_out_start(struct dwc3 *dwc)
>         ret = dwc3_ep0_start_trans(dwc, 0, dwc->ctrl_req_addr, 8,
>                         DWC3_TRBCTL_CONTROL_SETUP, false);
>         WARN_ON(ret < 0);
> +
> +       complete(&dwc->ep0_in_setup);
>  }
>
>  static struct dwc3_ep *dwc3_wIndex_to_dep(struct dwc3 *dwc, __le16 wIndex_le)
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index ca2ae5b..3a30d51 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1437,6 +1437,15 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>         if (pm_runtime_suspended(dwc->dev))
>                 return 0;
>
> +       /*
> +        * Per databook, when we want to stop the gadget, if a control transfer
> +        * is still in process, complete it and get the core into setup phase.
> +        */
> +       if (!is_on && dwc->ep0state != EP0_SETUP_PHASE) {
> +               reinit_completion(&dwc->ep0_in_setup);
> +               return -EBUSY;
> +       }
> +
>         reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>         if (is_on) {
>                 if (dwc->revision <= DWC3_REVISION_187A) {
> @@ -1487,10 +1496,22 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on)
>
>         is_on = !!is_on;
>
> +try_again:
>         spin_lock_irqsave(&dwc->lock, flags);
>         ret = dwc3_gadget_run_stop(dwc, is_on, false);
>         spin_unlock_irqrestore(&dwc->lock, flags);
>
> +       if (ret == -EBUSY) {
> +               ret = wait_for_completion_timeout(&dwc->ep0_in_setup,
> +                                                 msecs_to_jiffies(500));
> +               if (ret == 0) {
> +                       dev_err(dwc->dev, "timeout to stop gadget.\n");
> +                       return -ETIMEDOUT;
> +               } else {
> +                       goto try_again;
> +               }
> +       }
> +
>         return ret;
>  }
>
> @@ -2914,6 +2935,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>                 goto err4;
>         }
>
> +       init_completion(&dwc->ep0_in_setup);
> +
>         dwc->gadget.ops                 = &dwc3_gadget_ops;
>         dwc->gadget.speed               = USB_SPEED_UNKNOWN;
>         dwc->gadget.sg_supported        = true;
> --
> 1.7.9.5
>



-- 
Baolin.wang
Best Regards