[PATCH] usb-serial: fix shutdown / device_unregister order

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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel