Messages in this thread | | | Subject | Re: [PATCH] usb: host: ohci-at91: suspend/resume ports after/before OHCI accesses | From | Nicolas Ferre <> | Date | Wed, 9 Jun 2021 14:40:19 +0200 |
| |
On 09/06/2021 at 14:10, Claudiu Beznea wrote: > On SAMA7G5 suspending ports will cut the access to OHCI registers and > any subsequent access to them will lead to CPU being blocked trying to > access that memory. Same thing happens on resume: if OHCI memory is > accessed before resuming ports the CPU will block on that access. The > OCHI memory is accessed on suspend/resume though > ohci_suspend()/ohci_resume(). > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
Claudiu,
Your patch look good to me.
In addition, I see ohci_at91_port_suspend() function also used in ohci_at91_hub_control(). It might suffer the same problem as I see accesses to ohci register, at first glance.
Can you please double check that we are not in such condition with calls to ohci_at91_hub_control()?
Best regards, Nicolas
> --- > > The patch was tested on SAMA7G5, SAMA5D2 and SAM9X60. > > Thank you, > Claudiu Beznea > > drivers/usb/host/ohci-at91.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c > index b3a6a497dcb1..7c6202b05ff4 100644 > --- a/drivers/usb/host/ohci-at91.c > +++ b/drivers/usb/host/ohci-at91.c > @@ -666,8 +666,6 @@ ohci_hcd_at91_drv_suspend(struct device *dev) > if (ohci_at91->wakeup) > enable_irq_wake(hcd->irq); > > - ohci_at91_port_suspend(ohci_at91, 1); > - > ret = ohci_suspend(hcd, ohci_at91->wakeup); > if (ret) { > if (ohci_at91->wakeup) > @@ -687,7 +685,10 @@ ohci_hcd_at91_drv_suspend(struct device *dev) > /* flush the writes */ > (void) ohci_readl (ohci, &ohci->regs->control); > msleep(1); > + ohci_at91_port_suspend(ohci_at91, 1); > at91_stop_clock(ohci_at91); > + } else { > + ohci_at91_port_suspend(ohci_at91, 1); > } > > return ret; > @@ -699,6 +700,8 @@ ohci_hcd_at91_drv_resume(struct device *dev) > struct usb_hcd *hcd = dev_get_drvdata(dev); > struct ohci_at91_priv *ohci_at91 = hcd_to_ohci_at91_priv(hcd); > > + ohci_at91_port_suspend(ohci_at91, 0); > + > if (ohci_at91->wakeup) > disable_irq_wake(hcd->irq); > else > @@ -706,8 +709,6 @@ ohci_hcd_at91_drv_resume(struct device *dev) > > ohci_resume(hcd, false); > > - ohci_at91_port_suspend(ohci_at91, 0); > - > return 0; > } > >
-- Nicolas Ferre
| |