[PATCH 1/2] hid2hci: iterate libusb devices twice

[PATCH 1/2] hid2hci: iterate libusb devices twice

[Mario Limonciello]

[-- Attachment #1.1: Type: text/plain, Size: 625 bytes --]

Hi:

After the previous cleanup submitted to hid2hci, a race condition has
been exposed with regard to libusb.  Even though we know the bus and
devnum are existing, the call to usb_find_devices isn't necessarily
finding it on the first time after the device is introduced to the bus
by the BT killswitch.  I'm not too keen upon iterating something twice
as a solution, it does solve the problem until a method is introduced in
usb_device_open_from_udev that doesn't require libusb's iteration of the
bus in the first place.

Regards
-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: iterate.diff --]
[-- Type: text/x-patch; name="iterate.diff", Size: 1070 bytes --]

Even though we know that the device is on the proper bus, libusb
doesn't always find it on the first try.  Once we find the right
bus, if we iterate it twice, we tend to find the device.
=== modified file 'extras/hid2hci/hid2hci.c'
--- extras/hid2hci/hid2hci.c	2009-07-24 16:06:22 +0000
+++ extras/hid2hci/hid2hci.c	2009-07-27 20:45:34 +0000
@@ -161,6 +161,7 @@
 	const char *str;
 	int busnum;
 	int devnum;
+	int trynum;
 
 	str = udev_device_get_sysattr_value(usb_dev, "busnum");
 	if (str == NULL)
@@ -174,7 +175,6 @@
 
 	usb_init();
 	usb_find_busses();
-	usb_find_devices();
 
 	for (bus = usb_get_busses(); bus; bus = bus->next) {
 		struct usb_device *dev;
@@ -182,9 +182,11 @@
 		if (strtol(bus->dirname, NULL, 10) != busnum)
 			continue;
 
-		for (dev = bus->devices; dev; dev = dev->next) {
-			if (dev->devnum == devnum)
-				return dev;
+		for (trynum = 0; trynum < 2; trynum++) {
+			usb_find_devices();
+			for (dev = bus->devices; dev; dev = dev->next)
+				if (dev->devnum == devnum)
+					return dev;
 		}
 	}
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Marcel Holtmann]

Hi Mario,

> After the previous cleanup submitted to hid2hci, a race condition has
> been exposed with regard to libusb.  Even though we know the bus and
> devnum are existing, the call to usb_find_devices isn't necessarily
> finding it on the first time after the device is introduced to the bus
> by the BT killswitch.  I'm not too keen upon iterating something twice
> as a solution, it does solve the problem until a method is introduced in
> usb_device_open_from_udev that doesn't require libusb's iteration of the
> bus in the first place.

then the usage of libusb needs to be removed. Read Kay's statement about
libusb being utterly stupid.

Regards

Marcel

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Kay Sievers]

On Mon, Jul 27, 2009 at 23:32, Marcel Holtmann<marcel@holtmann.org> wrote:
>> After the previous cleanup submitted to hid2hci, a race condition has
>> been exposed with regard to libusb.  Even though we know the bus and
>> devnum are existing, the call to usb_find_devices isn't necessarily
>> finding it on the first time after the device is introduced to the bus
>> by the BT killswitch.  I'm not too keen upon iterating something twice
>> as a solution, it does solve the problem until a method is introduced in
>> usb_device_open_from_udev that doesn't require libusb's iteration of the
>> bus in the first place.

Now it's getting funny. We need to call the nonsense two times to make
it work? We need to fix the real issue here instead of doing guesswork
and adding hacks like this. Any idea what's going on with the first
scan? The device node is guaranteed to exist when we call stuff from
RUN+= instructions.

> then the usage of libusb needs to be removed. Read Kay's statement about
> libusb being utterly stupid.

Well, libusb might be ok for stuff that just searches things which are
always there, it's definitely not suitable to be used in conjunction
with udev. The entire coldplug with 500 devices takes ~0.5 seconds
here, while a single call to libusb takes ~0.15 just to find the
device we already have. That alone is not acceptable, and now we
should call the useless scan twice? Tsss ... :)

Thanks,
Kay

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]

Hi Kay:

