RFC: Code to snatch a device from a generic driver

RFC: Code to snatch a device from a generic driver

[Ron]

Hi,

We're presently working on enabling the cpad[1] and wacom kernel
modules to retrieve their particular devices from a more generic
driver that may have already claimed them, without resorting to
patching other drivers as we see with the quirks in usbhid.
In 2.6 we can no longer even pull the 'add below hid' stunt
that got us by from userspace in 2.4 [2] -- however the new driver
core model as I understand it seems like it should be able to
handle this very nicely from within the module itself.

The following code (derived from a patch sent to me by Jan
Steinhoff) seems to do the job, but is surely not correct yet.

The cpad really is a one off device at present, so it can get
by with something like this at this stage, but the wacom driver
will need to iterate over its id table if it is to do it the
same way.  And if I haven't totally botched the locking below[3]
that means it will need to collect the locks of all the devices
it finds before it registers with they usb core -- if there is
more than one such device attached to the system.

Which seems to me like a sure recipe for occasional deadlock,
unless there is a guarantee I'm missing here somewhere.

I believe that Greg KH has a pretty good handle on this, and
possibly this is the issue he is aiming to fix still, but I
think there are surely other people who need to do this too
so I'm posting here now, because the following doesn't crash
under the testing I've given it so far, so only more eyeballs
or more testing is going to tell me what else may be wrong
with it, and tell us what still needs to be mended in the
kernel proper.

Comments from anyone with an interest in this would be
greatly appreciated.  Please cc, it's been many years since
my inbox could cope with vger.

thanks,
Ron


static int __init cpad_init(void)
{
	struct usb_device *udev = usb_find_device(USB_VENDOR_ID_SYNAPTICS,
	                                          USB_DEVICE_ID_CPAD);
	if (udev) {
		down( &udev->serialize );
		down_write( &udev->dev.bus->subsys.rwsem );

		struct usb_interface *interface = usb_ifnum_to_if(udev, 0);

		if (interface && usb_interface_claimed(interface))
		{
			info("releasing cPad from generic driver '%s'.",
			     interface->dev.driver->name);
			usb_driver_release_interface(
			       to_usb_driver(interface->dev.driver),interface);
		}

		up_write( &udev->dev.bus->subsys.rwsem );
		usb_put_dev(udev);
	}

	int result = usb_register(&cpad_driver);

	if (udev)
		up( &udev->serialize );

	if (result == 0) {
		cpad_procfs_init();
		info(DRIVER_DESC " " DRIVER_VERSION);
	}
	else
		err("usb_register failed. Error number %d", result);

	return result;
}


[1] The cPad is an lcd backed touchpad from synaptics,
    the module is not in the mainline kernel yet, we were
    too late for 2.4 and slow to get onboard 2.6 ...
    (we being the authors, none of us are associated with
    synaptics)
[2] without some nasty acrobatics
[3] which is quite possible, I'm still very green to kernel 2.6

Re: RFC: Code to snatch a device from a generic driver

[Greg KH]

On Tue, Jan 11, 2005 at 01:10:50PM +1030, Ron wrote:
> 
> Hi,
> 
> We're presently working on enabling the cpad[1] and wacom kernel
> modules to retrieve their particular devices from a more generic
> driver that may have already claimed them, without resorting to
> patching other drivers as we see with the quirks in usbhid.
> In 2.6 we can no longer even pull the 'add below hid' stunt
> that got us by from userspace in 2.4 [2] -- however the new driver
> core model as I understand it seems like it should be able to
> handle this very nicely from within the module itself.
> 
> The following code (derived from a patch sent to me by Jan
> Steinhoff) seems to do the job, but is surely not correct yet.

No, try something like the following.  It creates a sysfs file that you
can write to to unbind the device manually.

The binding a driver to a device manually is left as an exercise to the
reader :)

Seriously, I'm working on adding this to the driver core so you don't
have to do stuff like this in drivers.

thanks,

greg k-h


diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2005-01-11 09:35:48 -08:00
+++ b/drivers/base/bus.c	2005-01-11 09:35:48 -08:00
@@ -243,6 +243,17 @@
 	return ret;
 }
 
