usb sysfs intf files no longer created when probe fails

usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Hi,

With the latest git tree, the USB core no longer creates sysfs entries
for interfaces which haven't been probed by a driver. It used to work
until yesterday. I am talking about these:

defiant:/sys/devices/pci0001:10/0001:10:1a.0/usb1/1-2 125 > ls -al
total 0
drwxr-xr-x  5 root root    0 2005-06-22 15:32 .
drwxr-xr-x  6 root root    0 2005-06-22 15:31 ..
drwxr-xr-x  3 root root    0 2005-06-22 14:14 1-2:1.0
drwxr-xr-x  3 root root    0 2005-06-22 14:14 1-2:1.2
-r--r--r--  1 root root 4096 2005-06-22 15:32 bcdDevice
-rw-r--r--  1 root root 4096 2005-06-22 15:32 bConfigurationValue
[...]

Notice the '1-2:1.1' is missing. Upon booting I get:

Jun 22 13:34:04 localhost kernel: HID device not claimed by input or
hiddev
Jun 22 13:34:04 localhost kernel: usbhid: probe of 1-2:1.1 failed with
error -5
Jun 22 13:34:04 localhost kernel: usb 1-2: device_add(1-2:1.1) --> -5

The two first lines are normal (the device claims to be a HID one but it
is a proprietary one). The third line is new, it is not present when
booting an older kernel.

In case it matters, this is on an Apple Powerbook Alu (post Feb 2005
modem), and /proc/bus/usb/devices says:
T:  Bus=01 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#=  6 Spd=12  MxCh= 0
D:  Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS= 8 #Cfgs=  1
P:  Vendor=05ac ProdID=020f Rev= 0.28
S:  Manufacturer=Apple Computer
S:  Product=Apple Internal Keyboard/Trackpad
C:* #Ifs= 3 Cfg#= 1 Atr=a0 MxPwr= 40mA
I:  If#= 0 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=01 Driver=usbhid
E:  Ad=83(I) Atr=03(Int.) MxPS=   8 Ivl=10ms
I:  If#= 1 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=01 Prot=02 Driver=(none)
E:  Ad=81(I) Atr=03(Int.) MxPS=  32 Ivl=1ms
I:  If#= 2 Alt= 0 #EPs= 1 Cls=03(HID  ) Sub=00 Prot=00 Driver=usbhid
E:  Ad=84(I) Atr=03(Int.) MxPS=   1 Ivl=10ms

