usbatm : modprobe & rmmod

[Duncan Sands]

Hi Roman,

> > The code now shoots down the kernel thread if the usb device is disconnected
> > (which means you should check kthread_should_stop every now and again during
> > firmware initialization - see kthread.h), so this is no longer a problem either.
> 
> Yes I thought about something along these lines but somehow decided that
> doing proper refcounting (which I broke BTW :) and letting the kthread
> die asynchronously was easier, and didn't rely on the heavy_init to be
> nice and return quickly (is the USB subsystem held for the duration of
> the disconnect methods just like it is for probe?).

Yes, that lock is held.  It might be a problem, but I decided the best route was to
try it and see what happens in practice.  Don't forget that once the usb core detects
that the device is disconnected, it blocks urb submission (and causes queued urbs to
fail), so any firmware loading code is likely to fail and exit rather fast.

> Anyway if you change your mind it'll be just a matter of adding a
> cleanup function to usbatm_driver and calling it in
> udsl_destroy_instance, thus lifting up the requirements on the
> minidriver's heavy_init.  All the bookkeeping is already there.

Exactly.

> > [...] As I said to David on IRC, I think you and he were
> > repelled by the "huge heap of crud in a file" nature of usbnet, while missing the
> > nice mini-driver aspect and the simple registration scheme (i.e. the driver_info trick,
> 
> Indeed.  Thanks for the explanation!
> 
> A nitpick re. the code:
> is atm_dev parameter needed for atm_{start,stop} methods of
> usbatm_driver?  It can be made always available in usbatm_data, and can
> be reset to NULL in usbatm_atm_init if atm_start method failed.
> Otherwise kill line 988 :)

The problem here is that instance->atm_dev is being used to work around a race in the
ATM layer: in theory, the ATM device can be opened and used the moment atm_dev_register
returns - in particular, before we've had a chance to initialise our fields in it!  To
get around this, all our ATM callbacks check to see if instance->atm_dev is NULL - and
if so they bail out.  Thus instance->atm_dev is being used as a flag indicating whether
we have finished initialising the ATM device or not.  This is natural, because in any
case the callbacks have to check whether it is NULL, because even if we set instance->atm_dev
right away, they might (*in theory*) be called between the return from atm_dev_register
and the execution of the assignment.  Still, we could also have a flag in the instance
data, and check that as well, in which case the mini-driver could just get the atm_dev
parameter from the usbatm instance as you suggest.

By the way, it's not clear to me yet which bits of usbatm's data should be made available
to mini-drivers (i.e. be part of the API), and which bits should be considered private.

> Starting to code cxacru2,

Great!

D.