linux-kernel - Re: [PATCH] USB: ohci: da8xx: Balance ochi_disable with ohci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1611231545040.22736-100000@netrider.rowland.org>
Date:   Wed, 23 Nov 2016 15:48:53 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Axel Haslam <ahaslam@...libre.com>
cc:     gregkh@...uxfoundation.org, <nsekhar@...com>,
        <khilman@...libre.com>, <david@...hnology.com>,
        <ptitiano@...libre.com>, <linux-kernel@...r.kernel.org>,
        <linux-usb@...r.kernel.org>
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 <stern@...land.harvard.edu>

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@...libre.com>
> ---
>  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;
>  }
>