[PATCH v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget

[PATCH v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget

[Xu Yang]

In current design, spin_lock_irqsave() will run twice when suspend gadget
device if the controller is using OTG block:

dwc3_suspend_common()
  spin_lock_irqsave(&dwc->lock, flags);      <-- 1st
  dwc3_gadget_suspend(dwc);
    dwc3_gadget_soft_disconnect(dwc);
      spin_lock_irqsave(&dwc->lock, flags);  <-- 2nd

This will cause deadlock on the suspend path. To let it work, just remove
spin_lock/unlock_* in dwc3_suspend_common() since they are redundant.

Fixes: 5265397f9442 ("usb: dwc3: Remove DWC3 locking during gadget suspend/resume")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - add fix tag and stable list
---
 drivers/usb/dwc3/core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 7ee61a89520b..b8193edc41bd 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2250,7 +2250,6 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 
 static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 {
-	unsigned long	flags;
 	u32 reg;
 	int i;
 
@@ -2293,9 +2292,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 			break;
 
 		if (dwc->current_otg_role == DWC3_OTG_ROLE_DEVICE) {
-			spin_lock_irqsave(&dwc->lock, flags);
 			dwc3_gadget_suspend(dwc);
-			spin_unlock_irqrestore(&dwc->lock, flags);
 			synchronize_irq(dwc->irq_gadget);
 		}
 
-- 
2.34.1

Re: [PATCH v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget

[Thinh Nguyen]

On Tue, May 28, 2024, Xu Yang wrote:
> In current design, spin_lock_irqsave() will run twice when suspend gadget
> device if the controller is using OTG block:
> 
> dwc3_suspend_common()
>   spin_lock_irqsave(&dwc->lock, flags);      <-- 1st
>   dwc3_gadget_suspend(dwc);
>     dwc3_gadget_soft_disconnect(dwc);
>       spin_lock_irqsave(&dwc->lock, flags);  <-- 2nd
> 
> This will cause deadlock on the suspend path. To let it work, just remove
> spin_lock/unlock_* in dwc3_suspend_common() since they are redundant.
> 
> Fixes: 5265397f9442 ("usb: dwc3: Remove DWC3 locking during gadget suspend/resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - add fix tag and stable list
> ---
>  drivers/usb/dwc3/core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 7ee61a89520b..b8193edc41bd 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2250,7 +2250,6 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  
>  static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  {
> -	unsigned long	flags;
>  	u32 reg;
>  	int i;
>  
> @@ -2293,9 +2292,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  			break;
>  
>  		if (dwc->current_otg_role == DWC3_OTG_ROLE_DEVICE) {
> -			spin_lock_irqsave(&dwc->lock, flags);
>  			dwc3_gadget_suspend(dwc);
> -			spin_unlock_irqrestore(&dwc->lock, flags);
>  			synchronize_irq(dwc->irq_gadget);
>  		}
>  
> -- 
> 2.34.1
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

Re: [PATCH v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget

[Greg KH]

On Tue, May 28, 2024 at 01:04:13AM +0800, Xu Yang wrote:
> In current design, spin_lock_irqsave() will run twice when suspend gadget
> device if the controller is using OTG block:
> 
> dwc3_suspend_common()
>   spin_lock_irqsave(&dwc->lock, flags);      <-- 1st
>   dwc3_gadget_suspend(dwc);
>     dwc3_gadget_soft_disconnect(dwc);
>       spin_lock_irqsave(&dwc->lock, flags);  <-- 2nd
> 
> This will cause deadlock on the suspend path. To let it work, just remove
> spin_lock/unlock_* in dwc3_suspend_common() since they are redundant.
> 
> Fixes: 5265397f9442 ("usb: dwc3: Remove DWC3 locking during gadget suspend/resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>  - add fix tag and stable list
> ---
>  drivers/usb/dwc3/core.c | 3 ---
>  1 file changed, 3 deletions(-)

Is this still needed?  if so, can you rebase and resubmit?

thanks,

greg k-h

Re: [PATCH v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget

[Xu Yang]

On Wed, Jul 31, 2024 at 10:30:54AM +0200, Greg KH wrote:
> On Tue, May 28, 2024 at 01:04:13AM +0800, Xu Yang wrote:
> > In current design, spin_lock_irqsave() will run twice when suspend gadget
> > device if the controller is using OTG block:
> > 
> > dwc3_suspend_common()
> >   spin_lock_irqsave(&dwc->lock, flags);      <-- 1st
> >   dwc3_gadget_suspend(dwc);
> >     dwc3_gadget_soft_disconnect(dwc);
> >       spin_lock_irqsave(&dwc->lock, flags);  <-- 2nd
> > 
> > This will cause deadlock on the suspend path. To let it work, just remove
> > spin_lock/unlock_* in dwc3_suspend_common() since they are redundant.
> > 
> > Fixes: 5265397f9442 ("usb: dwc3: Remove DWC3 locking during gadget suspend/resume")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > 
> > ---
> > Changes in v2:
> >  - add fix tag and stable list
> > ---
> >  drivers/usb/dwc3/core.c | 3 ---
> >  1 file changed, 3 deletions(-)
> 
> Is this still needed?  if so, can you rebase and resubmit?

It's not needed now. 
Meng Li sent a better one and it's already accepted.
https://lore.kernel.org/r/20240618031918.2585799-1-Meng.Li@windriver.com

Thanks,
Xu Yang

> 
> thanks,
> 
> greg k-h