| Message ID | 1550138622-10761-1-git-send-email-jh80.chung@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc2: hcd: call dwc2_is_controller_alive under spinlock | expand |
Hi Jaehoon Chung, On 2/14/2019 2:04 PM, Jaehoon Chung wrote: > This patch is referred to Robert's patch > commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock") > > During running sdb with otg mode, the usb is hung sometime. > > The one of SDB hang issues should be fixed with this patch. > After hang, it doesn't never trigger the usb interrupt. > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > drivers/usb/dwc2/hcd_intr.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c > index 88b5dcf..8d3b155 100644 > --- a/drivers/usb/dwc2/hcd_intr.c > +++ b/drivers/usb/dwc2/hcd_intr.c > @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) > u32 gintsts, dbg_gintsts; > irqreturn_t retval = IRQ_NONE; > > + spin_lock(&hsotg->lock); > + > if (!dwc2_is_controller_alive(hsotg)) { > dev_warn(hsotg->dev, "Controller is dead\n"); > - return retval; > + goto out; > } > > - spin_lock(&hsotg->lock); > - > /* Check if HOST Mode */ > if (dwc2_is_host_mode(hsotg)) { > gintsts = dwc2_read_core_intr(hsotg); > @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) > } > } > > +out: > spin_unlock(&hsotg->lock); > > return retval; > 1. Checking core alive or not was introduced in our internal reference driver for our setups, because sometime AHB-PCI bridge can hung, not core itself. If it happen then only way to overcome it - reboot core setup and PC. 2. Actually this issue was fixed on our setups and currently no need to check access via bridge to core. 3. Any case if it still happen on some platforms then first need to check and fix HW. On asserted any interrupt for dwc2, handlers call sequence is follow: common->gadget->host handlers. If core alive checked in common handler then checking same stuff in host handler again not needed at all. So, core alive checking in host handler create issue for your test, I suggest to not fix alive checking in host interrupt handler as you suggested and done in common handler, but just remove it. Thanks, Minas
Hi Minas, On 2/14/19 8:43 PM, Minas Harutyunyan wrote: > Hi Jaehoon Chung, > > On 2/14/2019 2:04 PM, Jaehoon Chung wrote: >> This patch is referred to Robert's patch >> commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock") >> >> During running sdb with otg mode, the usb is hung sometime. >> >> The one of SDB hang issues should be fixed with this patch. >> After hang, it doesn't never trigger the usb interrupt. >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> --- >> drivers/usb/dwc2/hcd_intr.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c >> index 88b5dcf..8d3b155 100644 >> --- a/drivers/usb/dwc2/hcd_intr.c >> +++ b/drivers/usb/dwc2/hcd_intr.c >> @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) >> u32 gintsts, dbg_gintsts; >> irqreturn_t retval = IRQ_NONE; >> >> + spin_lock(&hsotg->lock); >> + >> if (!dwc2_is_controller_alive(hsotg)) { >> dev_warn(hsotg->dev, "Controller is dead\n"); >> - return retval; >> + goto out; >> } >> >> - spin_lock(&hsotg->lock); >> - >> /* Check if HOST Mode */ >> if (dwc2_is_host_mode(hsotg)) { >> gintsts = dwc2_read_core_intr(hsotg); >> @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) >> } >> } >> >> +out: >> spin_unlock(&hsotg->lock); >> >> return retval; >> > > 1. Checking core alive or not was introduced in our internal reference > driver for our setups, because sometime AHB-PCI bridge can hung, not > core itself. If it happen then only way to overcome it - reboot core > setup and PC. > 2. Actually this issue was fixed on our setups and currently no need to > check access via bridge to core. > 3. Any case if it still happen on some platforms then first need to > check and fix HW I didn't check with latest kernel. we're using v4.4.172. This issue is occurred on only ARTIK boards. - During running test about 10~15 hours, suddenly hang. - It's impossible to fix HW side. After applied this patch, passed the test case as 100%. (Before applied, it didn't pass the test.) > > On asserted any interrupt for dwc2, handlers call sequence is follow: > common->gadget->host handlers. If core alive checked in common handler > then checking same stuff in host handler again not needed at all. Right, it's the duplicated checking according to sequence. > > So, core alive checking in host handler create issue for your test, I > suggest to not fix alive checking in host interrupt handler as you > suggested and done in common handler, but just remove it. Will remove it. Best Regards, Jaehoon Chung > > Thanks, > Minas > > > > >
diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c index 88b5dcf..8d3b155 100644 --- a/drivers/usb/dwc2/hcd_intr.c +++ b/drivers/usb/dwc2/hcd_intr.c @@ -2222,13 +2222,13 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) u32 gintsts, dbg_gintsts; irqreturn_t retval = IRQ_NONE; + spin_lock(&hsotg->lock); + if (!dwc2_is_controller_alive(hsotg)) { dev_warn(hsotg->dev, "Controller is dead\n"); - return retval; + goto out; } - spin_lock(&hsotg->lock); - /* Check if HOST Mode */ if (dwc2_is_host_mode(hsotg)) { gintsts = dwc2_read_core_intr(hsotg); @@ -2276,6 +2276,7 @@ irqreturn_t dwc2_handle_hcd_intr(struct dwc2_hsotg *hsotg) } } +out: spin_unlock(&hsotg->lock); return retval;
This patch is referred to Robert's patch commit cf54772b913b ("usb: dwc2: call dwc2_is_controller_alive() under spinlock") During running sdb with otg mode, the usb is hung sometime. The one of SDB hang issues should be fixed with this patch. After hang, it doesn't never trigger the usb interrupt. Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- drivers/usb/dwc2/hcd_intr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)