I use the 'atp' input driver from http://popies.net/atp/ to drive this
touchpad. When removing the driver I also get an oops, possibly related
to the previous failure to create the sysfs file:
usbcore: deregistering driver atp
Oops: kernel access of bad area, sig: 11 [#1]
NIP: C009F5F8 LR: C00A1728 SP: C3AD9DE0 REGS: c3ad9d30 TRAP: 0300    Not
tainted
MSR: 00009032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
DAR: 0000000C, DSISR: 40000000
TASK = ccc94150[7148] 'rmmod' THREAD: c3ad8000
Last syscall: 129
GPR00: 00000000 C3AD9DE0 CCC94150 C6ED3D48 C02C889C DE6273C7 00000000
00000000
GPR08: 00000001 C6ED3CD4 DFFF9410 00000000 64A79ADA 1001A18C 100C0000
100A0000
GPR16: 00000000 100FEF88 24222482 100C0000 100F2CE8 100F2BE8 10001430
00000000
GPR24: 10000000 00000000 00000000 C02C889C DE76819C DE984678 00000000
E1074280
NIP [c009f5f8] sysfs_hash_and_remove+0x3c/0x170
LR [c00a1728] sysfs_remove_link+0x14/0x24
Call trace:
[c00a1728] sysfs_remove_link+0x14/0x24
[c0132ebc] __device_release_driver+0x48/0x90
[c0133030] driver_detach+0xb0/0xdc
[c01327c8] bus_remove_driver+0x50/0x6c
[c01332a8] driver_unregister+0x18/0x38
[c01a1760] usb_deregister+0x3c/0x58
[e1072cac] atp_exit+0x18/0x40c [atp]
[c00360ac] sys_delete_module+0x19c/0x214
[c0004660] ret_from_syscall+0x0/0x44

Does anybody have an idea or should I try to debug this further ?

Thanks,

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 15:50 +0200, Stelian Pop a écrit :

> I use the 'atp' input driver from http://popies.net/atp/ to drive this
> touchpad. When removing the driver I also get an oops, possibly related
> to the previous failure to create the sysfs file:
> usbcore: deregistering driver atp
> Oops: kernel access of bad area, sig: 11 [#1]
> NIP: C009F5F8 LR: C00A1728 SP: C3AD9DE0 REGS: c3ad9d30 TRAP: 0300    Not
> tainted
> MSR: 00009032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
> DAR: 0000000C, DSISR: 40000000
> TASK = ccc94150[7148] 'rmmod' THREAD: c3ad8000
> Last syscall: 129
> GPR00: 00000000 C3AD9DE0 CCC94150 C6ED3D48 C02C889C DE6273C7 00000000
> 00000000
> GPR08: 00000001 C6ED3CD4 DFFF9410 00000000 64A79ADA 1001A18C 100C0000
> 100A0000
> GPR16: 00000000 100FEF88 24222482 100C0000 100F2CE8 100F2BE8 10001430
> 00000000
> GPR24: 10000000 00000000 00000000 C02C889C DE76819C DE984678 00000000
> E1074280
> NIP [c009f5f8] sysfs_hash_and_remove+0x3c/0x170
> LR [c00a1728] sysfs_remove_link+0x14/0x24
> Call trace:
> [c00a1728] sysfs_remove_link+0x14/0x24
> [c0132ebc] __device_release_driver+0x48/0x90
> [c0133030] driver_detach+0xb0/0xdc
> [c01327c8] bus_remove_driver+0x50/0x6c
> [c01332a8] driver_unregister+0x18/0x38
> [c01a1760] usb_deregister+0x3c/0x58
> [e1072cac] atp_exit+0x18/0x40c [atp]
> [c00360ac] sys_delete_module+0x19c/0x214
> [c0004660] ret_from_syscall+0x0/0x44

Some more details which may be relevant.

The atp module is automatically loaded on boot. However, it looks like
its probe function was not called or didn't succeed on the first
invocation, in the previous tests I had to remove and reload the module
to make it work.

This time however, after the rmmod, when insmod'ing it again I get:
	usbcore: deregistering driver atp
	input: atp connected
	Oops: kernel access of bad area, sig: 11 [#1]
	NIP: C00A039C LR: C00A037C SP: CD801D60 REGS: cd801cb0 TRAP: 0300
Not Tainted
	MSR: 00009032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
	DAR: 0000000C, DSISR: 40000000
	TASK = cdea5790[4797] 'modprobe' THREAD: cd800000
	Last syscall: 128
	GPR00: DAF0F458 CD801D60 CDEA5790 DAF0F458 00000000 00000000 DAF0F47C
00000020
	GPR08: DFFF9410 DAF0F45C 0000000C DAF0F464 0A6FFDC0 1001E294 00000000
100013C4
	GPR16: 00000000 00000000 00000000 00000000 100013C4 1001EA00 1001EB70
00000000
	GPR24: 00000000 10018868 C0340000 00000000 E2273294 DE49EC80 00000000
DAF0F458
	NIP [c00a039c] sysfs_new_dirent+0x64/0x94
	LR [c00a037c] sysfs_new_dirent+0x44/0x94
	Call trace:
	[c00a03f0] sysfs_make_dirent+0x24/0x90
	[c00a1614] sysfs_add_link+0x94/0xd0
	[c00a16c8] sysfs_create_link+0x78/0xc4
	[c0132bfc] device_bind_driver+0x54/0x68
	[c0132c98] driver_probe_device+0x88/0xe4
	[c0132e34] __driver_attach+0x8c/0x98
	[c0132228] bus_for_each_dev+0x74/0xa4
	[c0132e64] driver_attach+0x24/0x34
	[c0132750] bus_add_driver+0x98/0xc0
	[c013327c] driver_register+0x38/0x4c
	[c01a16c0] usb_register+0x68/0xcc
	[e106a018] atp_init+0x18/0x48 [atp]
	[c0038268] sys_init_module+0x228/0x328
	[c0004660] ret_from_syscall+0x0/0x44

However, on my previous test I was able to rmmod the driver, reload it
again without any oops (and use it, it was functional), the oops
appeared only on the second rmmod. This looks like a synchronisation
issue somewhere...

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 16:07 +0200, Stelian Pop a écrit :
> Le mercredi 22 juin 2005 à 15:50 +0200, Stelian Pop a écrit :
> 
> > I use the 'atp' input driver from http://popies.net/atp/ to drive this
> > touchpad. When removing the driver I also get an oops, possibly related
> > to the previous failure to create the sysfs file:

Ok, there are two separate problems here:

1. The sysfs intf entry is not created, and this causes the oops later
when trying to remove the entry, etc.

   I've tracked this problem back to this patch: 
	[PATCH] driver core: fix error handling in bus_add_device
http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ca2b94ba12f3c36fd3d6ed9d38b3798d4dad0d8b

   Once the patch above is reverted, I have no more oops, my driver can
be loaded/unloaded just fine, and the /sys/devices/.../ is present.

   However, I'm not really sure if the problem comes from the above
patch or from my driver which should manually call
usb_create_sysfs_intf_files() or something equivalent.

2. There is still a problem with the early loading of the driver. If
loaded at boot, it won't work. If I rmmod/insmod it later it does.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: usb sysfs intf files no longer created when probe fails

[Greg KH]

On Wed, Jun 22, 2005 at 03:50:56PM +0200, Stelian Pop wrote:
> I use the 'atp' input driver from http://popies.net/atp/ to drive this
> touchpad. When removing the driver I also get an oops, possibly related
> to the previous failure to create the sysfs file:

Sounds like a bug in that driver, care to ask the authors of it about
this?

thanks,

greg k-h

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 07:59 -0700, Greg KH a écrit :
> On Wed, Jun 22, 2005 at 03:50:56PM +0200, Stelian Pop wrote:
> > I use the 'atp' input driver from http://popies.net/atp/ to drive this
> > touchpad. When removing the driver I also get an oops, possibly related
> > to the previous failure to create the sysfs file:
> 
> Sounds like a bug in that driver, care to ask the authors of it about
> this?

I am the author :)

That driver worked until yesterday, and I was not able to find out about
any API change which would disrupt it now, that's why I reported this to
the list... 

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Alan Stern]

On Wed, 22 Jun 2005, Stelian Pop wrote:

> Le mercredi 22 juin 2005 à 07:59 -0700, Greg KH a écrit :
> > On Wed, Jun 22, 2005 at 03:50:56PM +0200, Stelian Pop wrote:
> > > I use the 'atp' input driver from http://popies.net/atp/ to drive this
> > > touchpad. When removing the driver I also get an oops, possibly related
> > > to the previous failure to create the sysfs file:
> > 
> > Sounds like a bug in that driver, care to ask the authors of it about
> > this?
> 
> I am the author :)
> 
> That driver worked until yesterday, and I was not able to find out about
> any API change which would disrupt it now, that's why I reported this to
> the list... 

This is a curious aspect of the driver model core.  Should failure of a 
driver to bind be considered serious enough to cause device_add to fail?
The current answer is Yes unless the driver's probe routine returns 
-ENODEV or -ENXIO, in which case the failure is not considered serious.

IMO this is a perverse way of doing things.  The existence of a device has 
nothing to do with what driver is bound to it.  Either the device exists 
or it doesn't -- and if it exists, failure to bind a driver shouldn't 
prevent adding the device into sysfs.  Right now, however, it does.

Alan Stern

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 11:41 -0400, Alan Stern a écrit :

> This is a curious aspect of the driver model core.  Should failure of a 
> driver to bind be considered serious enough to cause device_add to fail?
> The current answer is Yes unless the driver's probe routine returns 
> -ENODEV or -ENXIO, in which case the failure is not considered serious.

Indeed. I've also tracked my problem down to the hid core which returns
-EIO when it fails to drive an unknown HID device, instead of a more
logical -ENODEV (this is not a failure to init a known device, but
rather the impossibility to init an unknown device).

The patch below solves the problem for me:

Index: linux-2.6-trunk.git/drivers/usb/input/hid-core.c
===================================================================
--- linux-2.6-trunk.git.orig/drivers/usb/input/hid-core.c	2005-06-22
10:33:23.000000000 +0200
+++ linux-2.6-trunk.git/drivers/usb/input/hid-core.c	2005-06-22
17:43:10.000000000 +0200
@@ -1784,7 +1784,7 @@
 	if (!hid->claimed) {
 		printk ("HID device not claimed by input or hiddev\n");
 		hid_disconnect(intf);
-		return -EIO;
+		return -ENODEV;
 	}
 
 	printk(KERN_INFO);


> IMO this is a perverse way of doing things.  The existence of a device has 
> nothing to do with what driver is bound to it.  Either the device exists 
> or it doesn't -- and if it exists, failure to bind a driver shouldn't 
> prevent adding the device into sysfs.  Right now, however, it does.

I agree, presence in /sys/devices shouldn't be related to the existence
or success/failure of a driver. The link between /sys/class
towards /sys/devices is already saying this.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] usb sysfs intf files no longer created when probe fails

[Alan Stern]

On Wed, 22 Jun 2005, Stelian Pop wrote:

