On Thu, 30 Jul 2009, Kay Sievers wrote:
> On Thu, Jul 30, 2009 at 10:37, Alan Stern<stern@xxxxxxxxxxxxxxxxxxx> 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)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