+/* manually detach a device from it's associated driver. */
+/* Any write will cause it to happen. */
+static ssize_t device_unbind(struct device *dev, const char *buf, size_t count)
+{
+	down_write(&dev->bus->subsys.rwsem);
+	device_release_driver(dev);
+	up_write(&dev->bus->subsys.rwsem);
+	return count;
+}
+static DEVICE_ATTR(unbind, S_IWUSR, NULL, device_unbind);
+
 /**
  *	device_bind_driver - bind a driver to one device.
  *	@dev:	device.
@@ -264,6 +275,7 @@
 	sysfs_create_link(&dev->driver->kobj, &dev->kobj,
 			  kobject_name(&dev->kobj));
 	sysfs_create_link(&dev->kobj, &dev->driver->kobj, "driver");
+	device_create_file(dev, &dev_attr_unbind);
 }
 
 
@@ -389,6 +401,7 @@
 	if (drv) {
 		sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
 		sysfs_remove_link(&dev->kobj, "driver");
+		device_remove_file(dev, &dev_attr_unbind);
 		list_del_init(&dev->driver_list);
 		device_detach_shutdown(dev);
 		if (drv->remove)

Re: RFC: Code to snatch a device from a generic driver

[Ron]

On Tue, Jan 11, 2005 at 11:34:56AM -0800, Greg KH wrote:
> No, try something like the following.  It creates a sysfs file that you
> can write to to unbind the device manually.

Thanks!  That's a potentially treacherous little toy to add to my
system, but it does work as advertised and it took me on a very nice
tour of appropriate internals.

> The binding a driver to a device manually is left as an exercise to the
> reader :)
> 
> Seriously, I'm working on adding this to the driver core so you don't
> have to do stuff like this in drivers.

Yes, I didn't seriously intend people should paste such goop into their
modules, right now I'm just digging my way through to the desired
functionality -- armed only with some rusty memories of 2.4, a broken
hacksaw blade, and grep ...  This should certainly be a core library
function of some form by the time we are through.

My next candidate is shown below.  Aside from the (relatively) minor
things commented there (and any that I still don't see), it seems to
have one major deficiency still, which I'll call 'warmplug' support,
(being somewhere between the increasingly ill-named hot and 'cold' plug
problems)

It will correctly snatch a device from a generic driver when the module
is loaded.  This covers 'first time users' installing the module into a
running kernel, and likewise subsequent hotplug operations when the module
is not installed at the time the device is plugged in.  But it fails
when both the generic module and the specialised module are already
loaded and the generic module was registered with the usbcore first.

In that case the generic module will ack the probe and the more
specialised module will never be queried about the device (and hence
have no opportunity to steal it).  The user must force a module reload
for the later module to get a chance to grab it.  In the case of the
wacom device, it appears to be able to lose this race from a cold boot
on a UP machine.  If the device is discovered during boot both modules
will be probed and installed but all modules fail to initialise it at
that time, since other required things are still cycling up.  Once
everyone is on the same page and the device is ready to be bound,
the modules are already loaded and so the problem above plays out.

Which I think takes us back to a more basic problem...  that I need
to register this intention to want a device more than some generic
driver does with the usb core itself -- so it can dispatch the probe
calls in a more ordered fashion from most specialised to most generic
candidates for a particular device, rather than just stopping with the
first driver that says it can handle it in some form.

I'm going to look at this more today.  All thoughts welcome.  Code below
is for vanilla 2.6.10 and still using the subsys.rwsem for now.

cheers,
Ron


/* Module side boilerplate
*/
static int __init cpad_init(void)
{
	int result = usb_register(&cpad_driver);
	if (result == 0) {
		info(DRIVER_DESC " " DRIVER_VERSION);
		repossess_devices(&cpad_driver, cpad_idtable);
	} else
		err("usb_register failed, error number: %d", result);

	return result;
}

/* Snatch all devices listed by @id from (hopefully) more generic drivers
   and bind them to @driver.  @id may be the same list passed to
   MODULE_DEVICE_TABLE, or a separate list subject to this more aggressive
   binding.
*/
static void repossess_devices(struct usb_driver *udrv,
                              struct usb_device_id *id)
{
	while (id->idVendor)
	{
		// XXX Not enough, we need to find ALL devices, not just the
		//     first of any particular model.  Messy to try and fix
		//     here in the module though, so don't yet.

		struct usb_device *udev = usb_find_device(id->idVendor,
							  id->idProduct);

		if (udev)
		{
			down_write(&udev->dev.bus->subsys.rwsem);
			usb_lock_device(udev);

			// see above
			struct usb_interface *interface = usb_ifnum_to_if(udev,0);

			if (interface->dev.driver &&
			    interface->dev.driver->owner != udrv->driver.owner)
			{
			    if (interface && usb_interface_claimed(interface))
				    device_release_driver(&interface->dev);

			    driver_probe_device(&udrv->driver, &interface->dev);
			}

			usb_unlock_device(udev);
			up_write(&udev->dev.bus->subsys.rwsem);

			usb_put_dev(udev);
		}
		++id;
	}
}