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

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

[Hardik Gajjar]

Renesas R-Car H3ULCB + Kingfisher Infotainment Board is either not able
to detect the USB3.0 mass storage devices or is detecting those as
USB2.0 high speed devices.

The explanation given by Renesas is that, due to a HW issue, the XHCI
driver does not wake up after going to sleep on connecting a USB3.0
device.

In order to mitigate that, disable the auto-suspend feature
specifically for SMSC hubs from hub_probe() function, as a quirk.

Renesas Kingfisher Infotainment Board has two USB3.0 ports (CN2) which
are connected via USB5534B 4-port SuperSpeed/Hi-Speed, low-power,
configurable hub controller.

[1] SanDisk USB 3.0 device detected as USB-2.0 before the patch
 [   74.036390] usb 5-1.1: new high-speed USB device number 4 using xhci-hcd
 [   74.061598] usb 5-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
 [   74.069976] usb 5-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
 [   74.077303] usb 5-1.1: Product: Ultra
 [   74.080980] usb 5-1.1: Manufacturer: SanDisk
 [   74.085263] usb 5-1.1: SerialNumber: 4C530001110208116550

[2] SanDisk USB 3.0 device detected as USB-3.0 after the patch
 [   34.565078] usb 6-1.1: new SuperSpeed Gen 1 USB device number 3 using xhci-hcd
 [   34.588719] usb 6-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
 [   34.597098] usb 6-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
 [   34.604430] usb 6-1.1: Product: Ultra
 [   34.608110] usb 6-1.1: Manufacturer: SanDisk
 [   34.612397] usb 6-1.1: SerialNumber: 4C530001110208116550

Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

---
Changes in v4:
 - [Eugeniu Rosca] Corrected indentation in hub.h

Changes in v3:
 - [Alan Stern] Called usb_autopm_put_interface() from
   hub_disconnect() to enable auto suspend for interface.
 - [v3] https://lore.kernel.org/linux-renesas-soc/1580838253-31822-1-git-send-email-hgajjar@de.adit-jv.com/

Changes in v2:
 - [Alan Stern] Switched from pm_runtime_set_autosuspend_delay()
   to usb_autopm_get_interface()
 - Improved commit description
 - Rebased against v5.5
 - [v2] https://lore.kernel.org/linux-renesas-soc/1580403994-21076-1-git-send-email-hgajjar@de.adit-jv.com/

 drivers/usb/core/hub.c | 15 +++++++++++++++
 drivers/usb/core/hub.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3405b14..de94fa4 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -38,7 +38,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_TP_TRANSMISSION_DELAY	40	/* ns */
 #define USB_TP_TRANSMISSION_DELAY_MAX	65535	/* ns */
@@ -1731,6 +1733,10 @@ static void hub_disconnect(struct usb_interface *intf)
 	kfree(hub->buffer);
 
 	pm_suspend_ignore_children(&intf->dev, false);
+
+	if (hub->quirk_disable_autosuspend)
+		usb_autopm_put_interface(intf);
+
 	kref_put(&hub->kref, hub_release);
 }
 
@@ -1863,6 +1869,11 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
 		hub->quirk_check_port_auto_suspend = 1;
 
+	if (id->driver_info & HUB_QUIRK_DISABLE_AUTOSUSPEND) {
+		hub->quirk_disable_autosuspend = 1;
+		usb_autopm_get_interface(intf);
+	}
+
 	if (hub_configure(hub, &desc->endpoint[0].desc) >= 0)
 		return 0;
 