Kay Sievers wrote:
> On Mon, Jul 27, 2009 at 23:32, Marcel Holtmann<marcel@holtmann.org> wrote:
>   
>
> Now it's getting funny. We need to call the nonsense two times to make
> it work? We need to fix the real issue here instead of doing guesswork
> and adding hacks like this. Any idea what's going on with the first
> scan? The device node is guaranteed to exist when we call stuff from
> RUN+= instructions.
>
>   
I'm not sure what's going on here, but I think i'll just go down that
road of pulling the necessary code out of libusb's find_devices to just
craft a usb_device object with the information we already have so there
is no necessary scanning in the first place.  There will still  be a
dependency on libusb to be able to send a usb_control_msg, claim, etc,
but at least the time consuming, unnecessary scan will be gone.
>
> Well, libusb might be ok for stuff that just searches things which are
> always there, it's definitely not suitable to be used in conjunction
> with udev. The entire coldplug with 500 devices takes ~0.5 seconds
> here, while a single call to libusb takes ~0.15 just to find the
> device we already have. That alone is not acceptable, and now we
> should call the useless scan twice? Tsss ... :)
>   
Yeah I understand.  I'll follow up after I get something else together.

-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

Hi Kay:

Kay Sievers wrote:
> On Mon, Jul 27, 2009 at 23:32, Marcel Holtmann<marcel@holtmann.org> wrote:
>   
>
> Now it's getting funny. We need to call the nonsense two times to make
> it work? We need to fix the real issue here instead of doing guesswork
> and adding hacks like this. Any idea what's going on with the first
> scan? The device node is guaranteed to exist when we call stuff from
> RUN+= instructions.
>
>   
Are you absolutely positive that the device node will exist and be
usable by the time something is called from RUN?  I've been
enabling/adding debugging code around libusb, and finally got down to
the point that when it tries to do an open( ) on the device node, here's
what's happening:

    Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
    (stderr) 'USB error: failed to open /dev/bus/usb/003/061: No such
    file or directory'


This of course causes failures later because of a bad file descriptor:

    Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
    (stderr) 'USB error: could not detach kernel driver from interface
    0: Bad file descriptor'
    Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
    (stderr) 'USB error: could not claim interface 0: Bad file descriptor'


-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

RE: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Mario_Limonciello]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

Hi Kay:

Kay Sievers wrote:
>> Now it's getting funny. We need to call the nonsense two times to make 
>> it work? We need to fix the real issue here instead of doing guesswork 
>> and adding hacks like this. Any idea what's going on with the first 
>> scan? The device node is guaranteed to exist when we call stuff from
>> RUN+= instructions.
>>
>>   
> Are you absolutely positive that the device node will exist and be usable by the time something is called from RUN?  I've been enabling/adding debugging code around libusb, and finally got down to the point that when it tries to do > an open( ) on the device node, here's what's happening:
> 
>     Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
>     (stderr) 'USB error: failed to open /dev/bus/usb/003/061: No such
>     file or directory'
> 
> 
> This of course causes failures later because of a bad file descriptor:
> 
>     Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
>     (stderr) 'USB error: could not detach kernel driver from interface
>     0: Bad file descriptor'
>     Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
>     (stderr) 'USB error: could not claim interface 0: Bad file descriptor'

I'm attaching a diff to follow up to this that removes the 'braindead' libusb open() on every device, and instead only opens the device that is needed.

There is still a problem here with udev that the device node is not able to be opened necessarily at the time hid2hci is called, so I've got a workaround in place that I verified to sleep and try two more times before bailing out.

--
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: iterate.diff --]
[-- Type: application/octet-stream, Size: 3204 bytes --]

=== modified file 'extras/hid2hci/hid2hci.c'
--- extras/hid2hci/hid2hci.c	2009-07-24 16:06:22 +0000
+++ extras/hid2hci/hid2hci.c	2009-07-29 17:52:55 +0000
@@ -34,6 +34,21 @@
 #include "libudev.h"
 #include "libudev-private.h"
 
+struct usb_dev_handle {
+  int fd;
+
+  struct usb_bus *bus;
+  struct usb_device *device;
+
+  int config;
+  int interface;
+  int altsetting;
+
+  /* Added by RMT so implementations can store other per-open-device data */
+  void *impl_info;
+};
+
+
 enum mode {
 	HCI = 0,
 	HID = 1,
@@ -150,44 +165,46 @@
 	return err;
 }
 
