Dan Williams <dcbw@xxxxxxxxxx> writes: > On Fri, 2013-01-04 at 23:16 +0100, Oliver Neukum wrote: >> On Friday 04 January 2013 10:48:16 Dan Williams wrote: >> > Some drivers (ex sierra_net) need the status interrupt URB >> > active even when the device is closed, because they receive >> > custom indications from firmware. Allow sub-drivers to set >> > a flag that submits the status interrupt URB on probe and >> > keeps the URB alive over device open/close. The URB is still >> > killed/re-submitted for suspend/resume, as before. >> > >> > Signed-off-by: Dan Williams <dcbw@xxxxxxxxxx> >> > --- >> > Oliver: alternatively, is there a problem with *always* >> > submitting the interrupt URB, and then simply not calling >> > the subdriver's .status function when the netdev is >> > closed? That would be a much simpler patch. >> >> That is quite radical. We have no idea what a device >> does when we do not react to a status update. I would >> much prefer to not take the risk. >> Besides, we don't use bandwidth if we don't have to. > > Ok, so scratch the alternative. Thus, does the posted patch look like > the right course of action? > > If I wasn't clear enough before, sierra_net needs to listen to the > status interrupt URB to receive the custom Restart indication as part of > the driver's device setup. Thus for sierra_net at least, tying the > status interrupt URB submission to device open/close isn't right. > > I'd previously done a patch to handle this all in sierra_net, but the > problem there is suspend/resume: without directly accessing the usbnet > structure's ->suspend_count member (icky!) sierra_net can't correctly > kill/submit the URB itself. So I went with a flag to usbnet that Sierra > can set. Just a few random thoughts... The sierra_net driver would have been an excellent candidate for the cdc-wdm subdriver model if that had existed when sierra_net was written. I assume the current driver only implements a minimal subset of the DirectIP HIP protocol embedded in CDC, and exporting this to userspace instead would have made it possible to extend that support without changing the driver, in addtion to making the driver much more robust against firmware differences. It would have eliminated the problem you are facing and removed the minidriver workqueue complexity. But I guess it's too late to change this now. In theory we could implement the cdc-wdm hooks that were initially proposed for cdc_mbim and rewrite sierra_net to use it while being backwards compatible. Don't think anyone wants to do either, so forget about it... You can still use a trick similar to what qmi_wwan and cdc_mbim does to take over the status endpoint from usbnet: By not implementing .status, and possibly setting dev->status to NULL in .bind, you are free to handle the status endpoint entirely inside the minidriver. Not sure if that is smart though. You would have to reimplement init_status and intr_complete from usbnet, and kill or resubmit the interrupt urb on suspend/resume/disconnect yourself. The new usbnet flag is probably a better solution. FWIW, I agree with Oliver that always submitting the interrupt URB is both risky and will cause too much unnecessary USB activity for most usbnet devices. 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
