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

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

On Tue, 30 Sep 2025 10:57:05 +0200, Oliver Neukum wrote:
> > +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);  
> 
> Missing error handling. If you cannot register ax88179_178a_driver
> you definitely do not want to keep ax88179_cfgselector_driver
> 
> > +}
> > +
> > +static void __exit ax88179_driver_exit(void)
> > +{
> > +	usb_deregister(&ax88179_178a_driver);  
> 
> The window for the race
> 
> > +	usb_deregister_device_driver(&ax88179_cfgselector_driver);  
> 
> Wrong order. I you remove ax88179_178a_driver before you remove
> ax88179_cfgselector_driver, you'll leave a window during which
> devices would be switched to a mode no driver exists for.

Hmm, what about registration?

I added msleep(1000) and simulated usb_register() error, then
cfgselector binds to the device and switches configuration before
the interface driver is available. But the module fails to load
(I fixed this) and device is left with no driver whatsoever.

Moreover, according to c67cc4315a8e, config switch is irreversible
since the device reconnects with only the vendor config available.
I can't test it because my device doesn't have a CDC config at all.

There is a gotcha. I tried to test in a realistic scenario: device
hotplug, module not loaded yet. I found that udev apparently retries
loading the module, so this state would be fixed unless the module
init error is persistent. Still, better not to rely on this?

Would it make sense to swap registration order?

Regards,
Michal