RE: [PATCH] USB: hub: Fix the broken detection of USB3 device in SMSC hub

RE: [PATCH] USB: hub: Fix the broken detection of USB3 device in SMSC hub

[Gajjar, Hardik (TCP GmbH; ADITG/ESM1)]

Hi Alan,

Thank you so much for the speedy reply.  I have used the usb_autopm_get_interface() and it is working like a charm.

Best regards
Hardik Gajjar

Advanced Driver Information Technology GmbH
Engineering Software Multimedia 1 (ADITG/ESM1)
Robert-Bosch-Str. 200
31139 Hildesheim
Germany

Tel. +49 5121 49 6977
Fax +49 5121 49 6999
hgajjar@de.adit-jv.com
www.adit-jv.com

ADIT is a joint venture company of Robert Bosch GmbH/Robert Bosch Car Multimedia GmbH and DENSO Corporation
Sitz: Hildesheim, Registergericht: Amtsgericht Hildesheim HRB 3438
Geschaeftsfuehrung: Wilhelm Grabow, Ken Yaguchi

Data protection information: We use your contact information and e-mail communication to process your inquiry and for internal and possibly tax documentation purposes. Your information will only be passed on to third parties if this is necessary to carry out your request or if we are legally obliged to do so.


-----Original Message-----
From: Alan Stern <stern@rowland.harvard.edu> 
Sent: Freitag, 24. Januar 2020 16:08
To: Gajjar, Hardik (TCP GmbH; ADITG/ESM1) <hgajjar@de.adit-jv.com>
Cc: gregkh@linuxfoundation.org; thinhn@synopsys.com; Kento.A.Kobayashi@sony.com; atmgnd@outlook.com; andrew_gabbasov@mentor.com; Rosca, Eugeniu (ADITG/ESM1) <erosca@de.adit-jv.com>; linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH] USB: hub: Fix the broken detection of USB3 device in SMSC hub

On Fri, 24 Jan 2020, Hardik Gajjar wrote:

> This patch disables the auto-suspend feature for SMSC USB hub.
> Renesas-RCAR3-H3-KF board was not able to detect the USB3.0 devices.
> The XHCI driver was going to sleep and not wake up again on connection 
> of the USB3.0 device. According to Renesas, This is because of some 
> hardware issue.
> 
> Renesas-RCAR3-H3-KF has USB5534B 4-port SuperSpeed/Hi-Speed, 
> low-power, configurable hub controller.
> 
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> ---
>  drivers/usb/core/hub.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 
> 939dd73a..ffc7abf 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -36,7 +36,9 @@
>  #include "otg_whitelist.h"
>  
>  #define USB_VENDOR_GENESYS_LOGIC		0x05e3
> +#define USB_VENDOR_SMSC    			0x0424
>  #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND	0x01
> +#define HUB_QUIRK_DISABLE_AUTOSUSPEND		0x02
>  
>  #define USB_VENDOR_UNWIRED			0x2996
>  #define USB_VENDOR_DELPHI			0x2C48
> @@ -1822,6 +1824,9 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
>  	if (id->driver_info & HUB_QUIRK_NO_LOGICAL_DISCONNECT)
>  		hub->quirk_no_logical_disconnect = 1;
>  
> +	if (id->driver_info & HUB_QUIRK_DISABLE_AUTOSUSPEND)
> +		pm_runtime_set_autosuspend_delay(&hdev->dev, -1);

This is not a good way to prevent autosuspend, because the user can override it.  If you really want to prevent the hub from autosuspending, you should call usb_autoresume_device() or usb_autopm_get_interface().  Either way, it is then necessary to issue a corresponding call to usb_autosuspend_device() or
usb_autopm_put_interface_no_suspend() later on.

Alan Stern

> +
>  	if (hub_configure(hub, &desc->endpoint[0].desc) >= 0)
>  		return 0;
>  
> @@ -5313,6 +5318,10 @@ static void hub_event(struct work_struct *work)  
> }
>  
>  static const struct usb_device_id hub_id_table[] = {
> +    { .match_flags = USB_DEVICE_ID_MATCH_VENDOR | USB_DEVICE_ID_MATCH_INT_CLASS,
> +      .idVendor = USB_VENDOR_SMSC,
> +      .bInterfaceClass = USB_CLASS_HUB,
> +      .driver_info = HUB_QUIRK_DISABLE_AUTOSUSPEND},
>      { .match_flags = USB_DEVICE_ID_MATCH_VENDOR
>  			| USB_DEVICE_ID_MATCH_INT_CLASS,
>        .idVendor = USB_VENDOR_GENESYS_LOGIC,
>