-/*
- * The braindead libusb needs to scan and open all devices, just to
- * to find the device we already have. This needs to be fixed in libusb
- * or it will be ripped out and we carry our own code.
- */
 static struct usb_device *usb_device_open_from_udev(struct udev_device *usb_dev)
 {
 	struct usb_bus *bus;
+	struct usb_device *dev;
 	const char *str;
-	int busnum;
-	int devnum;
+	char num[4];
+
+	usb_init();
+
+	bus = malloc(sizeof(*bus));
+	dev = malloc(sizeof(*dev));
+
+	if (!bus || !dev)
+		return NULL;
+
+	memset((void *)bus, 0, sizeof(*bus));
+	memset((void *)dev, 0, sizeof(*dev));
 
 	str = udev_device_get_sysattr_value(usb_dev, "busnum");
 	if (str == NULL)
-		return NULL;
-	busnum = strtol(str, NULL, 0);
+		goto err;
+	snprintf(num, sizeof(num), "%03i",(int)strtol(str,NULL,0));
+	strncpy(bus->dirname, num, sizeof(bus->dirname) - 1);
+	bus->dirname[sizeof(bus->dirname) - 1] = 0;
+	dev->bus = bus;
 
 	str = udev_device_get_sysattr_value(usb_dev, "devnum");
 	if (str == NULL)
-		return NULL;
-	devnum = strtol(str, NULL, 0);
-
-	usb_init();
-	usb_find_busses();
-	usb_find_devices();
-
-	for (bus = usb_get_busses(); bus; bus = bus->next) {
-		struct usb_device *dev;
-
-		if (strtol(bus->dirname, NULL, 10) != busnum)
-			continue;
-
-		for (dev = bus->devices; dev; dev = dev->next) {
-			if (dev->devnum == devnum)
-				return dev;
-		}
-	}
-
+		goto err;
+	dev->devnum = strtol(str, NULL, 0);
+
+	snprintf(num, sizeof(num), "%03i",dev->devnum);
+	strncpy(dev->filename, num, sizeof(dev->filename) - 1);
+	dev->filename[sizeof(dev->filename) - 1] = 0;
+
+	return dev;
+
+err:
+	free(dev);
+	free(bus);
 	return NULL;
 }
 
@@ -200,12 +217,24 @@
 	struct udev_enumerate *enumerate;
 	struct udev_list_entry *entry;
 	struct usb_dev_handle *handle = NULL;
+	int try;
 
 	if (sibling_intf == NULL) {
 		dev = usb_device_open_from_udev(udev_dev);
 		if (dev == NULL)
 			return NULL;
-		return usb_open(dev);
+		/* Workaround a race condition where /dev/bus/BUSNUM/DEVNUM has a 
+		 * tendency to not be ready for an open() yet */
+		for (try = 0; try < 3; try++) {
+			handle = usb_open(dev);
+			if (handle && handle->fd >= 0)
+				break;
+			handle = NULL;
+			sleep(1);
+		}
+		free(dev->bus);
+		free(dev);
+		return handle;
 	}
 
 	/* find matching sibling of the current usb_device, they share the same hub */
@@ -247,8 +276,11 @@
 				continue;
 			/* only look at the first matching device */
 			dev = usb_device_open_from_udev(udev_parent);
-			if (dev != NULL)
+			if (dev != NULL) {
 				handle = usb_open(dev);
+				free(dev->bus);
+				free(dev);
+			}
 			udev_device_unref(udev_device);
 			break;
 		}

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Kay Sievers]

On Wed, Jul 29, 2009 at 12:30, Mario
Limonciello<mario_limonciello@dell.com> wrote:
>> On Mon, Jul 27, 2009 at 23:32, Marcel Holtmann<marcel@holtmann.org> wrote:

>> Now it's getting funny. We need to call the nonsense two times to make
>> it work? We need to fix the real issue here instead of doing guesswork
>> and adding hacks like this. Any idea what's going on with the first
>> scan? The device node is guaranteed to exist when we call stuff from
>> RUN+= instructions.
>>
> Are you absolutely positive that the device node will exist and be
> usable by the time something is called from RUN?  I've been
> enabling/adding debugging code around libusb, and finally got down to
> the point that when it tries to do an open( ) on the device node, here's
> what's happening:
>
>    Jul 29 11:03:25 test-laptop udevd-work[20561]: '/lib/udev/hid2hci'
>    (stderr) 'USB error: failed to open /dev/bus/usb/003/061: No such
>    file or directory'

