From: Yi Cong This reverts commit c67cc4315a8e605ec875bd3a1210a549e3562ddc. Currently, in the Linux kernel, USB NIC with ASIX chips use the cdc_ncm driver. However, this driver lacks functionality and performs worse than the vendor's proprietary driver. In my testing, I have identified the following issues: 1. The cdc_ncm driver does not support changing the link speed via ethtool because the corresponding callback function is set to NULL. 2. The CDC protocol does not support retrieving the network duplex status. 3. In TCP_RR and UDP_RR tests, the performance of the cdc_ncm driver is significantly lower than that of the vendor's driver: Average of three netperf runs: `netperf -t {TCP/UDP_RR} -H serverIP -l 120` - cdc_ncm.ko: TCP_RR: 740, UDP_RR: 750 - ax88179_178a.ko: TCP_RR: 8900, UDP_RR: 9200 Issues related to the vendor's driver ax88179_178a.ko will be addressed in the next patch. Signed-off-by: Yi Cong --- drivers/net/usb/ax88179_178a.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index b034ef8a73ea..29cbe9ddd610 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1885,55 +1885,55 @@ static const struct driver_info at_umc2000sp_info = { static const struct usb_device_id products[] = { { /* ASIX AX88179 10/100/1000 */ - USB_DEVICE_AND_INTERFACE_INFO(0x0b95, 0x1790, 0xff, 0xff, 0), + USB_DEVICE(0x0b95, 0x1790), .driver_info = (unsigned long)&ax88179_info, }, { /* ASIX AX88178A 10/100/1000 */ - USB_DEVICE_AND_INTERFACE_INFO(0x0b95, 0x178a, 0xff, 0xff, 0), + USB_DEVICE(0x0b95, 0x178a), .driver_info = (unsigned long)&ax88178a_info, }, { /* Cypress GX3 SuperSpeed to Gigabit Ethernet Bridge Controller */ - USB_DEVICE_AND_INTERFACE_INFO(0x04b4, 0x3610, 0xff, 0xff, 0), + USB_DEVICE(0x04b4, 0x3610), .driver_info = (unsigned long)&cypress_GX3_info, }, { /* D-Link DUB-1312 USB 3.0 to Gigabit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x4a00, 0xff, 0xff, 0), + USB_DEVICE(0x2001, 0x4a00), .driver_info = (unsigned long)&dlink_dub1312_info, }, { /* Sitecom USB 3.0 to Gigabit Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x0df6, 0x0072, 0xff, 0xff, 0), + USB_DEVICE(0x0df6, 0x0072), .driver_info = (unsigned long)&sitecom_info, }, { /* Samsung USB Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x04e8, 0xa100, 0xff, 0xff, 0), + USB_DEVICE(0x04e8, 0xa100), .driver_info = (unsigned long)&samsung_info, }, { /* Lenovo OneLinkDock Gigabit LAN */ - USB_DEVICE_AND_INTERFACE_INFO(0x17ef, 0x304b, 0xff, 0xff, 0), + USB_DEVICE(0x17ef, 0x304b), .driver_info = (unsigned long)&lenovo_info, }, { /* Belkin B2B128 USB 3.0 Hub + Gigabit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x050d, 0x0128, 0xff, 0xff, 0), + USB_DEVICE(0x050d, 0x0128), .driver_info = (unsigned long)&belkin_info, }, { /* Toshiba USB 3.0 GBit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x0930, 0x0a13, 0xff, 0xff, 0), + USB_DEVICE(0x0930, 0x0a13), .driver_info = (unsigned long)&toshiba_info, }, { /* Magic Control Technology U3-A9003 USB 3.0 Gigabit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x0711, 0x0179, 0xff, 0xff, 0), + USB_DEVICE(0x0711, 0x0179), .driver_info = (unsigned long)&mct_info, }, { /* Allied Telesis AT-UMC2000 USB 3.0/USB 3.1 Gen 1 to Gigabit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x07c9, 0x000e, 0xff, 0xff, 0), + USB_DEVICE(0x07c9, 0x000e), .driver_info = (unsigned long)&at_umc2000_info, }, { /* Allied Telesis AT-UMC200 USB 3.0/USB 3.1 Gen 1 to Fast Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x07c9, 0x000f, 0xff, 0xff, 0), + USB_DEVICE(0x07c9, 0x000f), .driver_info = (unsigned long)&at_umc200_info, }, { /* Allied Telesis AT-UMC2000/SP USB 3.0/USB 3.1 Gen 1 to Gigabit Ethernet Adapter */ - USB_DEVICE_AND_INTERFACE_INFO(0x07c9, 0x0010, 0xff, 0xff, 0), + USB_DEVICE(0x07c9, 0x0010), .driver_info = (unsigned long)&at_umc2000sp_info, }, { }, -- 2.25.1 From: Yi Cong Some vendors' USB network interface controllers (NICs) may be compatible with multiple drivers. I consulted with relevant vendors. Taking the AX88179 chip as an example, NICs based on this chip may be used across various OS—for instance, cdc_ncm is used on macOS, while ax88179_178a.ko is the intended driver on Linux (despite a previous patch having disabled it). Therefore, the firmware must support multiple protocols. Currently, both cdc_ncm and ax88179_178a coexist in the Linux kernel. Supporting both drivers simultaneously leads to the following issues: 1. Inconsistent driver loading order during reboot stress testing: The order in which drivers are loaded can vary across reboots, potentially resulting in the unintended driver being loaded. For example: [ 4.239893] cdc_ncm 2-1:2.0: MAC-Address: c8:a3:62:ef:99:8e [ 4.239897] cdc_ncm 2-1:2.0: setting rx_max = 16384 [ 4.240149] cdc_ncm 2-1:2.0: setting tx_max = 16384 [ 4.240583] cdc_ncm 2-1:2.0 usb0: register 'cdc_ncm' at usb- xxxxx:00-1, CDC NCM, c8:a3:62:ef:99:8e [ 4.240627] usbcore: registered new interface driver cdc_ncm [ 4.240908] usbcore: registered new interface driver ax88179_178a In this case, network connectivity functions, but the cdc_ncm driver is loaded instead of the expected ax88179_178a. 2. Similar issues during cable plug/unplug testing: The same race condition can occur when reconnecting the USB device: [ 79.879922] usb 4-1: new SuperSpeed USB device number 3 using xhci_hcd [ 79.905168] usb 4-1: New USB device found, idVendor=0b95, idProduct= 1790, bcdDevice= 2.00 [ 79.905185] usb 4-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 79.905191] usb 4-1: Product: AX88179B [ 79.905198] usb 4-1: Manufacturer: ASIX [ 79.905201] usb 4-1: SerialNumber: 00EF998E [ 79.915215] ax88179_probe, bConfigurationValue:2 [ 79.952638] cdc_ncm 4-1:2.0: MAC-Address: c8:a3:62:ef:99:8e [ 79.952654] cdc_ncm 4-1:2.0: setting rx_max = 16384 [ 79.952919] cdc_ncm 4-1:2.0: setting tx_max = 16384 [ 79.953598] cdc_ncm 4-1:2.0 eth0: register 'cdc_ncm' at usb-0000:04: 00.2-1, CDC NCM (NO ZLP), c8:a3:62:ef:99:8e [ 79.954029] cdc_ncm 4-1:2.0 eth0: unregister 'cdc_ncm' usb-0000:04: 00.2-1, CDC NCM (NO ZLP) At this point, the network becomes unusable. To resolve these issues, introduce a *quirks* mechanism into the usbnet module. By adding chip-specific identification within the generic usbnet framework, we can skip the usbnet probe process for devices that require a dedicated driver. v2: Correct the description of usbnet_quirks.h and modify the code style v3: Add checking whether the CONFIG_USB_NET_AX88179_178A is enabled Signed-off-by: Yi Cong --- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/usb/usbnet.c | 14 +++++++++++ drivers/net/usb/usbnet_quirks.h | 41 +++++++++++++++++++++++++++++++++ include/linux/usb/usbnet.h | 2 ++ 4 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 drivers/net/usb/usbnet_quirks.h diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index 5d123df0a866..6fa03e5bd054 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -2117,7 +2117,7 @@ MODULE_DEVICE_TABLE(usb, cdc_devs); static struct usb_driver cdc_ncm_driver = { .name = "cdc_ncm", .id_table = cdc_devs, - .probe = usbnet_probe, + .probe = usbnet_probe_quirks, .disconnect = usbnet_disconnect, .suspend = usbnet_suspend, .resume = usbnet_resume, diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 511c4154cf74..51ba466057f9 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -31,6 +31,7 @@ #include #include +#include "usbnet_quirks.h" /*-------------------------------------------------------------------------*/ /* @@ -1697,6 +1698,19 @@ static const struct device_type wwan_type = { .name = "wwan", }; +int usbnet_probe_quirks(struct usb_interface *udev, + const struct usb_device_id *prod) +{ + /* Should it be ignored? */ + if (unlikely(usbnet_ignore(udev))) { + dev_dbg(&udev->dev, "usbnet ignore this device!\n"); + return -ENODEV; + } + + return usbnet_probe(udev, prod); +} +EXPORT_SYMBOL_GPL(usbnet_probe_quirks); + int usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod) { diff --git a/drivers/net/usb/usbnet_quirks.h b/drivers/net/usb/usbnet_quirks.h new file mode 100644 index 000000000000..859004046a86 --- /dev/null +++ b/drivers/net/usb/usbnet_quirks.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * A collection of chip information to be ignored + */ + +#ifndef __USB_NET_IGNORE_H__ +#define __USB_NET_IGNORE_H__ + +#include + +/* usbnet_ignore_list: + * Chip info which already support int vendor specific driver, + * and then should be ignored in generic usbnet + */ +static const struct usb_device_id usbnet_ignore_list[] = { +#if IS_ENABLED(CONFIG_USB_NET_AX88179_178A) + /* Chips already support in ax88179_178a.c */ + { USB_DEVICE(0x0b95, 0x1790) }, + { USB_DEVICE(0x0b95, 0x178a) }, + { USB_DEVICE(0x04b4, 0x3610) }, + { USB_DEVICE(0x2001, 0x4a00) }, + { USB_DEVICE(0x0df6, 0x0072) }, + { USB_DEVICE(0x04e8, 0xa100) }, + { USB_DEVICE(0x17ef, 0x304b) }, + { USB_DEVICE(0x050d, 0x0128) }, + { USB_DEVICE(0x0930, 0x0a13) }, + { USB_DEVICE(0x0711, 0x0179) }, + { USB_DEVICE(0x07c9, 0x000e) }, + { USB_DEVICE(0x07c9, 0x000f) }, + { USB_DEVICE(0x07c9, 0x0010) }, + /* End of support in ax88179_178a.c */ +#endif + + { } /*END*/ +}; + +static inline bool usbnet_ignore(struct usb_interface *intf) +{ + return !!usb_match_id(intf, usbnet_ignore_list); +} +#endif diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h index a2d54122823d..de198fcaf76d 100644 --- a/include/linux/usb/usbnet.h +++ b/include/linux/usb/usbnet.h @@ -188,6 +188,8 @@ struct driver_info { * much everything except custom framing and chip-specific stuff. */ extern int usbnet_probe(struct usb_interface *, const struct usb_device_id *); +extern int usbnet_probe_quirks(struct usb_interface *udev, + const struct usb_device_id *prod); extern int usbnet_suspend(struct usb_interface *, pm_message_t); extern int usbnet_resume(struct usb_interface *); extern void usbnet_disconnect(struct usb_interface *); -- 2.25.1 From: Yi Cong A similar reason was raised in commit ec51fbd1b8a2 ("r8152: add USB device driver for config selection"): Linux prioritizes probing non-vendor-specific configurations. Referring to the implementation of this patch, cfgselect is also used for ax88179 to override the default configuration selection. v2: fix warning from checkpatch Signed-off-by: Yi Cong --- drivers/net/usb/ax88179_178a.c | 70 ++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 29cbe9ddd610..f2e86b9256dc 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -14,6 +14,7 @@ #include #include +#define MODULENAME "ax88179_178a" #define AX88179_PHY_ID 0x03 #define AX_EEPROM_LEN 0x100 #define AX88179_EEPROM_MAGIC 0x17900b95 @@ -1713,6 +1714,14 @@ static int ax88179_stop(struct usbnet *dev) return 0; } +static int ax88179_probe(struct usb_interface *intf, const struct usb_device_id *i) +{ + if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC) + return -ENODEV; + + return usbnet_probe(intf, i); +} + static const struct driver_info ax88179_info = { .description = "ASIX AX88179 USB 3.0 Gigabit Ethernet", .bind = ax88179_bind, @@ -1941,9 +1950,9 @@ static const struct usb_device_id products[] = { MODULE_DEVICE_TABLE(usb, products); static struct usb_driver ax88179_178a_driver = { - .name = "ax88179_178a", + .name = MODULENAME, .id_table = products, - .probe = usbnet_probe, + .probe = ax88179_probe, .suspend = ax88179_suspend, .resume = ax88179_resume, .reset_resume = ax88179_resume, @@ -1952,7 +1961,62 @@ static struct usb_driver ax88179_178a_driver = { .disable_hub_initiated_lpm = 1, }; -module_usb_driver(ax88179_178a_driver); +static int ax88179_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 ax88179_cfgselector_driver = { + .name = MODULENAME "-cfgselector", + .probe = ax88179_cfgselector_probe, + .id_table = products, + .generic_subclass = 1, + .supports_autosuspend = 1, +}; + +static int __init ax88179_driver_init(void) +{ + int ret; + + ret = usb_register_device_driver(&ax88179_cfgselector_driver, THIS_MODULE); + if (ret) + return ret; + return usb_register(&ax88179_178a_driver); +} + +static void __exit ax88179_driver_exit(void) +{ + usb_deregister(&ax88179_178a_driver); + usb_deregister_device_driver(&ax88179_cfgselector_driver); +} + +module_init(ax88179_driver_init); +module_exit(ax88179_driver_exit); MODULE_DESCRIPTION("ASIX AX88179/178A based USB 3.0/2.0 Gigabit Ethernet Devices"); MODULE_LICENSE("GPL"); -- 2.25.1