lkml.org 
[lkml]   [2021]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] usb: host: ohci-at91: suspend/resume ports after/before OHCI accesses
From
Date
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

\
 
 \ /
  Last update: 2021-06-09 14:41    [W:0.118 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site