| Message ID | 20230625161553.11073-1-aarongt.shen@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: ohci-at91: Fix the unhandle interrupt when resume | expand |
On Mon, Jun 26, 2023 at 12:15:53AM +0800, Guiting Shen wrote: > The ohci_hcd_at91_drv_suspend() sets ohci->rh_state to OHCI_RH_HALTED when > suspend which will let the ohci_irq() skip the interrupt after resume. And > nobody to handle this interrupt. > > According to the comment in ohci_hcd_at91_drv_suspend(), it need to reset > when resume from suspend(MEM) to fix. > > Signed-off-by: Guiting Shen <aarongt.shen@gmail.com> > --- > drivers/usb/host/ohci-at91.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c > index b9ce8d80f20b..1a0356d9ea15 100644 > --- a/drivers/usb/host/ohci-at91.c > +++ b/drivers/usb/host/ohci-at91.c > @@ -672,7 +672,12 @@ ohci_hcd_at91_drv_resume(struct device *dev) > else > at91_start_clock(ohci_at91); > > - ohci_resume(hcd, false); > + /* > + * need to reset according to the comment of suspend routine, > + * otherwise the ohci_irq would skip the interrupt if ohci_state > + * was OHCI_RH_HALTED. > + */ > + ohci_resume(hcd, !ohci_at91->wakeup); This comment doesn't say why the code uses !ohci_at91->wakeup. It should explain that. For example: /* * According to the comment in ohci_hcd_at91_drv_suspend() * we need to do a reset if the 48-MHz clock was stopped, * that is, if ohci_at91->wakeup is clear. Tell ohci_resume() * to reset in this case by setting its "hibernated" flag. */ Alan Stern
On Mon, Jun 26, 2023 at 04:04:29AM GMT+8, Alan Stern wrote: > On Mon, Jun 26, 2023 at 12:15:53AM +0800, Guiting Shen wrote: >> The ohci_hcd_at91_drv_suspend() sets ohci->rh_state to OHCI_RH_HALTED when >> suspend which will let the ohci_irq() skip the interrupt after resume. And >> nobody to handle this interrupt. >> >> According to the comment in ohci_hcd_at91_drv_suspend(), it need to reset >> when resume from suspend(MEM) to fix. >> >> Signed-off-by: Guiting Shen <aarongt.shen@gmail.com> >> --- >> drivers/usb/host/ohci-at91.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c >> index b9ce8d80f20b..1a0356d9ea15 100644 >> --- a/drivers/usb/host/ohci-at91.c >> +++ b/drivers/usb/host/ohci-at91.c >> @@ -672,7 +672,12 @@ ohci_hcd_at91_drv_resume(struct device *dev) >> else >> at91_start_clock(ohci_at91); >> >> - ohci_resume(hcd, false); >> + /* >> + * need to reset according to the comment of suspend routine, >> + * otherwise the ohci_irq would skip the interrupt if ohci_state >> + * was OHCI_RH_HALTED. >> + */ >> + ohci_resume(hcd, !ohci_at91->wakeup); > > This comment doesn't say why the code uses !ohci_at91->wakeup. It > should explain that. For example: > > /* > * According to the comment in ohci_hcd_at91_drv_suspend() > * we need to do a reset if the 48-MHz clock was stopped, > * that is, if ohci_at91->wakeup is clear. Tell ohci_resume() > * to reset in this case by setting its "hibernated" flag. > */ > Ok, Thank you! Do I send the v3 patch after the merge window close?
On Mon, Jun 26, 2023 at 10:28:26AM +0800, Guiting Shen wrote: > On Mon, Jun 26, 2023 at 04:04:29AM GMT+8, Alan Stern wrote: > > This comment doesn't say why the code uses !ohci_at91->wakeup. It > > should explain that. For example: > > > > /* > > * According to the comment in ohci_hcd_at91_drv_suspend() > > * we need to do a reset if the 48-MHz clock was stopped, > > * that is, if ohci_at91->wakeup is clear. Tell ohci_resume() > > * to reset in this case by setting its "hibernated" flag. > > */ > > > > Ok, Thank you! > Do I send the v3 patch after the merge window close? You can send the patch at any time. It doesn't matter whether the merge window is open or closed. Alan Stern
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index b9ce8d80f20b..1a0356d9ea15 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -672,7 +672,12 @@ ohci_hcd_at91_drv_resume(struct device *dev) else at91_start_clock(ohci_at91); - ohci_resume(hcd, false); + /* + * need to reset according to the comment of suspend routine, + * otherwise the ohci_irq would skip the interrupt if ohci_state + * was OHCI_RH_HALTED. + */ + ohci_resume(hcd, !ohci_at91->wakeup); return 0; }
The ohci_hcd_at91_drv_suspend() sets ohci->rh_state to OHCI_RH_HALTED when suspend which will let the ohci_irq() skip the interrupt after resume. And nobody to handle this interrupt. According to the comment in ohci_hcd_at91_drv_suspend(), it need to reset when resume from suspend(MEM) to fix. Signed-off-by: Guiting Shen <aarongt.shen@gmail.com> --- drivers/usb/host/ohci-at91.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)