Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeedHUBs

[Alan Stern]

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@xxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/