Re: [v2 PATCH] usb: dwc3: core: remove lock of otg mode during gadget suspend/resume to avoid deadlock

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Wed, Jun 19, 2024, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> On Wed, Jun 19, 2024 at 12:42 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> >
> > Hi,
> >
> > On Wed, Jun 19, 2024, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Tue, Jun 18, 2024 at 11:55 PM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> > > >
> > > > On Tue, Jun 18, 2024, Meng Li wrote:
> > > > > When config CONFIG_USB_DWC3_DUAL_ROLE is selected, and trigger system
> > > > > to enter suspend status with below command:
> > > > > echo mem > /sys/power/state
> > > > > There will be a deadlock issue occurring. Detailed invoking path as
> > > > > below:
> > > > > 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
> > >
> > >
> > > In resume path the spinlock was taken too and I see that there is a call
> > > dwc3_gadget_soft_connect that call the dwc3_core_soft_reset that can poll
> > > using msleep for time.  I don't know if this is a valid fix but seems
> > > does not fix
> > > only commit c7ebd8149ee5
> > >
> >
> > Meng did take the resume path into account. Can you clarify what you
> > meant with "does not fix only commit c7ebd8149ee5"?
> 
> I have seen that is suppose to fix something introduced by that commit
> but I think
> that resume path was already broken before
> 
> spinlock_irqsave()
> dwc3_gadget_resume -> dwc3_gadget_soft_connect -> dwc3_core_soft_reset
> (can sleep)
> 

The code path for OTG should not hit that sleep. The sleep is only
applicable to DWC_usb31 and DWC_usb32. Technically, it wasn't broken
before. But we should remove this spinlock on resume as it is done here.

BR,
Thinh