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

From: Alan Stern
Date: Wed Nov 23 2016 - 15:49:03 EST


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 <stern@xxxxxxxxxxxxxxxxxxx>

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 <ahaslam@xxxxxxxxxxxx>
> ---
> 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;
> }
>