Yeah, I'm sure. The thing is that the kernel USB code for some weird
reason returns -ENOENT instead -ENODEV -- hence the misleading error
string.

Alan, I suspect (haven't checked) that at the time we get the event
for the usb_device, the device is not ready to be handled by usbfs
operations. Is that likely? And if, any idea how, and until which
point, to delay the usb_device uevent, in which we hook in to do the
usbfs actions from userspace?

Thanks,
Kay

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Alan Stern]

On Wed, 29 Jul 2009, Kay Sievers wrote:

> Yeah, I'm sure. The thing is that the kernel USB code for some weird
> reason returns -ENOENT instead -ENODEV -- hence the misleading error
> string.

That return code can easily be changed.

> Alan, I suspect (haven't checked) that at the time we get the event
> for the usb_device, the device is not ready to be handled by usbfs
> operations. Is that likely? And if, any idea how, and until which
> point, to delay the usb_device uevent, in which we hook in to do the
> usbfs actions from userspace?

There are two aspects we have to consider: usbfs device nodes
(/dev/bus/usb/*/*) and usbfs file nodes (/proc/bus/usb/*/*).  The 
device nodes are created by udev whereas the file nodes are created by 
the kernel (if CONFIG_USB_DEVICEFS is enabled).  Nowadays most people 
use the device nodes, but embedded systems without udev may want to 
rely on the file nodes.


With the device nodes, the problem is this: usbdev_open() looks up
dev_t values by calling bus_find_device().  But devices are added to
the bus's list by bus_attach_device(), which isn't called until after
the KOBJ_ADD uevent is sent.  Hence there's a window after the uevent
is issued and before the device can be looked up on the bus.  That's
probably what's happening here.

Interchanging bus_attach_device() and kobject_uevent() in device_add()  
would solve this problem, but it would introduce other problems.  In
particular, if a probe routine registered a child device then the 
child's uevent would be sent before the parent's!

You know, the real problem may lie in bus_attach_device().  If probing
fails then it doesn't add the device to the bus -- and it doesn't even
return an error code when this happens.  One can argue that the device
should _always_ be added to the bus, regardless of whether a suitable
driver can be found.  If you accept that argument then linking the
device into the bus's list should be moved up into bus_add_device(),
which is called before the uevent.  Indeed, each of those two routines
has a comment in the kerneldoc saying "Add the device to its bus's list
of devices."  They can't both be right!


There's also a problem with usbfs file nodes.  The file nodes are 
created by the device's probe routine.  So again, if the uevent comes 
before probing then it will come before the file nodes exist.  I 
suppose we could create the file nodes earlier, before the device is 
registered, but is this really a good idea?

Alan Stern

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Kay Sievers]

On Thu, Jul 30, 2009 at 10:37, Alan Stern<stern@rowland.harvard.edu> wrote:
> On Wed, 29 Jul 2009, Kay Sievers wrote:
>
>> Yeah, I'm sure. The thing is that the kernel USB code for some weird
>> reason returns -ENOENT instead -ENODEV -- hence the misleading error
>> string.
>
> That return code can easily be changed.

If we don't confuse anything, I guess that would be better, as it's
really about a device not being there, not a file.

