All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Wesley Cheng <quic_wcheng@quicinc.com>
Cc: Thinh.Nguyen@synopsys.com, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH v5 0/3] Avoid having pending end transfers on soft disconnect
Date: Thu, 20 Apr 2023 11:21:37 +0200	[thread overview]
Message-ID: <ZEEEIfqTKiM6CKAR@kroah.com> (raw)
In-Reply-To: <20230413195742.11821-1-quic_wcheng@quicinc.com>

On Thu, Apr 13, 2023 at 12:57:39PM -0700, Wesley Cheng wrote:
> In case there is a host which takes time to complete a SETUP transaction,
> during the soft disconnect sequence multiple DWC3 EPs will have their
> DWC3_EP_DELAY_STOP flag set w/o issuing the end transfer command.  Once the
> controller halt sequence occurs, the soft disconnect is successful, and
> the subsequent soft connect will attempt to flush the pending end transfers.
> 
> Soft disconnect sequence:
>   dwc3_gadget_ep_disable   name=ep8in flags=0x3009  direction=1
>   dwc3_gadget_ep_disable   name=ep4in flags=1  direction=1
>   dwc3_gadget_ep_disable   name=ep3out flags=1  direction=0
>   usb_gadget_disconnect   deactivated=0  connected=0  ret=0
> 
> Soft connect bug:
>   BUG: spinlock already unlocked on CPU
>   spin_bug+0x0
>   dwc3_remove_requests+0x278
>   dwc3_ep0_out_start+0xb0
>   __dwc3_gadget_start+0x25c
> 
> The bug occurs due to the flush of the pending end transfers, as the gadget
> start routine is not held with a spinlock.  However, if the DWC3_EP_DELAY_STOP
> is set, it will call the giveback API, which attempts to unlock the dwc->lock.
> Ideally, the DWC3 gadget should not have pending end transfers on a soft
> connect, so fix this by:
> 
>  1. Re-locating the SETUP phase check after stop active transfers, since
>  that is where the DWC3_EP_DELAY_STOP is potentially set.  This also allows
>  for handling of a host that may be unresponsive by using the completion
>  timeout to trigger the stall and restart for EP0.
> 
>  2. Do not call gadget stop until the poll for controller halt is
>  completed.  DEVTEN is cleared as part of gadget stop, so the intention to
>  allow ep0 events to continue while waiting for controller halt is not
>  happening.

Only patch 1 applied, can you rebase the rest and resend with the acks
applied?

thanks,

greg k-h


      parent reply	other threads:[~2023-04-20  9:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-13 19:57 [PATCH v5 0/3] Avoid having pending end transfers on soft disconnect Wesley Cheng
2023-04-13 19:57 ` [PATCH v5 1/3] usb: dwc3: gadget: Stall and restart EP0 if host is unresponsive Wesley Cheng
2023-04-18  1:46   ` Thinh Nguyen
2023-04-13 19:57 ` [PATCH v5 2/3] usb: dwc3: gadget: Execute gadget stop after halting the controller Wesley Cheng
2023-04-13 19:57 ` [PATCH v5 3/3] usb: dwc3: gadget: Refactor EP0 forced stall/restart into a separate API Wesley Cheng
2023-04-18  1:47   ` Thinh Nguyen
2023-04-20  9:21 ` Greg KH [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZEEEIfqTKiM6CKAR@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=quic_wcheng@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.