> -----Original Message----- > From: Bruce Ashfield <bruce.ashfi...@gmail.com> > Sent: Tuesday, June 18, 2024 9:21 PM > To: Li, Meng <meng...@windriver.com> > Cc: linux-yocto@lists.yoctoproject.org > Subject: Re: [linux-yocto] [v2 PATCH] usb: dwc3: core: remove lock of otg > mode during gadget suspend/resume to avoid deadlock > > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender and > know the content is safe. > > On Tue, Jun 18, 2024 at 8:35 AM Li, Meng <meng...@windriver.com> wrote: > > > > > > > > > -----Original Message----- > > > From: linux-yocto@lists.yoctoproject.org > > > <linux-yocto@lists.yoctoproject.org> > > > On Behalf Of Bruce Ashfield > > > Sent: Tuesday, June 18, 2024 8:29 PM > > > To: Li, Meng <meng...@windriver.com> > > > Cc: linux-yocto@lists.yoctoproject.org > > > Subject: Re: [linux-yocto] [v2 PATCH] usb: dwc3: core: remove lock > > > of otg mode during gadget suspend/resume to avoid deadlock > > > > > > CAUTION: This email comes from a non Wind River email account! > > > Do not click links or open attachments unless you recognize the > > > sender and know the content is safe. > > > > > > On Tue, Jun 18, 2024 at 12:02 AM Meng Li <meng...@windriver.com> > 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 > > > > This issue is exposed by commit c7ebd8149ee5 ("usb: dwc3: gadget: > > > > Fix NULL pointer dereference in dwc3_gadget_suspend") that removes > > > > the code of checking whether dwc->gadget_driver is NULL or not. It > > > > causes the following code is executed and deadlock occurs when > > > > trying to get the spinlock. In fact, the root cause is the commit > 5265397f9442("usb: > > > dwc3: > > > > Remove DWC3 locking during gadget suspend/resume") that forgot to > > > > remove the lock of otg mode. So, remove the redundant lock of otg > > > > mode during gadget suspend/resume. > > > > > > > > > > I'll apply this to v6.6/standard/base and linux-yocto-dev > > > v6.10/standard/base. > > > > > > > This patch works fine for v6.6/standard/base. > > About the v6.10/standard/base branch, there is code context conflict. > > I assume that the maintainers patch or other changes are protecting > 6.10 from the issue ? >
Yes! I think my v2 patch for mainline will be merged into one of v6.10-rc, and then this issue will be fixed in v6.10. Thanks, LImeng > > > > > Correct me if I'm wrong and you'd still prefer it on the BSP branches. > > > > > > > Yes! I would like to it on BSP branches, too. > > This is now merged. Whenever something goes to standard/base, it will > always be on all BSPs as well since my scripts merge from the core branches to > all BSP leaf branches on every change. > > Bruce > > > > > Thanks, > > Limeng > > > > > Bruce > > > > > > > Fixes: 5265397f9442 ("usb: dwc3: Remove DWC3 locking during gadget > > > > suspend/resume") > > > > Cc: Xu Yang <xu.yan...@nxp.com> > > > > Cc: sta...@vger.kernel.org > > > > Signed-off-by: Meng Li <meng...@windriver.com> > > > > --- > > > > drivers/usb/dwc3/core.c | 6 ------ > > > > 1 file changed, 6 deletions(-) > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > index a8fb10cc65bc..9bb4ab409bbb 100644 > > > > --- a/drivers/usb/dwc3/core.c > > > > +++ b/drivers/usb/dwc3/core.c > > > > @@ -2112,7 +2112,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; > > > > > > > > switch (dwc->current_dr_role) { @@ -2150,9 +2149,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); > > > > } > > > > > > > > @@ -2169,7 +2166,6 @@ static int dwc3_suspend_common(struct > dwc3 > > > *dwc, > > > > pm_message_t msg) > > > > > > > > static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t > msg) { > > > > - unsigned long flags; > > > > int ret; > > > > u32 reg; > > > > > > > > @@ -2227,9 +2223,7 @@ static int dwc3_resume_common(struct dwc3 > > > *dwc, pm_message_t msg) > > > > if (dwc->current_otg_role == DWC3_OTG_ROLE_HOST) { > > > > dwc3_otg_host_init(dwc); > > > > } else if (dwc->current_otg_role == > > > > DWC3_OTG_ROLE_DEVICE) { > > > > - spin_lock_irqsave(&dwc->lock, flags); > > > > dwc3_gadget_resume(dwc); > > > > - spin_unlock_irqrestore(&dwc->lock, flags); > > > > } > > > > > > > > break; > > > > -- > > > > 2.34.1 > > > > > > > > > > > > > -- > > > - Thou shalt not follow the NULL pointer, for chaos and madness > > > await thee at its end > > > - "Use the force Harry" - Gandalf, Star Trek II > > > > -- > - Thou shalt not follow the NULL pointer, for chaos and madness await thee at > its end > - "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#14067): https://lists.yoctoproject.org/g/linux-yocto/message/14067 Mute This Topic: https://lists.yoctoproject.org/mt/106735260/21656 Group Owner: linux-yocto+ow...@lists.yoctoproject.org Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-