Marcel Holtmann <marcel@xxxxxxxxxxxx> writes:
> You could just register a callback notification after the network driver
> parsed the interrupt URB and decided it is not useful.
It's actually easier to do it the other way round, as the core usbnet
doesn't use the notifications for anything useful anyway and the usbnet
minidrivers which do care already provide a callback hook.
Besides, the real problem is knowing when (not) to kill the urb, and
that seems easier to handle in the cdc-wdm code. The usbnet code will
happily kill it all over the place, including dropping to resubmit
inside the completion handler if netif_running() says the network is
down. That makes it difficult to add the needed "do not kill if there
are open files" test. cdc-wdm is much cleaner in this aspect, and it
should be easy to add the test for a running network interface if
required. But I don't think it is unless we want to support usbnet
minidrivers requiring it.
> I would also almost say it is fine to integrate cdc-wdm support directly
> into the network driver and activate it for certain VID:PID
> combinations.
Yeah, after playing with this it seems very easy. Just leave the
interrupt endpoint for cdc-wdm to play with and let usbnet handle the
bulk endpoints. Not much job really: add cdc-wdm probe() and
disconnect() alternatives allowing this, and probably review all the pm
and reset methods to see if they need to know about the driver symbiosis.
Only real challenges are
- only one driver/module can be allowed to do usb_set_intfdata(), and I
do not want to merge usbnet and cdc-wdm to the point that they both
use the same intfdata struct
- using usb_register_dev() prevents us from using the normal
container_of(inode->i_cdev, ..) trick as the cdev is hidden somewhere
in the core usb code. This leaves us with the usb-minor being the
only usable piece of information to wdm_open(). That wouldn't be a
problem if it wasn't for the fact that usb_find_interface() insists
on doing driver matching. It will not consider interfaces owned by
other drivers, even if they have the correct minor. Not a problem as
long as wdm_open() is specific to a single driver, but that is
excactly what I'm trying to change
Currently wdm_open() does (with error handling removed for simplicity):
minor = iminor(inode);
intf = usb_find_interface(&wdm_driver, minor);
desc = usb_get_intfdata(intf);
I'm open to suggestions on how to work around that in an acceptable way.
Remember that this code may have to deal with interfaces owned by a
number of different drivers, if we are to allow sharing the interface
with any usbnet based driver. In theory there could be a new driver for
every minor...
The only real solution I've come up with doesn't look very nice:
Register the mapping between minor and driver and/or interface or
intfdata in a static table or list after calling usb_register_dev().
Which is *so* 2011. Or maybe even more ancient than that - what do I
know? Anyway, I don't like module global state.
Hmm, while writing this, another idea occured to me: We could require
every network driver wanting to use cdc-wdm to provide a driver specific
open(), or maybe a hook function for wdm_open()? That's probably better,
or do people spill their coffee all over the keyboard when they see
something like that.
> You get USB_CDC_NOTIFY_NETWORK_CONNECTION,
Doesn't look like it. At least I haven't seen one yet from this modem.
They might have just dropped that assuming that the managing app would
use the QMI notification anyway. But it doesn't matter. We'll just
pretend to be connected and let the modem manager sort out the details.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
