Some time ago I sent to Greg a backport of the code in 2.5, to allow a
user program to request a disconnect of a device from the usbfs, so
that the user program could make use of the device. I took the code
from 2.5.32 (which was the current 2.5 kernel at that time), to try to 
get the locking correct and applied it to the 2.4.18 kernel. There 
were some comments from the list about several parts. I believe that 
everything except the code locking in devio.c has been addressed, and 
I don't know how to handle that. Either there have been some changes 
in the locking since 2.5.32, or I didn't get it right the first time. 
The original patch and comments are below:



   *************** original patch **************


diff -urN vanilla/drivers/usb/devio.c patched/drivers/usb/devio.c
--- vanilla/drivers/usb/devio.c Sun Jun  2 17:37:47 2002
+++ patched/drivers/usb/devio.c Fri Aug 30 15:16:53 2002
@@ -43,7 +43,7 @@
   #include <linux/usb.h>
   #include <linux/usbdevice_fs.h>
   #include <asm/uaccess.h>
-
+#include <linux/module.h>

   struct async {
           struct list_head asynclist;
@@ -1048,6 +1048,8 @@
        int                     size;
        void                    *buf = 0;
        int                     retval = 0;
+       struct usb_interface    *ifp = 0;
+       struct usb_driver       *driver = 0;

        /* get input parameters and alloc buffer */
        if (copy_from_user(&ctrl, (void *) arg, sizeof (ctrl)))
@@ -1065,32 +1067,55 @@
                }
        }

-       /* ioctl to device */
-       if (ctrl.ifno < 0) {
-               switch (ctrl.ioctl_code) {
-               /* access/release token for issuing control messages
-                * ask a particular driver to bind/unbind, ... etc
-                */
-               }
-               retval = -ENOSYS;
-
-       /* ioctl to the driver which has claimed a given interface */
-       } else {
-               struct usb_interface    *ifp = 0;
-               if (!ps->dev)
-                       retval = -ENODEV;
-               else if (ctrl.ifno >= ps->dev->actconfig->bNumInterfaces)
+       if (!ps->dev)
+               retval = -ENODEV;
+       else if (!(ifp = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
+               retval = -EINVAL;
+       else switch (ctrl.ioctl_code) {
+       
+       /* disconnect kernel driver from interface, leaving it unbound */
+       case USBDEVFS_DISCONNECT:
+               driver = ifp->driver;
+               if (driver) {
+                       down (&driver->serialize);
+                       dbg ("disconnect '%s' from dev %d interface %d",
+                               driver->name, ps->dev->devnum, ctrl.ifno);
+                       driver->disconnect (ps->dev, ifp->private_data);
+                       usb_driver_release_interface (driver, ifp);
+                       up (&driver->serialize);
+               } else
                        retval = -EINVAL;
-               else {
-                       if (!(ifp = usb_ifnum_to_if (ps->dev, ctrl.ifno)))
-                               retval = -EINVAL;
-                       else if (ifp->driver == 0 || ifp->driver->ioctl == 0)
-                               retval = -ENOSYS;
-               }
-               if (retval == 0)
+               break;
+               
+       /* let kernel drivers try to (re)bind to the interface */
+       case USBDEVFS_CONNECT:
+               usb_find_interface_driver_for_ifnum (ps->dev, ctrl.ifno);
+               break;
+               
+       /* talk directly to the interface's driver */
+       default:
+               lock_kernel(); /* against module unload */
+               driver = ifp->driver;
+               if (driver == 0 || driver->ioctl == 0) {
+                       unlock_kernel();
+                       retval = -ENOSYS;
+               } else {
+                       if (ifp->driver->owner) {
+                               __MOD_INC_USE_COUNT(ifp->driver->owner);
+                               unlock_kernel();
+                       }
                        /* ifno might usefully be passed ... */
-                       retval = ifp->driver->ioctl (ps->dev, ctrl.ioctl_code, buf);
+                       retval = driver->ioctl (ps->dev,
ctrl.ioctl_code, buf);
                        /* size = min_t(int, size, retval)? */
+                       if (ifp->driver->owner) {
+                               __MOD_DEC_USE_COUNT(ifp->driver->owner);
+                       } else {
+                               unlock_kernel();
+                       }
+               }
+               
+               if (retval == -ENOIOCTLCMD)
+                       retval = -ENOTTY;
        }

        /* cleanup and return */
diff -urN vanilla/drivers/usb/usb.c patched/drivers/usb/usb.c
--- vanilla/drivers/usb/usb.c   Sun Jun  2 17:37:57 2002
+++ patched/drivers/usb/usb.c   Mon Jun  3 11:50:52 2002
@@ -162,6 +162,26 @@
        }
   }

+/*
+ * usb_ifnum_to_ifpos - convert the interface _number_ (as in
interface.bInterfaceNumber)
+ * to the interface _position_ (as in dev->actconfig->interface +
position)
+ * @dev: the device to use
+ * @ifnum: the interface number (bInterfaceNumber); not interface
position
+ *
+ * Note that the number is the same as the position for all
interfaces _except_
+ * devices with interfaces not sequentially numbered (e.g., 0, 2, 3,
etc).
+ */
+int usb_ifnum_to_ifpos(struct usb_device *dev, unsigned ifnum)
+{
+       int i;
+
+       for (i = 0; i < dev->actconfig->bNumInterfaces; i++)
+               if (dev->actconfig->interface[i].altsetting[0].bInterfaceNumber ==
ifnum)
+                       return i;
+
+       return -EINVAL;
+}
+
   /**
    *   usb_deregister - unregister a USB driver
    *   @driver: USB operations of the driver to unregister
@@ -536,7 +556,6 @@
        iface->private_data = NULL;
   }

-
   /**
    * usb_match_id - find first usb_device_id matching device or interface
    * @dev: the device whose descriptors are considered when matching
@@ -749,6 +768,23 @@
        return -1;
   }

+/*
+ * usb_find_interface_driver_for_ifnum - convert ifnum to ifpos via
+ * usb_ifnum_to_ifpos and call usb_find_interface_driver().
+ * @dev: the device to use
+ * @ifnum: the interface number (bInterfaceNumber); not interface
position!
+ *
+ * Note usb_find_interface_driver's ifnum parameter is actually
interface position.
+ */
+int usb_find_interface_driver_for_ifnum(struct usb_device *dev,
unsigned int ifnum)
+{
+       int ifpos = usb_ifnum_to_ifpos(dev, ifnum);
+
+       if (0 > ifpos)
+               return -EINVAL;
+
+       return usb_find_interface_driver(dev, ifpos);
+}

   #ifdef       CONFIG_HOTPLUG

@@ -2367,6 +2403,7 @@
    * into the kernel, and other device drivers are built as modules,
    * then these symbols need to be exported for the modules to use.
    */
+EXPORT_SYMBOL(usb_ifnum_to_ifpos);
   EXPORT_SYMBOL(usb_ifnum_to_if);
   EXPORT_SYMBOL(usb_epnum_to_ep_desc);

@@ -2381,6 +2418,7 @@
   EXPORT_SYMBOL(usb_free_dev);
   EXPORT_SYMBOL(usb_inc_dev_use);

+EXPORT_SYMBOL(usb_find_interface_driver_for_ifnum);
   EXPORT_SYMBOL(usb_driver_claim_interface);
   EXPORT_SYMBOL(usb_interface_claimed);
   EXPORT_SYMBOL(usb_driver_release_interface);
diff -urN vanilla/include/linux/usb.h patched/include/linux/usb.h
--- vanilla/include/linux/usb.h Sun Jun  2 17:46:38 2002
+++ patched/include/linux/usb.h Fri Aug 30 15:21:46 2002
@@ -386,6 +386,7 @@
   };

   struct usb_driver {
+       struct module *owner;
        const char *name;

        void *(*probe)(
@@ -808,6 +809,7 @@
        struct usb_device *children[USB_MAXCHILDREN];
   };

+extern int usb_ifnum_to_ifpos(struct usb_device *dev, unsigned ifnum);
   extern struct usb_interface *usb_ifnum_to_if(struct usb_device *dev,
unsigned ifnum);
   extern struct usb_endpoint_descriptor *usb_epnum_to_ep_desc(struct
usb_device *dev, unsigned epnum);

@@ -816,6 +818,7 @@
   extern void usb_scan_devices(void);

   /* used these for multi-interface device registration */
+extern int usb_find_interface_driver_for_ifnum(struct usb_device
*dev, unsigned ifnum);
   extern void usb_driver_claim_interface(struct usb_driver *driver,
struct usb_interface *iface, void* priv);
   extern int usb_interface_claimed(struct usb_interface *iface);
   extern void usb_driver_release_interface(struct usb_driver *driver,
struct usb_interface *iface);
diff -urN vanilla/include/linux/usbdevice_fs.h
patched/include/linux/usbdevice_fs.h
--- vanilla/include/linux/usbdevice_fs.h        Sun Jun  2 17:47:02 2002
+++ patched/include/linux/usbdevice_fs.h        Sun Jun  2 17:47:59 2002
@@ -142,6 +142,8 @@
   #define USBDEVFS_HUB_PORTINFO      _IOR('U', 19, struct
usbdevfs_hub_portinfo)
   #define USBDEVFS_RESET             _IO('U', 20)
   #define USBDEVFS_CLEAR_HALT        _IOR('U', 21, unsigned int)
