usbatm : modprobe & rmmod

[Duncan Sands]

Hi Matthieu,

> > by the way there are several different ways multiple interfaces can come into play:
> > (1) an interface for transmit+receive, an interface for firmware loading, an
> > interface for line state (like the speedtouch).  This fits fine into usbatm2,
> > since there is basically a one-to-one correspondance (ATM device) <-> transmit/receive
> > interface.
> > (2) one interface for transmission, another for reception (like eagle-usb).
> Well is more complicated for eagle-usb : one interface for receive 
> data(bulk or iso), one for incoming message from the modem (statistic + 
> request from the modem to send it another page of the firmware), one for 
> sending data to the modem (ATM frame + firmware page requested)

if I understand right, the modem is not usable unless all three interfaces are
grabbed, and moreover there needs to be coordination between the transmission of
normal packets and transmission of firmware.  How does that work?  Do you just
submit a firmware urb occasionally on the data endpoint?  What if the firmware
bit comes between two urbs making up one data packet?  Or must data transmission
be stopped until the new firmware is uploaded?  What triggers the need for new
firmware?

> the eagle-usb firmware is swapped in "PC" memory because it didn't 
> enough memory for all the code...
> 
> So in this case the interface 1 (sending data) is share between usbatm 
> and mini driver.

I see.

> > I think this can be made to work fine with usbatm2 also: when bind is called on the
> > first interface, it grabs the second interface too (usb_driver_claim_interface).
> Actually don't work because well because need to access private data 
> usb. Also when removing, disconnect will be call 3 times, and usbatm2 
> don't support it...

I don't see that there is any problem (except that you need to be able to get hold of
the struct usb_driver): in the bind method for the first interface that gets probed,
you claim the other two interfaces, and put the usbatm_data as interface data (usb_set_intfdata).
When an interface is unbound, the minidrivers unbind method will be called.  The unbind method
sets the interface data for the other interfaces to NULL (usb_set_intfdata), then releases
the other interfaces.  The reason for setting the interface data to NULL first, is that
way the disconnect routine (which is called when you release the interface) will exit without
doing anything.  The net result is that probe is called once, and grabs three interfaces;
disconnect is called three times, but the second two times (triggered by releasing the
interfaces in the during the first disconnect) nothing happens, so it's like disconnect
only being called once.

> > Unbind drops both interfaces (usb_driver_release_interface).  The only problem is
> > that you need the struct usb_driver, but that's easy to arrange (just stick it in
> > usbatm_data for instance).
> well some time ago usb_driver_release_interface was used, but it was 
> removed from eagle-usb(i don't know why, but i think it caused ooops).

That may have been my fault: I'm the guy who changed the core to make usb_driver_release_interface
call disconnect...  This can cause problems if you're not aware of it.

> But it is not a real problem...

I agree.

> > (3) some other way I didn't think of.
> > 
> > As far as I can see, everything is fine (or can easily be made to be fine) with
> > the usbatm2 setup.  Perhaps Matthieu can comment.
> > 
> 
> Actually it have some problems :
> -no ioctl

What is the ioctl for?

> -pb with claim to other interfaces

See above.

> -pb with atm start : the second stage firmware is load via userspace and 
> ioctl, so I need to find a way to wait in heavy_init or see if we can 
> safely start without second stage firmware.
> wait_event_interruptible should do the job...

Can you explain this please.

> -need to register 2 usb driver in module_init : one for prefirmware 
> device, one for postfirmware device via usbatm_register.

Does this mean the device mutates itself into a new device once some
firmware is loaded?

> -don't allow the use driver_option (give an example this evening)

?

> I think we should dissociate usb_device and usbatm device like done in 
> pci-ide.

OK.

> After that the two model are quite similar : the older do some init 
> before registering and the newer does it in bind/atm_start function.

Yes, there was never much difference.

> Also I like the idea that the refcount is on mini driver, so you can't 
> remove the module, unless all atm connection are closed (I am not sure 
> of this : I only look quickly) and don't like the idea of permanent module.

Do you mean "can remove the module"?  You can't remove the usbatm2 module while
the ATM device exists.  Right now, it continues to exist until the user
has closed all connections, but I think I know how to force the closure
of all connections when the USB device is disconnected.  That doesn't
help with unloading the usbatm2 module though, because of the reference the
ATM later takes to it.  You should be able to unload the minidriver: the ATM
layer has a pointer to the usbatm2 module, not the minidriver.  Hmmm, I need
to analyse whether it is still possible to call into a minidriver module or
not after the device has been disconnected...

> I have one remark for roman : in your clean method you clean things you 
> init in probe : it is a bit strange.

Ciao,

Duncan.