>> Alan, I suspect (haven't checked) that at the time we get the event
>> for the usb_device, the device is not ready to be handled by usbfs
>> operations. Is that likely? And if, any idea how, and until which
>> point, to delay the usb_device uevent, in which we hook in to do the
>> usbfs actions from userspace?
>
> There are two aspects we have to consider: usbfs device nodes
> (/dev/bus/usb/*/*) and usbfs file nodes (/proc/bus/usb/*/*).  The
> device nodes are created by udev whereas the file nodes are created by
> the kernel (if CONFIG_USB_DEVICEFS is enabled).  Nowadays most people
> use the device nodes, but embedded systems without udev may want to
> rely on the file nodes.
>
> With the device nodes, the problem is this: usbdev_open() looks up
> dev_t values by calling bus_find_device().  But devices are added to
> the bus's list by bus_attach_device(), which isn't called until after
> the KOBJ_ADD uevent is sent.  Hence there's a window after the uevent
> is issued and before the device can be looked up on the bus.  That's
> probably what's happening here.

Yeah, that sounds likely.

> Interchanging bus_attach_device() and kobject_uevent() in device_add()
> would solve this problem, but it would introduce other problems.  In
> particular, if a probe routine registered a child device then the
> child's uevent would be sent before the parent's!

Right, we can not do that.

> You know, the real problem may lie in bus_attach_device().  If probing
> fails then it doesn't add the device to the bus -- and it doesn't even
> return an error code when this happens.  One can argue that the device
> should _always_ be added to the bus, regardless of whether a suitable
> driver can be found.  If you accept that argument then linking the
> device into the bus's list should be moved up into bus_add_device(),
> which is called before the uevent.  Indeed, each of those two routines
> has a comment in the kerneldoc saying "Add the device to its bus's list
> of devices."  They can't both be right!

That sounds like the right thing. We create all the links which
indicate a membership at the bus, so we should put it to the bus at
the same time.

> There's also a problem with usbfs file nodes.  The file nodes are
> created by the device's probe routine.  So again, if the uevent comes
> before probing then it will come before the file nodes exist.  I
> suppose we could create the file nodes earlier, before the device is
> registered, but is this really a good idea?

Oh, I don't think we really need to care about the proc files and the
uevent timing. Users of the proc files usually don't hook into the
events but poll the devices file. The event timing and the proc files
have been broken since the first day, and you always needed to loop
until the proc file showed up.

Also, we expect to get a devtmpfs which will create the /dev/bus/usb/
nodes from inside the kernel, so the proc things can be ignored for
almost all use cases after that.

Thanks,
Kay

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Kay Sievers]

On Thu, Jul 30, 2009 at 11:27, Kay Sievers<kay.sievers@vrfy.org> wrote:
> On Thu, Jul 30, 2009 at 10:37, Alan Stern<stern@rowland.harvard.edu> wrote:

>> You know, the real problem may lie in bus_attach_device().  If probing
>> fails then it doesn't add the device to the bus -- and it doesn't even
>> return an error code when this happens.  One can argue that the device
>> should _always_ be added to the bus, regardless of whether a suitable
>> driver can be found.  If you accept that argument then linking the
>> device into the bus's list should be moved up into bus_add_device(),
>> which is called before the uevent.  Indeed, each of those two routines
>> has a comment in the kerneldoc saying "Add the device to its bus's list
>> of devices."  They can't both be right!
>
> That sounds like the right thing. We create all the links which
> indicate a membership at the bus, so we should put it to the bus at
> the same time.

It boots and runs fine here to move the klist_add_tail() up into
bus_add_device(). I guess we should do that.

Kay

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Alan Stern]

On Thu, 30 Jul 2009, Kay Sievers wrote:

> On Thu, Jul 30, 2009 at 10:37, Alan Stern<stern@rowland.harvard.edu> wrote:
> > On Wed, 29 Jul 2009, Kay Sievers wrote:
> >
> >> Yeah, I'm sure. The thing is that the kernel USB code for some weird
> >> reason returns -ENOENT instead -ENODEV -- hence the misleading error
> >> string.
> >
> > That return code can easily be changed.
> 
> If we don't confuse anything, I guess that would be better, as it's
> really about a device not being there, not a file.

Okay.  I included that change in the patch below for testing.  
Ultimately I will submit it separately.

> > You know, the real problem may lie in bus_attach_device().  If probing
> > fails then it doesn't add the device to the bus -- and it doesn't even
> > return an error code when this happens.  One can argue that the device
> > should _always_ be added to the bus, regardless of whether a suitable
> > driver can be found.  If you accept that argument then linking the
> > device into the bus's list should be moved up into bus_add_device(),
> > which is called before the uevent.  Indeed, each of those two routines
> > has a comment in the kerneldoc saying "Add the device to its bus's list
> > of devices."  They can't both be right!
> 
> That sounds like the right thing. We create all the links which
> indicate a membership at the bus, so we should put it to the bus at
> the same time.

Try the patch below.  It should fix the problem.

