linux-kernel - Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1312091019400.1439-100000@iolanthe.rowland.org>
Date:	Mon, 9 Dec 2013 10:24:52 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Vikas Sajjan <vikas.sajjan@...aro.org>
cc:	linux-samsung-soc@...r.kernel.org, <kgene.kim@...sung.com>,
	<linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
	<sarah.a.sharp@...ux.intel.com>, <vpalatin@...omium.org>,
	<jwerner@...omium.org>, <tianyu.lan@...el.com>,
	<burzalodowa@...il.com>, <gregkh@...uxfoundation.org>,
	<gautam.vivek@...sung.com>, <dianders@...omium.org>,
	<balbi@...com>, <joshi@...sung.com>
Subject: Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed
 HUBs

On Mon, 9 Dec 2013, Vikas Sajjan wrote:

> Does warm reset while activating SuperSpeed HUBs if the hub activate type
> is HUB_RESET_RESUME.
> 
> When we do Suspend-to-RAM with (any one of the 16, 32, 64 Jetflash) transcend
> USB 3.0 device connected on 3.0 port, during resume I noticed that the
> XHCI controller has moved to sometimes RECOVERY, POLLING or INACTIVE STATE.
> This behaviour is inconsistent and the connection with connected USB 3.0 device
> on 3.0 port was LOST.
> 
> Doing warm reset while activating SuperSpeed HUBs if the hub
> activate type is HUB_RESET_RESUME, gets the connected device to the stable state.
> 
> Reviewed at https://chromium-review.googlesource.com/#/c/177132/
> 
> Tested on exynos5420 and exynos5250 with Transcend Jetflash USB 3.0 device (8564:1000)
> 
> rebased on Greg Kroah-Hartman's usb-next
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
> 
> Signed-off-by: Vikas Sajjan <vikas.sajjan@...sung.com>
> ---
>  drivers/usb/core/hub.c |   41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index a7c04e2..d8432b0 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c

> @@ -1093,6 +1108,16 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
>  		u16 portstatus, portchange;
>  
>  		portstatus = portchange = 0;
> +
> +		/* Some connected devices might be still in unknown state even
> +		 * after reset-resume, a WARM_RESET gets the connected device
> +		 * to the normal state.
> +		 */
> +		if (udev && hub_is_superspeed(hub->hdev) &&
> +						type == HUB_RESET_RESUME)
> +			hub_port_reset(hub, port1, NULL,
> +						HUB_BH_RESET_TIME, true);

Please don't do this all the time to every attached port.  Do it only 
when it is really needed.

Shouldn't you pass udev as the third argument?  If not, please explain
why not.

Finally, I don't see why you put this in hub_activate().  Isn't it more 
closely connected with the reset-resume procedure for the child device?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/