Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
> r8153 on Dell TB dock corrupts rx packets.
> 
> The root cause is not found yet, but disabling rx checksumming can
> workaround the issue. We can use this connection to decide if it's
> a Dell TB dock:
> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> 
> BugLink: https://bugs.launchpad.net/bugs/1729674
> Cc: Mario Limonciello <mario.limonciello@dell.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/net/usb/r8152.c | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index d51d9abf7986..58b80b5e7803 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -27,6 +27,8 @@
>  #include <linux/usb/cdc.h>
>  #include <linux/suspend.h>
>  #include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/usb/hcd.h>
>  
>  /* Information for net-next */
>  #define NETNEXT_VERSION		"09"
> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
>  	return version;
>  }
>  
> +/* Ethernet on Dell TB 15/16 dock is connected this way:
> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
> + * We use this connection to make sure r8153 is on the Dell TB dock.
> + */
> +static bool check_dell_tb_dock(struct usb_device *udev)
> +{
> +	struct usb_device *hub = udev->parent;
> +	struct usb_device *root_hub;
> +	struct pci_dev *controller;
> +
> +	if (!hub)
> +		return false;
> +
> +	if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
> +	      le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
> +		return false;
> +
> +	root_hub = hub->parent;
> +	if (!root_hub || root_hub->parent)
> +		return false;
> +
> +	controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);

That's a very scary, and dangerous, cast.  You can not ever be sure that
the hub really is a "root hub" like this.

> +	if (controller->vendor == 0x1b21 && controller->device == 0x1142)
> +		return true;

Why can't you just look at the USB device itself and go off of a quirk
in it?  Something like a version or string or something else?

This sounds like a USB host controller issue, not a USB device issue,
can't we fix the "real" problem here instead of this crazy work-around?
Odds are any device plugged into the hub should have the same issue,
right?

thanks,

greg k-h