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

Reply via email to