> Notice the '1-2:1.1' is missing. Upon booting I get:
> 
> Jun 22 13:34:04 localhost kernel: HID device not claimed by input or hiddev
> Jun 22 13:34:04 localhost kernel: usbhid: probe of 1-2:1.1 failed with error -5
> Jun 22 13:34:04 localhost kernel: usb 1-2: device_add(1-2:1.1) --> -5

> Ok, there are two separate problems here:
> 
> 1. The sysfs intf entry is not created, and this causes the oops later
> when trying to remove the entry, etc.
> 
>    I've tracked this problem back to this patch: 
> 	[PATCH] driver core: fix error handling in bus_add_device
> http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ca2b94ba12f3c36fd3d6ed9d38b3798d4dad0d8b
> 
>    Once the patch above is reverted, I have no more oops, my driver can
> be loaded/unloaded just fine, and the /sys/devices/.../ is present.
> 
>    However, I'm not really sure if the problem comes from the above
> patch or from my driver which should manually call
> usb_create_sysfs_intf_files() or something equivalent.

You shouldn't call usb_create_sysfs_intf_files in any case.

Your driver is returning -EIO from its probe routine according to the log,
so it's not getting bound to the device.  Hence there shouldn't be any
attempt to unbind the device when your driver is removed.  This is a bug
in usbcore; it tries to delete all the interfaces without checking whether 
they were successfully added.

Alan Stern

Re: [linux-usb-devel] usb sysfs intf files no longer created when probe fails

[Greg KH]

On Wed, Jun 22, 2005 at 04:56:30PM +0200, Stelian Pop wrote:
> Le mercredi 22 juin 2005 ?? 16:07 +0200, Stelian Pop a ??crit :
> > Le mercredi 22 juin 2005 ?? 15:50 +0200, Stelian Pop a ??crit :
> > 
> > > I use the 'atp' input driver from http://popies.net/atp/ to drive this
> > > touchpad. When removing the driver I also get an oops, possibly related
> > > to the previous failure to create the sysfs file:
> 
> Ok, there are two separate problems here:
> 
> 1. The sysfs intf entry is not created, and this causes the oops later
> when trying to remove the entry, etc.
> 
>    I've tracked this problem back to this patch: 
> 	[PATCH] driver core: fix error handling in bus_add_device
> http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ca2b94ba12f3c36fd3d6ed9d38b3798d4dad0d8b
> 
>    Once the patch above is reverted, I have no more oops, my driver can
> be loaded/unloaded just fine, and the /sys/devices/.../ is present.

Thanks for tracking this down.

Hm, we must be actually checking the return value somewhere, and dying
because of it.  Let me dig deeper and find out.

Oh, can you enable debugging for the driver core (it's a config option)
and the USB core and without that patch reverted, let me know what the
syslog shows when the probe fails?

>    However, I'm not really sure if the problem comes from the above
> patch or from my driver which should manually call
> usb_create_sysfs_intf_files() or something equivalent.

No, you should not have to call that from your driver.  In looking at
your code, I don't see anything obviously wrong.  It works just fine
with 2.6.12, right?

> 2. There is still a problem with the early loading of the driver. If
> loaded at boot, it won't work. If I rmmod/insmod it later it does.

Does that also work with 2.6.12?

thanks,

greg k-h

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Greg KH]

On Wed, Jun 22, 2005 at 05:53:28PM +0200, Stelian Pop wrote:
> Le mercredi 22 juin 2005 ?? 11:41 -0400, Alan Stern a ??crit :
> 
> > This is a curious aspect of the driver model core.  Should failure of a 
> > driver to bind be considered serious enough to cause device_add to fail?
> > The current answer is Yes unless the driver's probe routine returns 
> > -ENODEV or -ENXIO, in which case the failure is not considered serious.
> 
> Indeed. I've also tracked my problem down to the hid core which returns
> -EIO when it fails to drive an unknown HID device, instead of a more
> logical -ENODEV (this is not a failure to init a known device, but
> rather the impossibility to init an unknown device).
> 
> The patch below solves the problem for me:

Damm, beat me by a few minutes :)

Yes, this is the proper fix for this.

But to answer Alan's main question, I think you are correct, we should
not fail device_add if binding a device fails.  I can see this causing a
lot of very difficult problems in the future (including the fact that
I've been hitting this bug with a new driver I'm writing and didn't even
realize it...)

So, I'll apply this one, and revert the main part of Hannes's patch too.

Thanks for tracking this down.

greg k-h

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Greg KH]