@@ -5599,6 +5610,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,
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index a9e24e4..a97dd1b 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -61,6 +61,7 @@ struct usb_hub {
 	unsigned		quiescing:1;
 	unsigned		disconnected:1;
 	unsigned		in_reset:1;
+	unsigned		quirk_disable_autosuspend:1;
 
 	unsigned		quirk_check_port_auto_suspend:1;
 
-- 
2.7.4

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

[Eugeniu Rosca]

Hi Alan, hi Greg,

On Thu, Feb 06, 2020 at 12:49:23PM +0100, Hardik Gajjar wrote:
> Renesas R-Car H3ULCB + Kingfisher Infotainment Board is either not able
> to detect the USB3.0 mass storage devices or is detecting those as
> USB2.0 high speed devices.
> 
> The explanation given by Renesas is that, due to a HW issue, the XHCI
> driver does not wake up after going to sleep on connecting a USB3.0
> device.
> 
> In order to mitigate that, disable the auto-suspend feature
> specifically for SMSC hubs from hub_probe() function, as a quirk.
> 
> Renesas Kingfisher Infotainment Board has two USB3.0 ports (CN2) which
> are connected via USB5534B 4-port SuperSpeed/Hi-Speed, low-power,
> configurable hub controller.
> 
> [1] SanDisk USB 3.0 device detected as USB-2.0 before the patch
>  [   74.036390] usb 5-1.1: new high-speed USB device number 4 using xhci-hcd
>  [   74.061598] usb 5-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
>  [   74.069976] usb 5-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>  [   74.077303] usb 5-1.1: Product: Ultra
>  [   74.080980] usb 5-1.1: Manufacturer: SanDisk
>  [   74.085263] usb 5-1.1: SerialNumber: 4C530001110208116550
> 
> [2] SanDisk USB 3.0 device detected as USB-3.0 after the patch
>  [   34.565078] usb 6-1.1: new SuperSpeed Gen 1 USB device number 3 using xhci-hcd
>  [   34.588719] usb 6-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
>  [   34.597098] usb 6-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>  [   34.604430] usb 6-1.1: Product: Ultra
>  [   34.608110] usb 6-1.1: Manufacturer: SanDisk
>  [   34.612397] usb 6-1.1: SerialNumber: 4C530001110208116550
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Is there anything else we can do to see the patch accepted?
Do you think it is also relevant for the stable tree?

-- 
Best Regards
Eugeniu Rosca

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

[Greg Kroah-Hartman]

On Mon, Feb 10, 2020 at 04:28:08PM +0100, Eugeniu Rosca wrote:
> Hi Alan, hi Greg,
> 
> On Thu, Feb 06, 2020 at 12:49:23PM +0100, Hardik Gajjar wrote:
> > Renesas R-Car H3ULCB + Kingfisher Infotainment Board is either not able
> > to detect the USB3.0 mass storage devices or is detecting those as
> > USB2.0 high speed devices.
> > 
> > The explanation given by Renesas is that, due to a HW issue, the XHCI
> > driver does not wake up after going to sleep on connecting a USB3.0
> > device.
> > 
> > In order to mitigate that, disable the auto-suspend feature
> > specifically for SMSC hubs from hub_probe() function, as a quirk.
> > 
> > Renesas Kingfisher Infotainment Board has two USB3.0 ports (CN2) which
> > are connected via USB5534B 4-port SuperSpeed/Hi-Speed, low-power,
> > configurable hub controller.
> > 
> > [1] SanDisk USB 3.0 device detected as USB-2.0 before the patch
> >  [   74.036390] usb 5-1.1: new high-speed USB device number 4 using xhci-hcd
> >  [   74.061598] usb 5-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> >  [   74.069976] usb 5-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> >  [   74.077303] usb 5-1.1: Product: Ultra
> >  [   74.080980] usb 5-1.1: Manufacturer: SanDisk
> >  [   74.085263] usb 5-1.1: SerialNumber: 4C530001110208116550
> > 
> > [2] SanDisk USB 3.0 device detected as USB-3.0 after the patch
> >  [   34.565078] usb 6-1.1: new SuperSpeed Gen 1 USB device number 3 using xhci-hcd
> >  [   34.588719] usb 6-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> >  [   34.597098] usb 6-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> >  [   34.604430] usb 6-1.1: Product: Ultra
> >  [   34.608110] usb 6-1.1: Manufacturer: SanDisk
> >  [   34.612397] usb 6-1.1: SerialNumber: 4C530001110208116550
> > 
> > Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> > Acked-by: Alan Stern <stern@rowland.harvard.edu>
> > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> 
> Is there anything else we can do to see the patch accepted?
> Do you think it is also relevant for the stable tree?

It was the merge window, I couldn't do anything with it until a few
hours ago.  Please give me some time to catch up with patches...

thanks,

greg k-h

Patchwork summary for: linux-renesas-soc

[patchwork-bot+linux-renesas-soc]

Hello:

The following patches were marked "accepted", because they were applied to
geert/renesas-devel (refs/heads/master):

Patch: [v4] USB: hub: Fix the broken detection of USB3 device in SMSC hub
  Submitter: Hardik Gajjar <hgajjar@de.adit-jv.com>
  Patchwork: https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=237743

Total patches: 1

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/pwbot

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

[Kai-Heng Feng]

Hi Hardik,

This patch prevents my Raven Ridge xHCI from getting runtime suspend.

> On Feb 6, 2020, at 19:49, Hardik Gajjar <hgajjar@de.adit-jv.com> wrote:
> 
> Renesas R-Car H3ULCB + Kingfisher Infotainment Board is either not able
> to detect the USB3.0 mass storage devices or is detecting those as
> USB2.0 high speed devices.
> 
> The explanation given by Renesas is that, due to a HW issue, the XHCI
> driver does not wake up after going to sleep on connecting a USB3.0
> device.

Since the issue is already root caused to xHCI, sounds the workaround should be implemented in xHCI?

Functions like xhci_alloc_dev() can be a better place to instrument the workaround.

Kai-Heng

> 
> In order to mitigate that, disable the auto-suspend feature
> specifically for SMSC hubs from hub_probe() function, as a quirk.
> 
> Renesas Kingfisher Infotainment Board has two USB3.0 ports (CN2) which
> are connected via USB5534B 4-port SuperSpeed/Hi-Speed, low-power,
> configurable hub controller.
> 
> [1] SanDisk USB 3.0 device detected as USB-2.0 before the patch
> [   74.036390] usb 5-1.1: new high-speed USB device number 4 using xhci-hcd
> [   74.061598] usb 5-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> [   74.069976] usb 5-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [   74.077303] usb 5-1.1: Product: Ultra
> [   74.080980] usb 5-1.1: Manufacturer: SanDisk
> [   74.085263] usb 5-1.1: SerialNumber: 4C530001110208116550
> 
> [2] SanDisk USB 3.0 device detected as USB-3.0 after the patch
> [   34.565078] usb 6-1.1: new SuperSpeed Gen 1 USB device number 3 using xhci-hcd
> [   34.588719] usb 6-1.1: New USB device found, idVendor=0781, idProduct=5581, bcdDevice= 1.00
> [   34.597098] usb 6-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [   34.604430] usb 6-1.1: Product: Ultra
> [   34.608110] usb 6-1.1: Manufacturer: SanDisk
> [   34.612397] usb 6-1.1: SerialNumber: 4C530001110208116550
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>
> 
> ---
> Changes in v4:
> - [Eugeniu Rosca] Corrected indentation in hub.h
> 
> Changes in v3:
> - [Alan Stern] Called usb_autopm_put_interface() from
>   hub_disconnect() to enable auto suspend for interface.
> - [v3] https://lore.kernel.org/linux-renesas-soc/1580838253-31822-1-git-send-email-hgajjar@de.adit-jv.com/
> 
> Changes in v2:
> - [Alan Stern] Switched from pm_runtime_set_autosuspend_delay()
>   to usb_autopm_get_interface()
> - Improved commit description
> - Rebased against v5.5
> - [v2] https://lore.kernel.org/linux-renesas-soc/1580403994-21076-1-git-send-email-hgajjar@de.adit-jv.com/
> 
> drivers/usb/core/hub.c | 15 +++++++++++++++
> drivers/usb/core/hub.h |  1 +
> 2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 3405b14..de94fa4 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -38,7 +38,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_TP_TRANSMISSION_DELAY	40	/* ns */
> #define USB_TP_TRANSMISSION_DELAY_MAX	65535	/* ns */
> @@ -1731,6 +1733,10 @@ static void hub_disconnect(struct usb_interface *intf)
> 	kfree(hub->buffer);
> 
> 	pm_suspend_ignore_children(&intf->dev, false);
> +
> +	if (hub->quirk_disable_autosuspend)
> +		usb_autopm_put_interface(intf);
> +
> 	kref_put(&hub->kref, hub_release);
> }
> 
> @@ -1863,6 +1869,11 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
> 	if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
> 		hub->quirk_check_port_auto_suspend = 1;
> 
> +	if (id->driver_info & HUB_QUIRK_DISABLE_AUTOSUSPEND) {
> +		hub->quirk_disable_autosuspend = 1;
> +		usb_autopm_get_interface(intf);
> +	}
> +
> 	if (hub_configure(hub, &desc->endpoint[0].desc) >= 0)
> 		return 0;
> 
> @@ -5599,6 +5610,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,
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index a9e24e4..a97dd1b 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -61,6 +61,7 @@ struct usb_hub {
> 	unsigned		quiescing:1;
> 	unsigned		disconnected:1;
> 	unsigned		in_reset:1;
> +	unsigned		quirk_disable_autosuspend:1;
> 
> 	unsigned		quirk_check_port_auto_suspend:1;
> 
> -- 
> 2.7.4
> 
> 

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

[Eugeniu Rosca]

Hi Kai-Heng,

Many thanks for reporting the issue!

On Tue, May 12, 2020 at 09:36:07PM +0800, Kai-Heng Feng wrote:
> This patch prevents my Raven Ridge xHCI from getting runtime suspend.
> 
> > On Feb 6, 2020, at 19:49, Hardik Gajjar <hgajjar@de.adit-jv.com> wrote:
> > 
> > Renesas R-Car H3ULCB + Kingfisher Infotainment Board is either not able
> > to detect the USB3.0 mass storage devices or is detecting those as
> > USB2.0 high speed devices.
> > 
> > The explanation given by Renesas is that, due to a HW issue, the XHCI
> > driver does not wake up after going to sleep on connecting a USB3.0
> > device.
> 
> Since the issue is already root caused to xHCI, sounds the workaround should be implemented in xHCI?
> 
> Functions like xhci_alloc_dev() can be a better place to instrument the workaround.

To my understanding, based on the USB Vendor ID used in this patch and
on the lsusb output [1] got on the original arm64 board, we are talking
about a hub device [2], hence drivers/usb/core/hub.c seems an
appropriate placement.

> > +#define USB_VENDOR_SMSC				0x0424

Based on the output [1], I believe the quirk could be made specific
to USB Product IDs '2134' and '5534'?

Could you please share the output of 'lsusb | grep 0424' on the machine
you experienced the regression?

Question to both USB and Renesas people. Does anybody expect SMSC hub
Product ID to vary across different Kingfisher [3] board samples?

> > 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},

[1] h3ulcb-kf #> lsusb | grep 0424
    Bus 006 Device 002: ID 0424:5534 Standard Microsystems Corp. Hub
    Bus 005 Device 002: ID 0424:2134 Standard Microsystems Corp. Hub
[2] https://devicehunt.com/search/type/usb/vendor/0424/device/any
[3] https://elinux.org/R-Car/Boards/Kingfisher

-- 
Best regards,
Eugeniu Rosca