+#define USBDEVFS_DISCONNECT        _IO('U', 22)
+#define USBDEVFS_CONNECT           _IO('U', 23)

   /*
--------------------------------------------------------------------- */

************** end of original patch ****************

So, David replied:

David Brownell wrote:
 > Glad to see a version of this available.
 >
 >
 >> +EXPORT_SYMBOL(usb_ifnum_to_ifpos);
 >> +EXPORT_SYMBOL(usb_find_interface_driver_for_ifnum);
 >
 >
 > Do we need to do this?  Those functions are gone in 2.5,
 > and today only usbfs uses them in 2.4 ... I don't see a
 > reason to export them or include them in <linux/usb.h>.
 >
 > - Dave
 >
 >
 >
 >

and Greg answered:


  > Good point, these functions can just go into the devio.c file.  Also,
  > the owner * is already present in usb.h, but that's my fault, not
  > Kevin's.

  > thanks,

  > greg k-h

   ***************************************

So those points should be taken care of. Now the problem with the
locking code:


*******************************************

Greg KH:

So, comments anyone?  Is the locking here all done properly?

Dave:

No, it's got a problem that's also found in the 2.5.39 code
(as I happened to notice).  See excerpts below ... all paths
through the code should agree on this!

I suspect there should just be a lock guarding access to the
driver bound to that interface (semaphore may be appropriate),
where the enumeration code that modifies those bindings is
the only thing that needs to know about a policy of using BKL.

- Dave


+    /* disconnect kernel driver from interface, leaving it unbound */
+    case USBDEVFS_DISCONNECT:
+        driver = ifp->driver;
+        if (driver) {
+            down (&driver->serialize);

... does NOT use BKL to protect against module unload ...

+       /* talk directly to the interface's driver */
+       default:
+        lock_kernel(); /* against module unload */
+               driver = ifp->driver;

... DOES use BKL to protect against unload (or anyone enumerating
this interface)



****************************************

I'm afraid that the details of this are beyound me. Could someone make
some suggestions or point me to an example of the correct code? Thanks,

        Kevin




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to