ACK: [PATCH 1/1][SRU][J][K][L][OEM-6.1] r8152: add USB device driver for config selection
Stefan Bader
stefan.bader at canonical.com
Tue Jul 18 08:43:36 UTC 2023
On 22.05.23 09:57, AceLan Kao wrote:
> From: Bjørn Mork <bjorn at mork.no>
>
> BugLink: https://launchpad.net/bugs/2020295
>
> Subclassing the generic USB device driver to override the
> default configuration selection regardless of matching interface
> drivers.
>
> The r815x family devices expose a vendor specific function which
> the r8152 interface driver wants to handle. This is the preferred
> device mode. Additionally one or more USB class functions are
> usually supported for hosts lacking a vendor specific driver. The
> choice is USB configuration based, with one alternate function per
> configuration.
>
> Example device with both NCM and ECM alternate cfgs:
>
> T: Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 4 Spd=5000 MxCh= 0
> D: Ver= 3.20 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 9 #Cfgs= 3
> P: Vendor=0bda ProdID=8156 Rev=31.00
> S: Manufacturer=Realtek
> S: Product=USB 10/100/1G/2.5G LAN
> S: SerialNumber=001000001
> C:* #Ifs= 1 Cfg#= 1 Atr=a0 MxPwr=256mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=00 Driver=r8152
> E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms
> E: Ad=83(I) Atr=03(Int.) MxPS= 2 Ivl=128ms
> C: #Ifs= 2 Cfg#= 2 Atr=a0 MxPwr=256mA
> I: If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=
> E: Ad=83(I) Atr=03(Int.) MxPS= 16 Ivl=128ms
> I: If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=
> I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=
> E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms
> C: #Ifs= 2 Cfg#= 3 Atr=a0 MxPwr=256mA
> I: If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=06 Prot=00 Driver=
> E: Ad=83(I) Atr=03(Int.) MxPS= 16 Ivl=128ms
> I: If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=00 Driver=
> I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=
> E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms
>
> A problem with this is that Linux will prefer class functions over
> vendor specific functions. Using the above example, Linux defaults
> to cfg #2, running the device in a sub-optimal NCM mode.
>
> Previously we've attempted to work around the problem by
> blacklisting the devices in the ECM class driver "cdc_ether", and
> matching on the ECM class function in the vendor specific interface
> driver. The latter has been used to switch back to the vendor
> specific configuration when the driver is probed for a class
> function.
>
> This workaround has several issues;
> - class driver blacklists is additional maintanence cruft in an
> unrelated driver
> - class driver blacklists prevents users from optionally running
> the devices in class mode
> - each device needs double match entries in the vendor driver
> - the initial probing as a class function slows down device
> discovery
>
> Now these issues have become even worse with the introduction of
> firmware supporting both NCM and ECM, where NCM ends up as the
> default mode in Linux. To use the same workaround, we now have
> to blacklist the devices in to two different class drivers and
> add yet another match entry to the vendor specific driver.
>
> This patch implements an alternative workaround strategy -
> independent of the interface drivers. It avoids adding a
> blacklist to the cdc_ncm driver and will let us remove the
> existing blacklist from the cdc_ether driver.
>
> As an additional bonus, removing the blacklists allow users to
> select one of the other device modes if wanted.
>
> Signed-off-by: Bjørn Mork <bjorn at mork.no>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (backported from commit ec51fbd1b8a2bca2948dede99c14ec63dc57ff6b)
> AceLan: conflicts with below commit which introduce a ID to the same table
> in Jammy kernel. Fixed by preserved the ID and changed to the same new
> format.
> 2edbc1459c1b ("r8152: add vendor/device ID pair for Microsoft Devkit")
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
> drivers/net/usb/r8152.c | 115 ++++++++++++++++++++++++++++------------
> 1 file changed, 82 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index cf6941b1d280..b82aeb7be7c0 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -9582,6 +9582,9 @@ static int rtl8152_probe(struct usb_interface *intf,
> if (version == RTL_VER_UNKNOWN)
> return -ENODEV;
>
> + if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
> + return -ENODEV;
> +
> if (!rtl_vendor_mode(intf))
> return -ENODEV;
>
> @@ -9787,43 +9790,35 @@ static void rtl8152_disconnect(struct usb_interface *intf)
> }
> }
>
> -#define REALTEK_USB_DEVICE(vend, prod) { \
> - USB_DEVICE_INTERFACE_CLASS(vend, prod, USB_CLASS_VENDOR_SPEC), \
> -}, \
> -{ \
> - USB_DEVICE_AND_INTERFACE_INFO(vend, prod, USB_CLASS_COMM, \
> - USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE), \
> -}
> -
> /* table of devices that work with this driver */
> static const struct usb_device_id rtl8152_table[] = {
> /* Realtek */
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8050),
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8053),
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8152),
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8153),
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8155),
> - REALTEK_USB_DEVICE(VENDOR_ID_REALTEK, 0x8156),
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8050) },
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8053) },
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8152) },
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8153) },
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8155) },
> + { USB_DEVICE(VENDOR_ID_REALTEK, 0x8156) },
>
> /* Microsoft */
> - REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07ab),
> - REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07c6),
> - REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0927),
> - REALTEK_USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0c5e),
> - REALTEK_USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x304f),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3054),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3062),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3069),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x3082),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7205),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x720c),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x7214),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0x721e),
> - REALTEK_USB_DEVICE(VENDOR_ID_LENOVO, 0xa387),
> - REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041),
> - REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff),
> - REALTEK_USB_DEVICE(VENDOR_ID_TPLINK, 0x0601),
> + { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07ab) },
> + { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x07c6) },
> + { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0927) },
> + { USB_DEVICE(VENDOR_ID_MICROSOFT, 0x0c5e) },
> + { USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x304f) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x3054) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x3062) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x3069) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x3082) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x7205) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x720c) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x7214) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0x721e) },
> + { USB_DEVICE(VENDOR_ID_LENOVO, 0xa387) },
> + { USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041) },
> + { USB_DEVICE(VENDOR_ID_NVIDIA, 0x09ff) },
> + { USB_DEVICE(VENDOR_ID_TPLINK, 0x0601) },
> {}
> };
>
> @@ -9843,7 +9838,61 @@ static struct usb_driver rtl8152_driver = {
> .disable_hub_initiated_lpm = 1,
> };
>
> -module_usb_driver(rtl8152_driver);
> +static int rtl8152_cfgselector_probe(struct usb_device *udev)
> +{
> + struct usb_host_config *c;
> + int i, num_configs;
> +
> + /* The vendor mode is not always config #1, so to find it out. */
> + c = udev->config;
> + num_configs = udev->descriptor.bNumConfigurations;
> + for (i = 0; i < num_configs; (i++, c++)) {
> + struct usb_interface_descriptor *desc = NULL;
> +
> + if (!c->desc.bNumInterfaces)
> + continue;
> + desc = &c->intf_cache[0]->altsetting->desc;
> + if (desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC)
> + break;
> + }
> +
> + if (i == num_configs)
> + return -ENODEV;
> +
> + if (usb_set_configuration(udev, c->desc.bConfigurationValue)) {
> + dev_err(&udev->dev, "Failed to set configuration %d\n",
> + c->desc.bConfigurationValue);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static struct usb_device_driver rtl8152_cfgselector_driver = {
> + .name = MODULENAME "-cfgselector",
> + .probe = rtl8152_cfgselector_probe,
> + .id_table = rtl8152_table,
> + .generic_subclass = 1,
> +};
> +
> +static int __init rtl8152_driver_init(void)
> +{
> + int ret;
> +
> + ret = usb_register_device_driver(&rtl8152_cfgselector_driver, THIS_MODULE);
> + if (ret)
> + return ret;
> + return usb_register(&rtl8152_driver);
> +}
> +
> +static void __exit rtl8152_driver_exit(void)
> +{
> + usb_deregister(&rtl8152_driver);
> + usb_deregister_device_driver(&rtl8152_cfgselector_driver);
> +}
> +
> +module_init(rtl8152_driver_init);
> +module_exit(rtl8152_driver_exit);
>
> MODULE_AUTHOR(DRIVER_AUTHOR);
> MODULE_DESCRIPTION(DRIVER_DESC);
--
- Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230718/bc6dcfb4/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230718/bc6dcfb4/attachment-0001.sig>
More information about the kernel-team
mailing list