On Wed, Jun 22, 2005 at 05:53:28PM +0200, Stelian Pop wrote:
> Le mercredi 22 juin 2005 ?? 11:41 -0400, Alan Stern a ??crit :
> 
> > This is a curious aspect of the driver model core.  Should failure of a 
> > driver to bind be considered serious enough to cause device_add to fail?
> > The current answer is Yes unless the driver's probe routine returns 
> > -ENODEV or -ENXIO, in which case the failure is not considered serious.
> 
> Indeed. I've also tracked my problem down to the hid core which returns
> -EIO when it fails to drive an unknown HID device, instead of a more
> logical -ENODEV (this is not a failure to init a known device, but
> rather the impossibility to init an unknown device).
> 
> The patch below solves the problem for me:
> 
> Index: linux-2.6-trunk.git/drivers/usb/input/hid-core.c
> ===================================================================
> --- linux-2.6-trunk.git.orig/drivers/usb/input/hid-core.c	2005-06-22
> 10:33:23.000000000 +0200
> +++ linux-2.6-trunk.git/drivers/usb/input/hid-core.c	2005-06-22
> 17:43:10.000000000 +0200
> @@ -1784,7 +1784,7 @@
>  	if (!hid->claimed) {
>  		printk ("HID device not claimed by input or hiddev\n");
>  		hid_disconnect(intf);
> -		return -EIO;
> +		return -ENODEV;
>  	}

Also need to do the same a few lines above in the code.  I've fixed that
too now.

thanks again,

greg k-h

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 09:26 -0700, Greg KH a écrit :

> > -		return -EIO;
> > +		return -ENODEV;
> >  	}
> 
> Also need to do the same a few lines above in the code.  I've fixed that
> too now.

Indeed, especially since the only way to use an alternate driver for a
real hid device is to blacklist it using HID_QUIRK_IGNORE, and in this
case we go through that path.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] Re: usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 09:22 -0700, Greg KH a écrit :
> On Wed, Jun 22, 2005 at 05:53:28PM +0200, Stelian Pop wrote:
> > Le mercredi 22 juin 2005 ?? 11:41 -0400, Alan Stern a ??crit :
> > 
> > > This is a curious aspect of the driver model core.  Should failure of a 
> > > driver to bind be considered serious enough to cause device_add to fail?
> > > The current answer is Yes unless the driver's probe routine returns 
> > > -ENODEV or -ENXIO, in which case the failure is not considered serious.
> > 
> > Indeed. I've also tracked my problem down to the hid core which returns
> > -EIO when it fails to drive an unknown HID device, instead of a more
> > logical -ENODEV (this is not a failure to init a known device, but
> > rather the impossibility to init an unknown device).
> > 
> > The patch below solves the problem for me:
> 
> Damm, beat me by a few minutes :)

:)

> Yes, this is the proper fix for this.
> 
> But to answer Alan's main question, I think you are correct, we should
> not fail device_add if binding a device fails.  I can see this causing a
> lot of very difficult problems in the future (including the fact that
> I've been hitting this bug with a new driver I'm writing and didn't even
> realize it...)
> 
> So, I'll apply this one, and revert the main part of Hannes's patch too.

Thanks.

Stelian.
-- 
Stelian Pop <stelian@popies.net>

Re: [linux-usb-devel] usb sysfs intf files no longer created when probe fails

[Stelian Pop]

Le mercredi 22 juin 2005 à 12:03 -0400, Alan Stern a écrit :
> On Wed, 22 Jun 2005, Stelian Pop wrote:
> 
> > Notice the '1-2:1.1' is missing. Upon booting I get:
> > 
> > Jun 22 13:34:04 localhost kernel: HID device not claimed by input or hiddev
> > Jun 22 13:34:04 localhost kernel: usbhid: probe of 1-2:1.1 failed with error -5
> > Jun 22 13:34:04 localhost kernel: usb 1-2: device_add(1-2:1.1) --> -5

> You shouldn't call usb_create_sysfs_intf_files in any case.

Ok.

> Your driver is returning -EIO from its probe routine according to the log,
> so it's not getting bound to the device. 

Actually that's usbhid which returns -EIO.

>  Hence there shouldn't be any
> attempt to unbind the device when your driver is removed.  This is a bug
> in usbcore; it tries to delete all the interfaces without checking whether 
> they were successfully added.

Since this is fixed by reverting the device_add patch, I'm wondering if
this isn't a driver model core bug, where it tries to device_remove all
the "devices" even if they weren't correctly added before...

But I haven't looked closely at the code, this is just a thought.

Stelian.
-- 
Stelian Pop <stelian@popies.net>