2016-11-23 18:32:30

by Axel Haslam

[permalink] [raw]
Subject: [PATCH] USB: ohci: da8xx: Balance ochi_disable with ohci_enable in resume.

On resume from suspend a failure with -ESHUTDOWN is returned
from ohci_bus_resume, and the usb is inoperable.

This happens because ohci_suspend disables the master interrupt
and sets an hcd flag to say that the hw is no longer accessible.

Calling ohci_resume reverts the steps taken on ohci_suspend
and flags the HW as "accessible" again, resume completes
successfully and usb is working after a suspend/resume sequence.

While we are here, remove setting device power_state,
as this is no longer needed and scheduled for removal.

Signed-off-by: Axel Haslam <[email protected]>
---
drivers/usb/host/ohci-da8xx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index b3de8bc..a598bd8 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -340,8 +340,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
if (ret)
return ret;

- dev->dev.power.power_state = PMSG_ON;
- usb_hcd_resume_root_hub(hcd);
+ ohci_resume(hcd, false);

return 0;
}
--
2.9.3


2016-11-23 20:48:56

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] USB: ohci: da8xx: Balance ochi_disable with ohci_enable in resume.

On Wed, 23 Nov 2016, Axel Haslam wrote:

> On resume from suspend a failure with -ESHUTDOWN is returned
> from ohci_bus_resume, and the usb is inoperable.
>
> This happens because ohci_suspend disables the master interrupt
> and sets an hcd flag to say that the hw is no longer accessible.

This patch is okay, but the title and the two sentences above are
completely beside the point. The real point of this patch is that
ohci_da8xx_suspend() calls ohci_suspend(), and therefore
ohci_da8xx_resume() should call ohci_resume() rather than
ohci_bus_resume().

Which is the right thing to do in any case, because the
ohci_da8xx_resume() callback wants to resume the entire controller and
not just the root hub.

After fixing the title and those two sentences, you can add:

Acked-by: Alan Stern <[email protected]>

Alan Stern

>
> Calling ohci_resume reverts the steps taken on ohci_suspend
> and flags the HW as "accessible" again, resume completes
> successfully and usb is working after a suspend/resume sequence.
>
> While we are here, remove setting device power_state,
> as this is no longer needed and scheduled for removal.
>
> Signed-off-by: Axel Haslam <[email protected]>
> ---
> drivers/usb/host/ohci-da8xx.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
> index b3de8bc..a598bd8 100644
> --- a/drivers/usb/host/ohci-da8xx.c
> +++ b/drivers/usb/host/ohci-da8xx.c
> @@ -340,8 +340,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
> if (ret)
> return ret;
>
> - dev->dev.power.power_state = PMSG_ON;
> - usb_hcd_resume_root_hub(hcd);
> + ohci_resume(hcd, false);
>
> return 0;
> }
>