| Message ID | 65daa916b29e0009083702cb4584c8274e58ddac.1618464534.git.Arthur.Petrosyan@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/15] usb: dwc2: Update exit hibernation when port reset is asserted | expand |
On 15.04.2021 8:40, Artur Petrosyan wrote: > When core is in hibernation state and an external > hub is connected, upper layer sends URB enqueue request, > which results in port reset issue. > > - Added exit from hibernation state to avoid port > reset issue and process upper layer request properly. > > Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com> > --- > drivers/usb/dwc2/hcd.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index cc9ad6cf02d9..3b03b2d73aaa 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, > struct dwc2_qh *qh; > bool qh_allocated = false; > struct dwc2_qtd *qtd; > + struct dwc2_gregs_backup *gr; > + > + gr = &hsotg->gr_backup; > > if (dbg_urb(urb)) { > dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n"); > dwc2_dump_urb_info(hcd, urb, "urb_enqueue"); > } > > + if (hsotg->hibernated) { > + if (gr->gotgctl & GOTGCTL_CURMODE_HOST) { > + retval = dwc2_exit_hibernation(hsotg, 0, 0, 1); > + if (retval) > + dev_err(hsotg->dev, > + "exit hibernation failed.\n"); > + } else { > + retval = dwc2_exit_hibernation(hsotg, 0, 0, 0); > + if (retval) > + dev_err(hsotg->dev, > + "exit hibernation failed.\n"); Why not put these identical *if*s outside the the outer *if*? > + } > + } > + > if (hsotg->in_ppd) { > retval = dwc2_exit_partial_power_down(hsotg, 0, true); > if (retval) MBR, Sergei
Hi Sergei, On 4/15/2021 13:12, Sergei Shtylyov wrote: > On 15.04.2021 8:40, Artur Petrosyan wrote: > >> When core is in hibernation state and an external >> hub is connected, upper layer sends URB enqueue request, >> which results in port reset issue. >> >> - Added exit from hibernation state to avoid port >> reset issue and process upper layer request properly. >> >> Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com> >> --- >> drivers/usb/dwc2/hcd.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >> index cc9ad6cf02d9..3b03b2d73aaa 100644 >> --- a/drivers/usb/dwc2/hcd.c >> +++ b/drivers/usb/dwc2/hcd.c >> @@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, >> struct dwc2_qh *qh; >> bool qh_allocated = false; >> struct dwc2_qtd *qtd; >> + struct dwc2_gregs_backup *gr; >> + >> + gr = &hsotg->gr_backup; >> >> if (dbg_urb(urb)) { >> dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n"); >> dwc2_dump_urb_info(hcd, urb, "urb_enqueue"); >> } >> >> + if (hsotg->hibernated) { >> + if (gr->gotgctl & GOTGCTL_CURMODE_HOST) { >> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 1); >> + if (retval) >> + dev_err(hsotg->dev, >> + "exit hibernation failed.\n"); >> + } else { >> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 0); >> + if (retval) >> + dev_err(hsotg->dev, >> + "exit hibernation failed.\n"); > > Why not put these identical *if*s outside the the outer *if*? > Well the reason that the conditions are implemented like they are, is that the inner if checks whether core operates in host mode or device mode and the outside if checks if the core is hibernated or not. So now imagine that the ifs are combined and core is not hibernated but the operational mode of the driver is let's say gadget. If the case is such then the driver will try to exit from gadget hibernation because of the else condition as the if condition will be false again because core is not hibernated. As a result if we combine the outside and inner ifs then it will try to exit from gadget hibernation but core is not hibernated and that would be an issue. > >> + } >> + } >> + >> if (hsotg->in_ppd) { >> retval = dwc2_exit_partial_power_down(hsotg, 0, true); >> if (retval) > > MBR, Sergei > Regards, Artur
Hi Sergei, On 4/16/2021 09:43, Artur Petrosyan wrote: > Hi Sergei, > > On 4/15/2021 13:12, Sergei Shtylyov wrote: >> On 15.04.2021 8:40, Artur Petrosyan wrote: >> >>> When core is in hibernation state and an external >>> hub is connected, upper layer sends URB enqueue request, >>> which results in port reset issue. >>> >>> - Added exit from hibernation state to avoid port >>> reset issue and process upper layer request properly. >>> >>> Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com> >>> --- >>> drivers/usb/dwc2/hcd.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c >>> index cc9ad6cf02d9..3b03b2d73aaa 100644 >>> --- a/drivers/usb/dwc2/hcd.c >>> +++ b/drivers/usb/dwc2/hcd.c >>> @@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, >>> struct dwc2_qh *qh; >>> bool qh_allocated = false; >>> struct dwc2_qtd *qtd; >>> + struct dwc2_gregs_backup *gr; >>> + >>> + gr = &hsotg->gr_backup; >>> >>> if (dbg_urb(urb)) { >>> dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n"); >>> dwc2_dump_urb_info(hcd, urb, "urb_enqueue"); >>> } >>> >>> + if (hsotg->hibernated) { >>> + if (gr->gotgctl & GOTGCTL_CURMODE_HOST) { >>> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 1); >>> + if (retval) >>> + dev_err(hsotg->dev, >>> + "exit hibernation failed.\n"); >>> + } else { >>> + retval = dwc2_exit_hibernation(hsotg, 0, 0, 0); >>> + if (retval) >>> + dev_err(hsotg->dev, >>> + "exit hibernation failed.\n"); >> >> Why not put these identical *if*s outside the the outer *if*? >> > Well the reason that the conditions are implemented like they are, is > that the inner if checks whether core operates in host mode or device > mode and the outside if checks if the core is hibernated or not. > > So now imagine that the ifs are combined and core is not hibernated but > the operational mode of the driver is let's say gadget. If the case is > such then the driver will try to exit from gadget hibernation because of > the else condition as the if condition will be false again because core > is not hibernated. As a result if we combine the outside and inner ifs > then it will try to exit from gadget hibernation but core is not > hibernated and that would be an issue. > Sorry I got your point wrong there. You meant the ifs for dev_err(). Thank you for the review I will update them. > >> >>> + } >>> + } >>> + >>> if (hsotg->in_ppd) { >>> retval = dwc2_exit_partial_power_down(hsotg, 0, true); >>> if (retval) >> >> MBR, Sergei >> > > Regards, > Artur >
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index cc9ad6cf02d9..3b03b2d73aaa 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4631,12 +4631,29 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, struct dwc2_qh *qh; bool qh_allocated = false; struct dwc2_qtd *qtd; + struct dwc2_gregs_backup *gr; + + gr = &hsotg->gr_backup; if (dbg_urb(urb)) { dev_vdbg(hsotg->dev, "DWC OTG HCD URB Enqueue\n"); dwc2_dump_urb_info(hcd, urb, "urb_enqueue"); } + if (hsotg->hibernated) { + if (gr->gotgctl & GOTGCTL_CURMODE_HOST) { + retval = dwc2_exit_hibernation(hsotg, 0, 0, 1); + if (retval) + dev_err(hsotg->dev, + "exit hibernation failed.\n"); + } else { + retval = dwc2_exit_hibernation(hsotg, 0, 0, 0); + if (retval) + dev_err(hsotg->dev, + "exit hibernation failed.\n"); + } + } + if (hsotg->in_ppd) { retval = dwc2_exit_partial_power_down(hsotg, 0, true); if (retval)
When core is in hibernation state and an external hub is connected, upper layer sends URB enqueue request, which results in port reset issue. - Added exit from hibernation state to avoid port reset issue and process upper layer request properly. Signed-off-by: Artur Petrosyan <Arthur.Petrosyan@synopsys.com> --- drivers/usb/dwc2/hcd.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)