Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: > On Thu, 26 Jul 2012, Bjørn Mork wrote: >> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: >> >> > In fact, I would say that the three loops in stop_read_write_urbs() and >> > usb_wwan_release() could be combined into a single loop that should run >> > in the port_remove callback. >> >> Maybe something like this would do? It is based on usb-3.6-rc1, but will >> also apply to the current linux-next (next-20120726). > > This seems more or less reasonable. In theory, stop_read_write_urbs() > could be moved directly into usb_wwan_suspend() since it isn't used > anywhere else now. Yes, I thought about it but didn't see any advantage to doing that so I just moved it as it is, thinking that it would make reviewing this easier of nothing else. >> I did not know exactly how to handle the suspend/resume issues, so I took >> the absolute minimum approach to avoid Ooopsing if the device is suspended >> when the module is unloaded. This is split out in a separate patch. Should >> probably be polished a bit. > > In theory it's also possible for the suspend callback to run after > port_remove, so you should add a similar test to stop_read_write_urbs, > wherever that routine ends up. OK. > There is a possible race here: The suspend or resume callback could run > at the same time as the port_remove callback. Perhaps > usb_serial_device_probe() and usb_serial_device_remove() should wrap > everything in usb_autopm_get_interface()/usb_autopm_put_interface(). Great idea! I'll give that a try. Thanks for the fast review. 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