On Thu, Feb 22, 2007 at 04:06:55PM -0800, Greg KH wrote: > On Thu, Feb 22, 2007 at 01:59:58PM -0800, Jim Radford wrote: > > One problem with this is that ->shutdown(), which cleans up my > > pointer, is called before device_unregister(). So now we have:
> > ->attach() > > dev_set_drvdata(priv) > > device_register() > > ->port_probe(priv) > > > > ->shutdown() > > dev_set_drvdata(NULL) > > device_unregister() > > ->port_remove(NULL) // BOOM! > > which is asymmetric. I think one pair should be reversed. I'm > > guessing the latter since you didn't like my patch to reverse the > > former. :) I don't have enough context to decide, so let me know and > > I'll work up a patch. > I don't think anyone has used these callbacks yet :) I noticed. :) Another problem I have is that when I try to remove the module without unplugging my device my ->port_remove() callback doesn't get called. module_exit __try_stop_module() usb_serial_device_remove module_get // this fails because we are in module_exit ->port_remove() // so this never gets called Along the same lines, do I need to specifically call device_remove_file from ftdi_sio_port_remove given that the attr files seem to disappear automatically? In particular, can my show/store attr functions ever be called again after ->port_remove() if I don't remove the attrs explicitly? > Yes, I think the last one should be switched, good catch. Ensure that the ->port_remove() callbacks get called before the ->shutdown() callback which makeing the order symmetric with ->attach() being called before ->port_probe(). Signed-Off: Jim Radford <[EMAIL PROTECTED]> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 6bf22a2..50f10c0 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -135,11 +137,6 @@ static void destroy_serial(struct kref *kref) dbg("%s - %s", __FUNCTION__, serial->type->description); - serial->type->shutdown(serial); - - /* return the minor range that this device had */ - return_serial(serial); - for (i = 0; i < serial->num_ports; ++i) serial->port[i]->open_count = 0; @@ -150,6 +147,12 @@ static void destroy_serial(struct kref *kref) serial->port[i] = NULL; } + if (serial->type->shutdown) + serial->type->shutdown(serial); + + /* return the minor range that this device had */ + return_serial(serial); + /* If this is a "fake" port, we have to clean it up here, as it will * not get cleaned up in port_release() as it was never registered with * the driver core */ ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel