Re: [PATCH v3 2/2] usb: dwc3: Wait for control tranfer completed when stopping gadget
From: Baolin Wang
Date: Fri Sep 30 2016 - 01:50:34 EST
Hi Felipe,
On 19 September 2016 at 19:52, Baolin Wang <baolin.wang@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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