Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device driver for config selection

[Michal Pecio <michal.pecio@gmail.com>]

On Sat, 11 Oct 2025 15:53:13 +0800, yicongsrfy@163.com wrote:
> From: Yi Cong <yicong@kylinos.cn>
> 
> 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.
> 
> Signed-off-by: Yi Cong <yicong@kylinos.cn>
> 
> ---
> v2: fix warning from checkpatch.
> v5: 1. use KBUILD_MODNAME to obtain the module name.
>     2. add error handling when usb_register fail.
>     3. use .choose_configuration instead of .probe.
>     4. reorder deregister logic.
> ---
>  drivers/net/usb/ax88179_178a.c | 68 ++++++++++++++++++++++++++++++++--
>  1 file changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index b034ef8a73ea..b6432d414a38 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1713,6 +1713,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);
> +}

This isn't part of the cfgselector driver being added by this commit
nor is it documented in the changelog, so why is it here?

It doesn't seem to be necessary, because USB_DEVICE_AND_INTERFACE_INFO
matches used by this driver ensure that probe() will only be called on
interfaces of the correct class 0xff.

> +
>  static const struct driver_info ax88179_info = {
>  	.description = "ASIX AX88179 USB 3.0 Gigabit Ethernet",
>  	.bind = ax88179_bind,
> @@ -1941,9 +1949,9 @@ static const struct usb_device_id products[] = {
>  MODULE_DEVICE_TABLE(usb, products);
>  
>  static struct usb_driver ax88179_178a_driver = {
> -	.name =		"ax88179_178a",
> +	.name =		KBUILD_MODNAME,
>  	.id_table =	products,
> -	.probe =	usbnet_probe,
> +	.probe =	ax88179_probe,
>  	.suspend =	ax88179_suspend,
>  	.resume =	ax88179_resume,
>  	.reset_resume =	ax88179_resume,
> @@ -1952,7 +1960,61 @@ static struct usb_driver ax88179_178a_driver = {
>  	.disable_hub_initiated_lpm = 1,
>  };
>  
> -module_usb_driver(ax88179_178a_driver);
> +static int ax88179_cfgselector_choose_configuration(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;
> +
> +	return c->desc.bConfigurationValue;
> +}

I wonder how many copies of this code would justify making it some
sort of library in usbnet or usbcore?

> +static struct usb_device_driver ax88179_cfgselector_driver = {
> +	.name =	KBUILD_MODNAME "-cfgselector",
> +	.choose_configuration =	ax88179_cfgselector_choose_configuration,
> +	.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;
> +
> +	ret = usb_register(&ax88179_178a_driver);
> +	if (ret)
> +		usb_deregister_device_driver(&ax88179_cfgselector_driver);

Any problems if the order of registration is reversed, to ensure that
the interface driver always exists if the device driver exists?

> +
> +	return 0;

return ret perhaps?

> +}
> +
> +static void __exit ax88179_driver_exit(void)
> +{
> +	usb_deregister_device_driver(&ax88179_cfgselector_driver);
> +	usb_deregister(&ax88179_178a_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
>