| Message ID | 20240527170413.1107831-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] usb: dwc3: core: remove spin_lock/unlock_* to avoid deadlock when suspend gadget | expand |
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
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
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
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); }
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(-)