> > There's also a problem with usbfs file nodes.  The file nodes are
> > created by the device's probe routine.  So again, if the uevent comes
> > before probing then it will come before the file nodes exist.  I
> > suppose we could create the file nodes earlier, before the device is
> > registered, but is this really a good idea?
> 
> Oh, I don't think we really need to care about the proc files and the
> uevent timing. Users of the proc files usually don't hook into the
> events but poll the devices file. The event timing and the proc files
> have been broken since the first day, and you always needed to loop
> until the proc file showed up.
> 
> Also, we expect to get a devtmpfs which will create the /dev/bus/usb/
> nodes from inside the kernel, so the proc things can be ignored for
> almost all use cases after that.

Okay, I won't worry about the file nodes then.

Alan Stern



Index: usb-2.6/drivers/base/base.h
=================================--- usb-2.6.orig/drivers/base/base.h
+++ usb-2.6/drivers/base/base.h
@@ -109,7 +109,7 @@ extern int system_bus_init(void);
 extern int cpu_dev_init(void);
 
 extern int bus_add_device(struct device *dev);
-extern void bus_attach_device(struct device *dev);
+extern void bus_probe_device(struct device *dev);
 extern void bus_remove_device(struct device *dev);
 
 extern int bus_add_driver(struct device_driver *drv);
Index: usb-2.6/drivers/base/bus.c
=================================--- usb-2.6.orig/drivers/base/bus.c
+++ usb-2.6/drivers/base/bus.c
@@ -459,8 +459,9 @@ static inline void remove_deprecated_bus
  * bus_add_device - add device to bus
  * @dev: device being added
  *
+ * - Add device's bus attributes.
+ * - Create links to device's bus.
  * - Add the device to its bus's list of devices.
- * - Create link to device's bus.
  */
 int bus_add_device(struct device *dev)
 {
@@ -483,6 +484,7 @@ int bus_add_device(struct device *dev)
 		error = make_deprecated_bus_links(dev);
 		if (error)
 			goto out_deprecated;
+		klist_add_tail(&dev->p->knode_bus, &bus->p->klist_devices);
 	}
 	return 0;
 
@@ -498,24 +500,19 @@ out_put:
 }
 
 /**
- * bus_attach_device - add device to bus
- * @dev: device tried to attach to a driver
+ * bus_probe_device - probe drivers for a new device
+ * @dev: device to probe
  *
- * - Add device to bus's list of devices.
- * - Try to attach to driver.
+ * - Automatically probe for a driver if the bus allows it.
  */
-void bus_attach_device(struct device *dev)
+void bus_probe_device(struct device *dev)
 {
 	struct bus_type *bus = dev->bus;
-	int ret = 0;
+	int ret;
 
-	if (bus) {
-		if (bus->p->drivers_autoprobe)
-			ret = device_attach(dev);
+	if (bus && bus->p->drivers_autoprobe) {
+		ret = device_attach(dev);
 		WARN_ON(ret < 0);
-		if (ret >= 0)
-			klist_add_tail(&dev->p->knode_bus,
-				       &bus->p->klist_devices);
 	}
 }
 
Index: usb-2.6/drivers/base/core.c
=================================--- usb-2.6.orig/drivers/base/core.c
+++ usb-2.6/drivers/base/core.c
@@ -955,7 +955,7 @@ int device_add(struct device *dev)
 					     BUS_NOTIFY_ADD_DEVICE, dev);
 
 	kobject_uevent(&dev->kobj, KOBJ_ADD);
-	bus_attach_device(dev);
+	bus_probe_device(dev);
 	if (parent)
 		klist_add_tail(&dev->p->knode_parent,
 			       &parent->p->klist_children);
Index: usb-2.6/drivers/usb/core/devio.c
=================================--- usb-2.6.orig/drivers/usb/core/devio.c
+++ usb-2.6/drivers/usb/core/devio.c
@@ -619,7 +619,7 @@ static int usbdev_open(struct inode *ino
 	if (!ps)
 		goto out;
 
-	ret = -ENOENT;
+	ret = -ENODEV;
 
 	/* usbdev device-node */
 	if (imajor(inode) = USB_DEVICE_MAJOR)

Re: [PATCH 1/2] hid2hci: iterate libusb devices twice

[Mario Limonciello]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

Hi Alan:

Alan Stern wrote:
> On Thu, 30 Jul 2009, Kay Sievers wrote:
>   
> Try the patch below.  It should fix the problem.
>   
I can confirm this reliably fixes the problem with hacks that were added
to hid2hci removed.

Thanks!
-- 
Mario Limonciello
*Dell | Linux Engineering*
mario_limonciello@dell.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]