[OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

The latest change in sysfs/symlink (conversion to use kobject_name instead
of name fiedld directly) broke atmel_cs driver:

Apr 23 00:30:10 core kernel: Oops: 0000 [#1]
Apr 23 00:30:10 core kernel: PREEMPT
Apr 23 00:30:10 core kernel: CPU:    0
Apr 23 00:30:10 core kernel: EIP:    0060:[<c0182ef9>]    Not tainted
Apr 23 00:30:10 core kernel: EFLAGS: 00010246   (2.6.6-rc2)
Apr 23 00:30:10 core kernel: EIP is at object_path_length+0x19/0x30
Apr 23 00:30:10 core kernel: eax: 00000000   ebx: 00000000   ecx: ffffffff   edx: e1930284
Apr 23 00:30:10 core kernel: esi: 00000001   edi: 00000000   ebp: dedd5d64   esp: dedd5d58
Apr 23 00:30:10 core kernel: ds: 007b   es: 007b   ss: 0068
Apr 23 00:30:10 core kernel: Process ifenslave (pid: 1693, threadinfo=dedd4000 task=ded6c1f0)
Apr 23 00:30:10 core kernel: Stack: 00000003 00000000 e1930284 dedd5d98 c0182f99 e1930284 00000000 e192c6b4
Apr 23 00:30:10 core kernel:        dedd5d90 c01ac578 c030f37b c03876c0 de2d4de0 e192c700 de5c80c0 e192c6a0
Apr 23 00:30:10 core kernel:        dedd5dac c0211490 de5c80c8 e1930284 c0312fc6 dedd5dd4 c02117ad de5c80c0
Apr 23 00:30:10 core kernel: Call Trace:
Apr 23 00:30:10 core kernel:  [<c0182f99>] sysfs_create_link+0x29/0x140
Apr 23 00:30:10 core kernel:  [<c01ac578>] kobject_hotplug+0x58/0x60
Apr 23 00:30:10 core kernel:  [<c0211490>] class_device_dev_link+0x30/0x40
Apr 23 00:30:10 core kernel:  [<c02117ad>] class_device_add+0xed/0x130
Apr 23 00:30:10 core kernel:  [<e192a618>] fw_register_class_device+0x118/0x180 [firmware_class]
Apr 23 00:30:10 core kernel:  [<e192a6ab>] fw_setup_class_device+0x2b/0x120 [firmware_class]
Apr 23 00:30:10 core kernel:  [<e192a802>] request_firmware+0x62/0x170 [firmware_class]
Apr 23 00:30:10 core kernel:  [<e197ccce>] reset_atmel_card+0x7ae/0x800 [atmel]
Apr 23 00:30:10 core kernel:  [<e1978a82>] atmel_open+0x22/0x50 [atmel]
Apr 23 00:30:10 core kernel:  [<c027f8b8>] dev_open+0xd8/0x120
Apr 23 00:30:10 core kernel:  [<c0284915>] dev_mc_upload+0x25/0x50
Apr 23 00:30:10 core kernel:  [<c0280f78>] dev_change_flags+0x138/0x150
Apr 23 00:30:10 core kernel:  [<c027f724>] dev_load+0x24/0x80
Apr 23 00:30:10 core kernel:  [<c02c0c1a>] devinet_ioctl+0x26a/0x660
Apr 23 00:30:10 core kernel:  [<c02c3324>] inet_ioctl+0x64/0xb0
Apr 23 00:30:10 core kernel:  [<c0278385>] sock_ioctl+0xf5/0x2b0
Apr 23 00:30:10 core kernel:  [<c0161320>] sys_ioctl+0x100/0x260
Apr 23 00:30:10 core kernel:  [<c010510b>] syscall_call+0x7/0xb

Below is the "fix" that helps avoid oopsing, and should be removed when
atmel_cs driver properly registers atmel_device.
 
-- 
Dmitry

===== drivers/net/wireless/atmel_cs.c 1.11 vs edited =====
--- 1.11/drivers/net/wireless/atmel_cs.c	Thu Feb  5 05:04:40 2004
+++ edited/drivers/net/wireless/atmel_cs.c	Fri Apr 23 01:14:11 2004
@@ -538,7 +538,11 @@
 		       "atmel: cannot assign IRQ: check that CONFIG_ISA is set in kernel config.");
 		goto cs_failed;
 	}
-	
+
+	/* FIXME: Horrible jhack to prevent sysfs from OOPsing */
+	/* FIXME: We really shoudl be registering atmel-device */
+	atmel_device.kobj.k_name = atmel_device.kobj.name;
+
 	((local_info_t*)link->priv)->eth_dev = 
 		init_atmel_card(link->irq.AssignedIRQ,
 				link->io.BasePort1,

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Dmitry,

> The latest change in sysfs/symlink (conversion to use kobject_name instead
> of name fiedld directly) broke atmel_cs driver:
> 
> Apr 23 00:30:10 core kernel: Oops: 0000 [#1]
> Apr 23 00:30:10 core kernel: PREEMPT
> Apr 23 00:30:10 core kernel: CPU:    0
> Apr 23 00:30:10 core kernel: EIP:    0060:[<c0182ef9>]    Not tainted
> Apr 23 00:30:10 core kernel: EFLAGS: 00010246   (2.6.6-rc2)
> Apr 23 00:30:10 core kernel: EIP is at object_path_length+0x19/0x30
> Apr 23 00:30:10 core kernel: eax: 00000000   ebx: 00000000   ecx: ffffffff   edx: e1930284
> Apr 23 00:30:10 core kernel: esi: 00000001   edi: 00000000   ebp: dedd5d64   esp: dedd5d58
> Apr 23 00:30:10 core kernel: ds: 007b   es: 007b   ss: 0068
> Apr 23 00:30:10 core kernel: Process ifenslave (pid: 1693, threadinfo=dedd4000 task=ded6c1f0)
> Apr 23 00:30:10 core kernel: Stack: 00000003 00000000 e1930284 dedd5d98 c0182f99 e1930284 00000000 e192c6b4
> Apr 23 00:30:10 core kernel:        dedd5d90 c01ac578 c030f37b c03876c0 de2d4de0 e192c700 de5c80c0 e192c6a0
> Apr 23 00:30:10 core kernel:        dedd5dac c0211490 de5c80c8 e1930284 c0312fc6 dedd5dd4 c02117ad de5c80c0
> Apr 23 00:30:10 core kernel: Call Trace:
> Apr 23 00:30:10 core kernel:  [<c0182f99>] sysfs_create_link+0x29/0x140
> Apr 23 00:30:10 core kernel:  [<c01ac578>] kobject_hotplug+0x58/0x60
> Apr 23 00:30:10 core kernel:  [<c0211490>] class_device_dev_link+0x30/0x40
> Apr 23 00:30:10 core kernel:  [<c02117ad>] class_device_add+0xed/0x130
> Apr 23 00:30:10 core kernel:  [<e192a618>] fw_register_class_device+0x118/0x180 [firmware_class]
> Apr 23 00:30:10 core kernel:  [<e192a6ab>] fw_setup_class_device+0x2b/0x120 [firmware_class]
> Apr 23 00:30:10 core kernel:  [<e192a802>] request_firmware+0x62/0x170 [firmware_class]
> Apr 23 00:30:10 core kernel:  [<e197ccce>] reset_atmel_card+0x7ae/0x800 [atmel]
> Apr 23 00:30:10 core kernel:  [<e1978a82>] atmel_open+0x22/0x50 [atmel]
> Apr 23 00:30:10 core kernel:  [<c027f8b8>] dev_open+0xd8/0x120
> Apr 23 00:30:10 core kernel:  [<c0284915>] dev_mc_upload+0x25/0x50
> Apr 23 00:30:10 core kernel:  [<c0280f78>] dev_change_flags+0x138/0x150
> Apr 23 00:30:10 core kernel:  [<c027f724>] dev_load+0x24/0x80
> Apr 23 00:30:10 core kernel:  [<c02c0c1a>] devinet_ioctl+0x26a/0x660
> Apr 23 00:30:10 core kernel:  [<c02c3324>] inet_ioctl+0x64/0xb0
> Apr 23 00:30:10 core kernel:  [<c0278385>] sock_ioctl+0xf5/0x2b0
> Apr 23 00:30:10 core kernel:  [<c0161320>] sys_ioctl+0x100/0x260
> Apr 23 00:30:10 core kernel:  [<c010510b>] syscall_call+0x7/0xb
> 
> Below is the "fix" that helps avoid oopsing, and should be removed when
> atmel_cs driver properly registers atmel_device.

I haven't tested it yet, but the same problem should apply to the
bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
available that integrates the PCMCIA subsystem into the driver model, so
we don't have to hack around it if a firmware download is needed?

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Friday 23 April 2004 07:25 am, Marcel Holtmann wrote:
> Hi Dmitry,
> 
> > The latest change in sysfs/symlink (conversion to use kobject_name instead
> > of name fiedld directly) broke atmel_cs driver:
> > 
> > Apr 23 00:30:10 core kernel: Oops: 0000 [#1]
> > Apr 23 00:30:10 core kernel: PREEMPT
> > Apr 23 00:30:10 core kernel: CPU:    0
> > Apr 23 00:30:10 core kernel: EIP:    0060:[<c0182ef9>]    Not tainted
> > Apr 23 00:30:10 core kernel: EFLAGS: 00010246   (2.6.6-rc2)
> > Apr 23 00:30:10 core kernel: EIP is at object_path_length+0x19/0x30
<skip>
> > Apr 23 00:30:10 core kernel: Call Trace:
> > Apr 23 00:30:10 core kernel:  [<c0182f99>] sysfs_create_link+0x29/0x140
> > Apr 23 00:30:10 core kernel:  [<c01ac578>] kobject_hotplug+0x58/0x60
> > Apr 23 00:30:10 core kernel:  [<c0211490>] class_device_dev_link+0x30/0x40
<skip>
> > 
> > Below is the "fix" that helps avoid oopsing, and should be removed when
> > atmel_cs driver properly registers atmel_device.
> 
> I haven't tested it yet, but the same problem should apply to the
> bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
> available that integrates the PCMCIA subsystem into the driver model, so
> we don't have to hack around it if a firmware download is needed?
> 
I do not know. But the problem seems to be somewhat widespread - I just got
oops with the following trace:

 [<c0182f99>] sysfs_create_link+0x29/0x140
 [<c01ac578>] kobject_hotplug+0x58/0x60
 [<c0211490>] class_device_dev_link+0x30/0x40
 [<c02117ad>] class_device_add+0xed/0x130
 [<e185ffab>] usb_register_dev+0x12b/0x170 [usbcore]
 [<e1b2bf2a>] hiddev_connect+0x7a/0x120 [usbhid]

I think we should not oops, just complain loudly, when we come across a
kobject which has never beek kobject_add()ed, like in patch below.

-- 
Dmitry

===== include/linux/kobject.h 1.26 vs edited =====
--- 1.26/include/linux/kobject.h	Thu Mar 11 08:20:22 2004
+++ edited/include/linux/kobject.h	Fri Apr 23 07:58:52 2004
@@ -39,6 +39,11 @@
 
 static inline char * kobject_name(struct kobject * kobj)
 {
+	if (unlikely(!kobj->k_name)) {
+		printk("kobject_name(): not registered kobject\n");
+		dump_stack();
+		return kobj->name;
+	}
 	return kobj->k_name;
 }
 

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Dmitry,

> > I haven't tested it yet, but the same problem should apply to the
> > bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
> > available that integrates the PCMCIA subsystem into the driver model, so
> > we don't have to hack around it if a firmware download is needed?
> >
> I do not know. But the problem seems to be somewhat widespread - I just got
> oops with the following trace:
> 
>  [<c0182f99>] sysfs_create_link+0x29/0x140
>  [<c01ac578>] kobject_hotplug+0x58/0x60
>  [<c0211490>] class_device_dev_link+0x30/0x40
>  [<c02117ad>] class_device_add+0xed/0x130
>  [<e185ffab>] usb_register_dev+0x12b/0x170 [usbcore]
>  [<e1b2bf2a>] hiddev_connect+0x7a/0x120 [usbhid]
> 
> I think we should not oops, just complain loudly, when we come across a
> kobject which has never beek kobject_add()ed, like in patch below.

I can't tell you anything about the hiddev problem, but for the PCMCIA
subsystem we have the problem that right now it is not integrated into
the driver model. Maybe a dummy integration for PCMCIA would be nice or
can we get a device_simple like we have class_simple?

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 01:42:45AM -0500, Dmitry Torokhov wrote:
> The latest change in sysfs/symlink (conversion to use kobject_name instead
> of name fiedld directly) broke atmel_cs driver:

That's because that driver is broken in the first place.  Let me repeat:
	NEVER DECLARE A STRUCT DEVICE STATICALLY!!!

Your workaround is just that, a workaround for something that is already
broken, please fix it up correctly, and do not paper over the real root
problem...

The fact that it hasn't oopsed before this symlink fix is only a lucky
accident.

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 08:02:40AM -0500, Dmitry Torokhov wrote:
> On Friday 23 April 2004 07:25 am, Marcel Holtmann wrote:
> > Hi Dmitry,
> > 
> > > The latest change in sysfs/symlink (conversion to use kobject_name instead
> > > of name fiedld directly) broke atmel_cs driver:
> > > 
> > > Apr 23 00:30:10 core kernel: Oops: 0000 [#1]
> > > Apr 23 00:30:10 core kernel: PREEMPT
> > > Apr 23 00:30:10 core kernel: CPU:    0
> > > Apr 23 00:30:10 core kernel: EIP:    0060:[<c0182ef9>]    Not tainted
> > > Apr 23 00:30:10 core kernel: EFLAGS: 00010246   (2.6.6-rc2)
> > > Apr 23 00:30:10 core kernel: EIP is at object_path_length+0x19/0x30
> <skip>
> > > Apr 23 00:30:10 core kernel: Call Trace:
> > > Apr 23 00:30:10 core kernel:  [<c0182f99>] sysfs_create_link+0x29/0x140
> > > Apr 23 00:30:10 core kernel:  [<c01ac578>] kobject_hotplug+0x58/0x60
> > > Apr 23 00:30:10 core kernel:  [<c0211490>] class_device_dev_link+0x30/0x40
> <skip>
> > > 
> > > Below is the "fix" that helps avoid oopsing, and should be removed when
> > > atmel_cs driver properly registers atmel_device.
> > 
> > I haven't tested it yet, but the same problem should apply to the
> > bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
> > available that integrates the PCMCIA subsystem into the driver model, so
> > we don't have to hack around it if a firmware download is needed?
> > 
> I do not know. But the problem seems to be somewhat widespread - I just got
> oops with the following trace:
> 
>  [<c0182f99>] sysfs_create_link+0x29/0x140
>  [<c01ac578>] kobject_hotplug+0x58/0x60
>  [<c0211490>] class_device_dev_link+0x30/0x40
>  [<c02117ad>] class_device_add+0xed/0x130
>  [<e185ffab>] usb_register_dev+0x12b/0x170 [usbcore]
>  [<e1b2bf2a>] hiddev_connect+0x7a/0x120 [usbhid]
> 
> I think we should not oops, just complain loudly, when we come across a
> kobject which has never beek kobject_add()ed, like in patch below.

No, we need to oops, as that's a real bug.  Can you post the whole oops
that was generated with this usb problem?  I can't seem to duplicate
this here.

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Friday 23 April 2004 09:26 am, Marcel Holtmann wrote:
> I can't tell you anything about the hiddev problem, but for the PCMCIA
> subsystem we have the problem that right now it is not integrated into
> the driver model. Maybe a dummy integration for PCMCIA would be nice or
> can we get a device_simple like we have class_simple?
> 
I wonder if===== drivers/net/wireless/atmel_cs.c 1.11 vs edited =====
--- 1.11/drivers/net/wireless/atmel_cs.c	Thu Feb  5 05:04:40 2004
+++ edited/drivers/net/wireless/atmel_cs.c	Fri Apr 23 11:43:42 2004
@@ -348,17 +348,13 @@
 	{ 0, 0, "11WAVE/11WP611AL-E", "atmel_at76c502e%s.bin", "11WAVE WaveBuddy" } 
 };
 
-/* This is strictly temporary, until PCMCIA devices get integrated into the device model. */
-static struct device atmel_device = {
-        .bus_id    = "pcmcia",
-};
-
 static void atmel_config(dev_link_t *link)
 {
 	client_handle_t handle;
 	tuple_t tuple;
 	cisparse_t parse;
 	local_info_t *dev;
+	struct device *atmel_device;
 	int last_fn, last_ret;
 	u_char buf[64];
 	int card_index = -1, done = 0;
@@ -538,14 +534,25 @@
 		       "atmel: cannot assign IRQ: check that CONFIG_ISA is set in kernel config.");
 		goto cs_failed;
 	}
-	
+
+	CS_CHECK(GetSysDevice, pcmcia_get_sys_device(link->handle, &atmel_device));
+
+	if (!atmel_device) {
+		printk(KERN_ALERT "atmel: cannot get sys device from the card.\n");
+		goto cs_failed;
+	}
+
 	((local_info_t*)link->priv)->eth_dev = 
 		init_atmel_card(link->irq.AssignedIRQ,
 				link->io.BasePort1,
 				card_index == -1 ? NULL :  card_table[card_index].firmware,
-				&atmel_device,
+				atmel_device,
 				card_present, 
 				link);
+
+	put_device(atmel_device);
+	atmel_device = NULL;
+
 	if (!((local_info_t*)link->priv)->eth_dev) 
 		goto cs_failed;
 	
===== drivers/pcmcia/cs.c 1.78 vs edited =====
--- 1.78/drivers/pcmcia/cs.c	Mon Apr 19 03:12:13 2004
+++ edited/drivers/pcmcia/cs.c	Fri Apr 23 11:51:19 2004
@@ -1009,6 +1009,24 @@
     return CS_SUCCESS;
 } /* get_configuration_info */
 
+/*====================================================================*/
+
+int pcmcia_get_sys_device(client_handle_t handle, struct device **sys_dev)
+{
+	struct pcmcia_socket *s;
+
+	if (CHECK_HANDLE(handle))
+		return CS_BAD_HANDLE;
+
+	s = SOCKET(handle);
+	if (!(s->state & SOCKET_PRESENT))
+		return CS_NO_CARD;
+
+	*sys_dev = get_device(s->dev.dev);
+
+	return CS_SUCCESS;
+} /* get_sys_device */
+
 /*======================================================================
 
     Return information about this version of Card Services.
@@ -2109,6 +2127,7 @@
 EXPORT_SYMBOL(pcmcia_get_next_tuple);
 EXPORT_SYMBOL(pcmcia_get_next_window);
 EXPORT_SYMBOL(pcmcia_get_status);
+EXPORT_SYMBOL(pcmcia_get_sys_device);
 EXPORT_SYMBOL(pcmcia_get_tuple_data);
 EXPORT_SYMBOL(pcmcia_insert_card);
 EXPORT_SYMBOL(pcmcia_map_mem_page);
===== drivers/pcmcia/ds.c 1.49 vs edited =====
--- 1.49/drivers/pcmcia/ds.c	Sun Mar 14 15:36:00 2004
+++ edited/drivers/pcmcia/ds.c	Fri Apr 23 11:51:47 2004
@@ -307,7 +307,8 @@
     { ResumeCard,			"ResumeCard" },
     { EjectCard,			"EjectCard" },
     { InsertCard,			"InsertCard" },
-    { ReplaceCIS,			"ReplaceCIS" }
+    { ReplaceCIS,			"ReplaceCIS" },
+    { GetSysDevice,			"GetSysDevice" }
 };
 
 
===== include/pcmcia/cs.h 1.9 vs edited =====
--- 1.9/include/pcmcia/cs.h	Sun Mar 14 15:36:00 2004
+++ edited/include/pcmcia/cs.h	Fri Apr 23 11:47:26 2004
@@ -418,9 +418,11 @@
     SetEventMask, SetRegion, ValidateCIS, VendorSpecific,
     WriteMemory, BindDevice, BindMTD, ReportError,
     SuspendCard, ResumeCard, EjectCard, InsertCard, ReplaceCIS,
-    GetFirstWindow, GetNextWindow, GetMemPage
+    GetFirstWindow, GetNextWindow, GetMemPage, GetSysDevice
 };
 
+struct device;
+
 int pcmcia_access_configuration_register(client_handle_t handle, conf_reg_t *reg);
 int pcmcia_deregister_client(client_handle_t handle);
 int pcmcia_get_configuration_info(client_handle_t handle, config_info_t *config);
@@ -431,6 +433,7 @@
 int pcmcia_get_first_window(window_handle_t *win, win_req_t *req);
 int pcmcia_get_next_window(window_handle_t *win, win_req_t *req);
 int pcmcia_get_status(client_handle_t handle, cs_status_t *status);
+int pcmcia_get_sys_device(client_handle_t handle, struct device **sys_dev);
 int pcmcia_get_mem_page(window_handle_t win, memreq_t *req);
 int pcmcia_map_mem_page(window_handle_t win, memreq_t *req);
 int pcmcia_modify_configuration(client_handle_t handle, modconf_t *mod);

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 11:55:59AM -0500, Dmitry Torokhov wrote:
> --- 1.11/drivers/net/wireless/atmel_cs.c	Thu Feb  5 05:04:40 2004
> +++ edited/drivers/net/wireless/atmel_cs.c	Fri Apr 23 11:43:42 2004
> @@ -348,17 +348,13 @@
>  	{ 0, 0, "11WAVE/11WP611AL-E", "atmel_at76c502e%s.bin", "11WAVE WaveBuddy" } 
>  };
>  
> -/* This is strictly temporary, until PCMCIA devices get integrated into the device model. */
> -static struct device atmel_device = {
> -        .bus_id    = "pcmcia",
> -};
> -

<snip>

Much nicer (well, in a wierd way at least.)  It seems that the pcmcia
system is intregrated into the driver model.  Why not push it down into
the individual pcmcia drivers so you don't have to do this GetSysDevice
kind of hack still?

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 08:31:11AM -0700, Greg KH wrote:
> 
> No, we need to oops, as that's a real bug.  Can you post the whole oops
> that was generated with this usb problem?  I can't seem to duplicate
> this here.

Nevermind I dug up a device here that causes this problem.  I'll track
it down...

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 10:19:53AM -0700, Greg KH wrote:
> On Fri, Apr 23, 2004 at 08:31:11AM -0700, Greg KH wrote:
> > 
> > No, we need to oops, as that's a real bug.  Can you post the whole oops
> > that was generated with this usb problem?  I can't seem to duplicate
> > this here.
> 
> Nevermind I dug up a device here that causes this problem.  I'll track
> it down...

Ok, here's a patch that fixes it for me.  I was waiting for a good
reason to finally get rid of this fake usb_interface structure, and now
I have it :)

Let me know if it solves the problem for you too and then I'll send it
off to Linus.

Any objections Vojtech?

thanks,

greg k-h


# USB: fix up fake usb_interface structure in hiddev
#
# This fixes a oops in the current kernel tree.

diff -Nru a/drivers/usb/input/hiddev.c b/drivers/usb/input/hiddev.c
--- a/drivers/usb/input/hiddev.c	Fri Apr 23 11:00:23 2004
+++ b/drivers/usb/input/hiddev.c	Fri Apr 23 11:00:23 2004
@@ -53,7 +53,6 @@
 	wait_queue_head_t wait;
 	struct hid_device *hid;
 	struct hiddev_list *list;
-	struct usb_interface intf;
 };
 
 struct hiddev_list {
@@ -234,7 +233,7 @@
 static struct usb_class_driver hiddev_class;
 static void hiddev_cleanup(struct hiddev *hiddev)
 {
-	usb_deregister_dev(&hiddev->intf, &hiddev_class);
+	usb_deregister_dev(hiddev->hid->intf, &hiddev_class);
 	hiddev_table[hiddev->minor] = NULL;
 	kfree(hiddev);
 }
@@ -775,7 +774,7 @@
 		return -1;
 	memset(hiddev, 0, sizeof(struct hiddev));
 
- 	retval = usb_register_dev(&hiddev->intf, &hiddev_class);
+ 	retval = usb_register_dev(hid->intf, &hiddev_class);
 	if (retval) {
 		err("Not able to get a minor for this device.");
 		kfree(hiddev);
@@ -784,13 +783,13 @@
 
 	init_waitqueue_head(&hiddev->wait);
 
- 	hiddev->minor = hiddev->intf.minor;
- 	hiddev_table[hiddev->intf.minor - HIDDEV_MINOR_BASE] = hiddev;
+ 	hiddev->minor = hid->intf->minor;
+ 	hiddev_table[hid->intf->minor - HIDDEV_MINOR_BASE] = hiddev;
 
 	hiddev->hid = hid;
 	hiddev->exist = 1;
 
- 	hid->minor = hiddev->intf.minor;
+ 	hid->minor = hid->intf->minor;
 	hid->hiddev = hiddev;
 
 	return 0;

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Greg,

> Much nicer (well, in a wierd way at least.)  It seems that the pcmcia
> system is intregrated into the driver model.  Why not push it down into
> the individual pcmcia drivers so you don't have to do this GetSysDevice
> kind of hack still?

let's split the patch into a PCMCIA subsystem part and atmel_cs part and
even if it looks like a hack we need it, because otherwise the atmel_cs
and bt3c_cs drivers are broken now.

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Fri, Apr 23, 2004 at 08:50:54PM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> > Much nicer (well, in a wierd way at least.)  It seems that the pcmcia
> > system is intregrated into the driver model.  Why not push it down into
> > the individual pcmcia drivers so you don't have to do this GetSysDevice
> > kind of hack still?
> 
> let's split the patch into a PCMCIA subsystem part and atmel_cs part and
> even if it looks like a hack we need it, because otherwise the atmel_cs
> and bt3c_cs drivers are broken now.

Well, they have always been broken, it's just that now you get an
obvious oops message :)

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Russell King]

On Fri, Apr 23, 2004 at 02:25:49PM +0200, Marcel Holtmann wrote:
> I haven't tested it yet, but the same problem should apply to the
> bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
> available that integrates the PCMCIA subsystem into the driver model, so
> we don't have to hack around it if a firmware download is needed?

Not yet.  It's something we're working towards, but its going to be
some time yet.  There's a fair queue of long outstanding patches
which need to be processed first.

Plus, before we can consider driver model in PCMCIA, we need to get
the object lifetimes properly sorted.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Russell,

> > I haven't tested it yet, but the same problem should apply to the
> > bt3c_cs driver for the 3Com Bluetooth card. Are there any patches
> > available that integrates the PCMCIA subsystem into the driver model, so
> > we don't have to hack around it if a firmware download is needed?
> 
> Not yet.  It's something we're working towards, but its going to be
> some time yet.  There's a fair queue of long outstanding patches
> which need to be processed first.
> 
> Plus, before we can consider driver model in PCMCIA, we need to get
> the object lifetimes properly sorted.

should we apply the pcmcia_get_sys_device() patch from Dmitry for now to
fix the current drivers that need a device for loading the firmware?

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Russell King]

On Fri, Apr 23, 2004 at 10:16:14AM -0700, Greg KH wrote:
> On Fri, Apr 23, 2004 at 11:55:59AM -0500, Dmitry Torokhov wrote:
> > --- 1.11/drivers/net/wireless/atmel_cs.c	Thu Feb  5 05:04:40 2004
> > +++ edited/drivers/net/wireless/atmel_cs.c	Fri Apr 23 11:43:42 2004
> > @@ -348,17 +348,13 @@
> >  	{ 0, 0, "11WAVE/11WP611AL-E", "atmel_at76c502e%s.bin", "11WAVE WaveBuddy" } 
> >  };
> >  
> > -/* This is strictly temporary, until PCMCIA devices get integrated into the device model. */
> > -static struct device atmel_device = {
> > -        .bus_id    = "pcmcia",
> > -};
> > -
> 
> <snip>
> 
> Much nicer (well, in a wierd way at least.)  It seems that the pcmcia
> system is intregrated into the driver model.  Why not push it down into
> the individual pcmcia drivers so you don't have to do this GetSysDevice
> kind of hack still?

They're actually getting at is the PCI device, or statically allocated
platform device, rather than anything specific to the card.

Obviously this is going to create the silly scenario where people
can attach PCMCIA device attributes to the bridge device, which
provides the wrong API to userspace.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Russell King]

On Fri, Apr 23, 2004 at 10:14:24PM +0200, Marcel Holtmann wrote:
> should we apply the pcmcia_get_sys_device() patch from Dmitry for now to
> fix the current drivers that need a device for loading the firmware?

I don't think so - it obtains the struct device for the bridge itself
which has nothing to do with the card inserted in the slot.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Russell,

> > Much nicer (well, in a wierd way at least.)  It seems that the pcmcia
> > system is intregrated into the driver model.  Why not push it down into
> > the individual pcmcia drivers so you don't have to do this GetSysDevice
> > kind of hack still?
> 
> They're actually getting at is the PCI device, or statically allocated
> platform device, rather than anything specific to the card.
> 
> Obviously this is going to create the silly scenario where people
> can attach PCMCIA device attributes to the bridge device, which
> provides the wrong API to userspace.

do you have any proposals how to fix this now? We only need the device
for loading the firmware.

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Friday 23 April 2004 01:03 pm, Greg KH wrote:
> On Fri, Apr 23, 2004 at 10:19:53AM -0700, Greg KH wrote:
> > On Fri, Apr 23, 2004 at 08:31:11AM -0700, Greg KH wrote:
> > > 
> > > No, we need to oops, as that's a real bug.  Can you post the whole oops
> > > that was generated with this usb problem?  I can't seem to duplicate
> > > this here.
> > 
> > Nevermind I dug up a device here that causes this problem.  I'll track
> > it down...
> 
> Ok, here's a patch that fixes it for me.  I was waiting for a good
> reason to finally get rid of this fake usb_interface structure, and now
> I have it :)
> 

Yes, it does fix it for me, thanks a lot!
Now if somebody could fix the following oops...

Unable to handle kernel paging request at virtual address e1b1a120
 printing eip:
e1b1504a
*pde = 1fe1d067
*pte = 00000000
Oops: 0002 [#1]
PREEMPT
CPU:    0
EIP:    0060:[<e1b1504a>]    Tainted: P
EFLAGS: 00010246   (2.6.6-rc2)
EIP is at hiddev_cleanup+0x2a/0x40 [usbhid]
eax: 00000060   ebx: d7215640   ecx: 00000000   edx: e18728f4
esi: e1b19b80   edi: dfe30800   ebp: df2bfe50   esp: df2bfe44
ds: 007b   es: 007b   ss: 0068
Process khubd (pid: 400, threadinfo=df2be000 task=df3a5230)
Stack: d7922680 e1b19c7c d3e8e000 df2bfe64 e1b14ac8 d7215640 e1b19b80 d7922680
       df2bfe80 e1859106 d7922680 d7922680 d44f9da0 d7922690 e1b19ba0 df2bfe98
       c0210bc6 d7922690 d79226b8 d7922690 dfe308cc df2bfeb0 c0210d03 d7922690
Call Trace:
 [<e1b14ac8>] hid_disconnect+0xb8/0xe0 [usbhid]
 [<e1859106>] usb_unbind_interface+0x76/0x80 [usbcore]
 [<c0210bc6>] device_release_driver+0x66/0x70
 [<c0210d03>] bus_remove_device+0x53/0xa0
 [<c020fbdd>] device_del+0x5d/0xa0
 [<c020fc34>] device_unregister+0x14/0x30
 [<e185f480>] usb_disable_device+0x70/0xb0 [usbcore]
 [<e1859d16>] usb_disconnect+0xa6/0x100 [usbcore]
 [<e1859d58>] usb_disconnect+0xe8/0x100 [usbcore]
 [<e185bfef>] hub_port_connect_change+0x28f/0x2a0 [usbcore]
 [<e185b971>] hub_port_status+0x41/0xb0 [usbcore]
 [<e185c343>] hub_events+0x343/0x3b0 [usbcore]
 [<e185c3e5>] hub_thread+0x35/0xf0 [usbcore]
 [<c0115ef0>] default_wake_function+0x0/0x20
 [<e185c3b0>] hub_thread+0x0/0xf0 [usbcore]
 [<c01032e5>] kernel_thread_helper+0x5/0x10


-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Sat, Apr 24, 2004 at 01:44:04AM -0500, Dmitry Torokhov wrote:
> On Friday 23 April 2004 01:03 pm, Greg KH wrote:
> > On Fri, Apr 23, 2004 at 10:19:53AM -0700, Greg KH wrote:
> > > On Fri, Apr 23, 2004 at 08:31:11AM -0700, Greg KH wrote:
> > > > 
> > > > No, we need to oops, as that's a real bug.  Can you post the whole oops
> > > > that was generated with this usb problem?  I can't seem to duplicate
> > > > this here.
> > > 
> > > Nevermind I dug up a device here that causes this problem.  I'll track
> > > it down...
> > 
> > Ok, here's a patch that fixes it for me.  I was waiting for a good
> > reason to finally get rid of this fake usb_interface structure, and now
> > I have it :)
> > 
> 
> Yes, it does fix it for me, thanks a lot!
> Now if somebody could fix the following oops...

When does that oops happen?  With my patch applied?  Are you yanking out
a device that currently has a userspace progam attached to it?  What
kind of device is it?

Hm, this might solve the problem, can you try the patch below?  It's on
top of my previous patch.

thanks,

greg k-h


--- a/drivers/usb/input/hiddev.c	Fri Apr 23 10:59:52 2004
+++ b/drivers/usb/input/hiddev.c	Sat Apr 24 19:46:29 2004
@@ -233,8 +233,8 @@
 static struct usb_class_driver hiddev_class;
 static void hiddev_cleanup(struct hiddev *hiddev)
 {
-	usb_deregister_dev(hiddev->hid->intf, &hiddev_class);
 	hiddev_table[hiddev->minor] = NULL;
+	usb_deregister_dev(hiddev->hid->intf, &hiddev_class);
 	kfree(hiddev);
 }
 

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Saturday 24 April 2004 09:49 pm, Greg KH wrote:
> On Sat, Apr 24, 2004 at 01:44:04AM -0500, Dmitry Torokhov wrote:
>
> > Yes, it does fix it for me, thanks a lot!
> > Now if somebody could fix the following oops...
> 
> When does that oops happen?  With my patch applied?  Are you yanking out
> a device that currently has a userspace progam attached to it?  What
> kind of device is it?
> 
> Hm, this might solve the problem, can you try the patch below?  It's on
> top of my previous patch.
>
(removed some recipients from the CC list..)
 
No, I am still getting the oops.. hmm.. it seems a little bit different,
but still in the hiddev. I investigated further and the oops only happens
if I yank a HID device connected to an USB hub or if I yank entire hub with
a HID device connected to it. Tried with APC UPS and MS Intellimouse
Explorer. If they are connected directly to the laptop's ports everything is
fine, also other devices (USB printer for example) handle hub disconnection
just fine. It does not matter if I have device open or closed for oops to
happen. And, for the record, oops itself:

Apr 25 16:18:40 core kernel: usb 1-1: USB disconnect, address 3
Apr 25 16:18:40 core kernel: Unable to handle kernel paging request at virtual address e19a9120
Apr 25 16:18:40 core kernel:  printing eip:
Apr 25 16:18:40 core kernel: e19a4031
Apr 25 16:18:40 core kernel: *pde = 1fe1d067
Apr 25 16:18:40 core kernel: *pte = 00000000
Apr 25 16:18:40 core kernel: Oops: 0002 [#1]
Apr 25 16:18:40 core kernel: PREEMPT
Apr 25 16:18:40 core kernel: CPU:    0
Apr 25 16:18:40 core kernel: EIP:    0060:[<e19a4031>]    Not tainted
Apr 25 16:18:40 core kernel: EFLAGS: 00010246   (2.6.6-rc2)
Apr 25 16:18:40 core kernel: EIP is at hiddev_cleanup+0x11/0x40 [usbhid]
Apr 25 16:18:40 core kernel: eax: 00000060   ebx: dd0c4760   ecx: 00000000   edx: de2b7b48
Apr 25 16:18:40 core kernel: esi: e19a8b80   edi: c1565400   ebp: de025e78   esp: de025e6c
Apr 25 16:18:41 core kernel: ds: 007b   es: 007b   ss: 0068
Apr 25 16:18:41 core kernel: Process khubd (pid: 400, threadinfo=de024000 task=ddbd0d30)
Apr 25 16:18:41 core kernel: Stack: de025e78 e185e5ef ddd30000 de025e8c e19a3ac8 dd0c4760 e19a8b80 dfc1d4c0
Apr 25 16:18:41 core kernel:        de025ea8 e1859106 dfc1d4c0 dfc1d4c0 dcfe33e0 dfc1d4d0 e19a8ba0 de025ec0
Apr 25 16:18:41 core kernel:        c0210bc6 dfc1d4d0 dfc1d4f8 dfc1d4d0 c15654cc de025ed8 c0210d03 dfc1d4d0
Apr 25 16:18:41 core kernel: Call Trace:
Apr 25 16:18:41 core kernel:  [<e185e5ef>] usb_unlink_urb+0x3f/0x50 [usbcore]
Apr 25 16:18:41 core kernel:  [<e19a3ac8>] hid_disconnect+0xb8/0xe0 [usbhid]
Apr 25 16:18:41 core kernel:  [<e1859106>] usb_unbind_interface+0x76/0x80 [usbcore]
Apr 25 16:18:41 core kernel:  [<c0210bc6>] device_release_driver+0x66/0x70
Apr 25 16:18:41 core kernel:  [<c0210d03>] bus_remove_device+0x53/0xa0
Apr 25 16:18:41 core kernel:  [<c020fbdd>] device_del+0x5d/0xa0
Apr 25 16:18:41 core kernel:  [<c020fc34>] device_unregister+0x14/0x30
Apr 25 16:18:41 core kernel:  [<e185f480>] usb_disable_device+0x70/0xb0 [usbcore]
Apr 25 16:18:41 core kernel:  [<e1859d16>] usb_disconnect+0xa6/0x100 [usbcore]
Apr 25 16:18:41 core kernel:  [<e185bfef>] hub_port_connect_change+0x28f/0x2a0 [usbcore]
Apr 25 16:18:41 core kernel:  [<e185b971>] hub_port_status+0x41/0xb0 [usbcore]
Apr 25 16:18:41 core kernel:  [<e185c343>] hub_events+0x343/0x3b0 [usbcore]
Apr 25 16:18:41 core kernel:  [<e185c3e5>] hub_thread+0x35/0xf0 [usbcore]
Apr 25 16:18:41 core kernel:  [<c0115ef0>] default_wake_function+0x0/0x20
Apr 25 16:18:41 core kernel:  [<e185c3b0>] hub_thread+0x0/0xf0 [usbcore]
Apr 25 16:18:41 core kernel:  [<c01032e5>] kernel_thread_helper+0x5/0x10
Apr 25 16:18:41 core kernel:
Apr 25 16:18:41 core kernel: Code: 89 0c 85 a0 8f 9a e1 c7 44 24 04 7c 8c 9a e1 8b 43 14 8b 80
 
-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Friday 23 April 2004 03:39 pm, Russell King wrote:
> On Fri, Apr 23, 2004 at 10:14:24PM +0200, Marcel Holtmann wrote:
> > should we apply the pcmcia_get_sys_device() patch from Dmitry for now to
> > fix the current drivers that need a device for loading the firmware?
> 
> I don't think so - it obtains the struct device for the bridge itself
> which has nothing to do with the card inserted in the slot.
> 

Yes, my bad... I wonder if something like the patch below could be useful
for now (although it created only one device entry even if card has multiple
functions so we really need another device for every function):

===== ss.h 1.27 vs edited =====
--- 1.27/include/pcmcia/ss.h	Sat Sep  6 16:32:55 2003
+++ edited/ss.h	Sun Apr 25 15:10:40 2004
@@ -196,7 +196,6 @@
  	/* deprecated */
 	unsigned int			sock;		/* socket number */
 
-
 	/* socket capabilities */
 	u_int				features;
 	u_int				irq_mask;
@@ -232,6 +231,7 @@
 
 	/* socket device */
 	struct class_device		dev;
+	struct device			*card_dev;
 	void				*driver_data;	/* data internal to the socket driver */
 
 };
===== cs.c 1.78 vs edited =====
--- 1.78/drivers/pcmcia/cs.c	Mon Apr 19 03:12:13 2004
+++ edited/cs.c	Sun Apr 25 15:14:36 2004
@@ -60,6 +60,7 @@
 #include <pcmcia/bulkmem.h>
 #include <pcmcia/cistpl.h>
 #include <pcmcia/cisreg.h>
+#include <pcmcia/ds.h>
 #include "cs_internal.h"
 
 #ifdef CONFIG_PCI
@@ -324,6 +325,40 @@
 }
 EXPORT_SYMBOL(pcmcia_get_socket_by_nr);
 
+static void release_card_device(struct device *dev)
+{
+	kfree(dev);
+}
+
+static void register_card_device(struct pcmcia_socket *skt)
+{
+	struct device *dev;
+
+	WARN_ON(skt->card_dev);
+
+	skt->card_dev = dev = kmalloc(sizeof(struct device), GFP_KERNEL);
+	if (dev) {
+
+		memset(dev, 0, sizeof(struct device));
+
+		dev->parent = skt->dev.dev;
+		dev->driver = NULL;
+		dev->bus = &pcmcia_bus_type;
+		dev->release = &release_card_device;
+
+		snprintf(dev->bus_id, sizeof(dev->bus_id), "pccard%d", skt->sock);
+
+		device_register(dev);
+	}
+}
+
+static void unregister_card_device(struct pcmcia_socket *skt)
+{
+	if (skt->card_dev) {
+		device_unregister(skt->card_dev);
+		skt->card_dev = NULL;
+	}
+}
 
 /*======================================================================
 
@@ -574,8 +609,11 @@
 #endif
 		cs_dbg(skt, 4, "insert done\n");
 
+		register_card_device(skt);
+
 		send_event(skt, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
 	} else {
+		unregister_card_device(skt);
 		socket_shutdown(skt);
 		cs_socket_put(skt);
 	}
@@ -596,6 +634,7 @@
 	return CS_SUCCESS;
 }
 
+
 /*
  * Resume a socket.  If a card is present, verify its CIS against
  * our cached copy.  If they are different, the card has been
@@ -620,11 +659,14 @@
 		if (verify_cis_cache(skt) != 0) {
 			socket_remove_drivers(skt);
 			destroy_cis_cache(skt);
+			unregister_card_device(skt);
+			register_card_device(skt);
 			send_event(skt, CS_EVENT_CARD_INSERTION, CS_EVENT_PRI_LOW);
 		} else {
 			send_event(skt, CS_EVENT_PM_RESUME, CS_EVENT_PRI_LOW);
 		}
 	} else {
+		unregister_card_device(skt);
 		socket_shutdown(skt);
 		cs_socket_put(skt);
 	}
@@ -636,6 +678,7 @@
 
 static void socket_remove(struct pcmcia_socket *skt)
 {
+	unregister_card_device(skt);
 	socket_shutdown(skt);
 	cs_socket_put(skt);
 }

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Russell King]

On Sun, Apr 25, 2004 at 04:53:53PM -0500, Dmitry Torokhov wrote:
> On Friday 23 April 2004 03:39 pm, Russell King wrote:
> > On Fri, Apr 23, 2004 at 10:14:24PM +0200, Marcel Holtmann wrote:
> > > should we apply the pcmcia_get_sys_device() patch from Dmitry for now to
> > > fix the current drivers that need a device for loading the firmware?
> > 
> > I don't think so - it obtains the struct device for the bridge itself
> > which has nothing to do with the card inserted in the slot.
> > 
> 
> Yes, my bad... I wonder if something like the patch below could be useful
> for now (although it created only one device entry even if card has multiple
> functions so we really need another device for every function):

This breaks modular builds - pcmcia_bus_type is in ds.c which is a
separate module.

Look, Dominik has done a fair amount of work in this area.  There is
a set of patches which need to be worked through and merged in a
controlled manner to get to the point where we can have a struct
device for PCMCIA cards.  We'll get there eventually.  Please don't
try to bypass this process - it won't work, and it'll only cause
unnecessary merge problems with the existing patch sets.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Vojtech Pavlik]

On Fri, Apr 23, 2004 at 11:03:42AM -0700, Greg KH wrote:
> On Fri, Apr 23, 2004 at 10:19:53AM -0700, Greg KH wrote:
> > On Fri, Apr 23, 2004 at 08:31:11AM -0700, Greg KH wrote:
> > > 
> > > No, we need to oops, as that's a real bug.  Can you post the whole oops
> > > that was generated with this usb problem?  I can't seem to duplicate
> > > this here.
> > 
> > Nevermind I dug up a device here that causes this problem.  I'll track
> > it down...
> 
> Ok, here's a patch that fixes it for me.  I was waiting for a good
> reason to finally get rid of this fake usb_interface structure, and now
> I have it :)
> 
> Let me know if it solves the problem for you too and then I'll send it
> off to Linus.
> 
> Any objections Vojtech?

None. :)

> 
> thanks,
> 
> greg k-h
> 
> 
> # USB: fix up fake usb_interface structure in hiddev
> #
> # This fixes a oops in the current kernel tree.
> 
> diff -Nru a/drivers/usb/input/hiddev.c b/drivers/usb/input/hiddev.c
> --- a/drivers/usb/input/hiddev.c	Fri Apr 23 11:00:23 2004
> +++ b/drivers/usb/input/hiddev.c	Fri Apr 23 11:00:23 2004
> @@ -53,7 +53,6 @@
>  	wait_queue_head_t wait;
>  	struct hid_device *hid;
>  	struct hiddev_list *list;
> -	struct usb_interface intf;
>  };
>  
>  struct hiddev_list {
> @@ -234,7 +233,7 @@
>  static struct usb_class_driver hiddev_class;
>  static void hiddev_cleanup(struct hiddev *hiddev)
>  {
> -	usb_deregister_dev(&hiddev->intf, &hiddev_class);
> +	usb_deregister_dev(hiddev->hid->intf, &hiddev_class);
>  	hiddev_table[hiddev->minor] = NULL;
>  	kfree(hiddev);
>  }
> @@ -775,7 +774,7 @@
>  		return -1;
>  	memset(hiddev, 0, sizeof(struct hiddev));
>  
> - 	retval = usb_register_dev(&hiddev->intf, &hiddev_class);
> + 	retval = usb_register_dev(hid->intf, &hiddev_class);
>  	if (retval) {
>  		err("Not able to get a minor for this device.");
>  		kfree(hiddev);
> @@ -784,13 +783,13 @@
>  
>  	init_waitqueue_head(&hiddev->wait);
>  
> - 	hiddev->minor = hiddev->intf.minor;
> - 	hiddev_table[hiddev->intf.minor - HIDDEV_MINOR_BASE] = hiddev;
> + 	hiddev->minor = hid->intf->minor;
> + 	hiddev_table[hid->intf->minor - HIDDEV_MINOR_BASE] = hiddev;
>  
>  	hiddev->hid = hid;
>  	hiddev->exist = 1;
>  
> - 	hid->minor = hiddev->intf.minor;
> + 	hid->minor = hid->intf->minor;
>  	hid->hiddev = hiddev;
>  
>  	return 0;
> 

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Russell,

> Look, Dominik has done a fair amount of work in this area.  There is
> a set of patches which need to be worked through and merged in a
> controlled manner to get to the point where we can have a struct
> device for PCMCIA cards.  We'll get there eventually.  Please don't
> try to bypass this process - it won't work, and it'll only cause
> unnecessary merge problems with the existing patch sets.

right now we have two broken drivers. They are only broken, because we
need a device for loading the firmware. If the PCMCIA driver model
integrations is not yet ready, we should find a way to make the firmware
loading possible without having a device. We don't need the device for
any other task. Actually I don't know how to achieve it, but I think if
we give a NULL pointer to the request_firmware() call the firmware_class
should create a dummy device.

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Sunday 25 April 2004 05:58 pm, Russell King wrote:
> On Sun, Apr 25, 2004 at 04:53:53PM -0500, Dmitry Torokhov wrote:
> > On Friday 23 April 2004 03:39 pm, Russell King wrote:
> > > On Fri, Apr 23, 2004 at 10:14:24PM +0200, Marcel Holtmann wrote:
> > > > should we apply the pcmcia_get_sys_device() patch from Dmitry for now to
> > > > fix the current drivers that need a device for loading the firmware?
> > > 
> > > I don't think so - it obtains the struct device for the bridge itself
> > > which has nothing to do with the card inserted in the slot.
> > > 
> > 
> > Yes, my bad... I wonder if something like the patch below could be useful
> > for now (although it created only one device entry even if card has multiple
> > functions so we really need another device for every function):
> 
> This breaks modular builds - pcmcia_bus_type is in ds.c which is a
> separate module.
>

Ok, then one question before I shut up - why is it exported if other modules
can not use it?

-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Monday 26 April 2004 05:35 am, Marcel Holtmann wrote:
> Hi Russell,
> 
> > Look, Dominik has done a fair amount of work in this area.  There is
> > a set of patches which need to be worked through and merged in a
> > controlled manner to get to the point where we can have a struct
> > device for PCMCIA cards.  We'll get there eventually.  Please don't
> > try to bypass this process - it won't work, and it'll only cause
> > unnecessary merge problems with the existing patch sets.
> 
> right now we have two broken drivers. They are only broken, because we
> need a device for loading the firmware. If the PCMCIA driver model
> integrations is not yet ready, we should find a way to make the firmware
> loading possible without having a device. We don't need the device for
> any other task. Actually I don't know how to achieve it, but I think if
> we give a NULL pointer to the request_firmware() call the firmware_class
> should create a dummy device.
>
Hi Marcel,

This is wrong... What if you have several devices that are needed firmware?
You are not only loading a specific firmware but do it for a specific device.
You may also want to do something else with it...

I think until pcmcia either provides or allows to create devices on pcmcia
bus you can just fixup the name breakage like I did for atmel driver and
leave it be. The device is not registered and not used in any way except to
provide "unique" name for fimrware loader, at list in atmel_cs that is the
case.

-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Marcel Holtmann]

Hi Dmitry,

> This is wrong... What if you have several devices that are needed firmware?
> You are not only loading a specific firmware but do it for a specific device.
> You may also want to do something else with it...
> 
> I think until pcmcia either provides or allows to create devices on pcmcia
> bus you can just fixup the name breakage like I did for atmel driver and
> leave it be. The device is not registered and not used in any way except to
> provide "unique" name for fimrware loader, at list in atmel_cs that is the
> case.

as you said the only real goal of the device in request_firmware() is to
provide us with a unique path to the "loading" and "data" files. The
advantage of having the device linked is currently not used by the
firmware.agent script and I don't think that it ever will be used.
However you can take a look at 2.4, where the device parameter is only a
string. So what is wrong with letting request_firmware() create a dummy
and temporary device that is not related to any hardware?

Regards

Marcel

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Monday 26 April 2004 08:09 am, Marcel Holtmann wrote:
> Hi Dmitry,
> 
> > This is wrong... What if you have several devices that are needed firmware?
> > You are not only loading a specific firmware but do it for a specific device.
> > You may also want to do something else with it...
> > 
> > I think until pcmcia either provides or allows to create devices on pcmcia
> > bus you can just fixup the name breakage like I did for atmel driver and
> > leave it be. The device is not registered and not used in any way except to
> > provide "unique" name for fimrware loader, at list in atmel_cs that is the
> > case.
> 
> as you said the only real goal of the device in request_firmware() is to
> provide us with a unique path to the "loading" and "data" files. The
> advantage of having the device linked is currently not used by the
> firmware.agent script and I don't think that it ever will be used.
> However you can take a look at 2.4, where the device parameter is only a
> string. So what is wrong with letting request_firmware() create a dummy
> and temporary device that is not related to any hardware?
>

Several drivers already use the firmware loading facility and I expect seeing
more of them in the future. So it is possible to have 2 seperate devices
request firmware at the same time. If we just provide single dummy device and
use it they will clash. Generating unique ID on every request with empty
device is a hack. I do not think that interface needs this kind of a hack, it
is better to keep them in the drivers that do not properly register themselves
with the driver model yet.

-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Sun, Apr 25, 2004 at 04:48:07PM -0500, Dmitry Torokhov wrote:
>  
> No, I am still getting the oops.. hmm.. it seems a little bit different,
> but still in the hiddev. I investigated further and the oops only happens
> if I yank a HID device connected to an USB hub or if I yank entire hub with
> a HID device connected to it. Tried with APC UPS and MS Intellimouse
> Explorer. If they are connected directly to the laptop's ports everything is
> fine, also other devices (USB printer for example) handle hub disconnection
> just fine. It does not matter if I have device open or closed for oops to
> happen. And, for the record, oops itself:

Are you still getting this in the 2.6.6-rc3 kernel?

How about the latest -mm release?

thanks,

greg k-h

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Dmitry Torokhov]

On Tuesday 04 May 2004 04:04 pm, Greg KH wrote:
> On Sun, Apr 25, 2004 at 04:48:07PM -0500, Dmitry Torokhov wrote:
> >  
> > No, I am still getting the oops.. hmm.. it seems a little bit different,
> > but still in the hiddev. I investigated further and the oops only happens
> > if I yank a HID device connected to an USB hub or if I yank entire hub with
> > a HID device connected to it. Tried with APC UPS and MS Intellimouse
> > Explorer. If they are connected directly to the laptop's ports everything is
> > fine, also other devices (USB printer for example) handle hub disconnection
> > just fine. It does not matter if I have device open or closed for oops to
> > happen. And, for the record, oops itself:
> 
> Are you still getting this in the 2.6.6-rc3 kernel?
> 
> How about the latest -mm release?
> 

With tonight's bk pull + USB patch from 2.6.6-rc3-mm1 I am still getting the
following oops when pulling a HID device out of a hub:

usb 1-1: USB disconnect, address 2
usb 1-2: USB disconnect, address 3
usb 1-2.3: USB disconnect, address 4
Unable to handle kernel paging request at virtual address e19f6140
 printing eip:
e19f1037
*pde = 1fe1e067
*pte = 00000000
Oops: 0002 [#1]
PREEMPT 
CPU:    0
EIP:    0060:[<e19f1037>]    Not tainted
EFLAGS: 00010246   (2.6.6-rc3) 
EIP is at hiddev_cleanup+0x17/0x50 [usbhid]
eax: 00000060   ebx: d8de4f9c   ecx: 00000000   edx: d7d6cb10
esi: e19f5ba0   edi: df905400   ebp: df69be54   esp: df69be48
ds: 007b   es: 007b   ss: 0068
Process khubd (pid: 532, threadinfo=df69a000 task=dfa35730)
Stack: df69be54 e185db5f d8c30000 df69be68 e19f0ac8 d8de4f9c e19f5ba0 dfd718b4 
       df69be84 e1858106 dfd718b4 dfd718b4 d7e64660 dfd718c4 e19f5bc0 df69be9c 
       c0210a66 dfd718c4 dfd718ec dfd718c4 df9054cc df69beb4 c0210ba3 dfd718c4 
Call Trace:
 [<e185db5f>] usb_unlink_urb+0x3f/0x50 [usbcore]
 [<e19f0ac8>] hid_disconnect+0xb8/0xe0 [usbhid]
 [<e1858106>] usb_unbind_interface+0x76/0x80 [usbcore]
 [<c0210a66>] device_release_driver+0x66/0x70
 [<c0210ba3>] bus_remove_device+0x53/0xa0
 [<c020fa7d>] device_del+0x5d/0xa0
 [<c020fad4>] device_unregister+0x14/0x30
 [<e185e9df>] usb_disable_device+0x6f/0xc0 [usbcore]
 [<e1858d16>] usb_disconnect+0xa6/0x100 [usbcore]
 [<e1858d58>] usb_disconnect+0xe8/0x100 [usbcore]
 [<e185b3e7>] hub_port_connect_change+0x2a7/0x2e0 [usbcore]
 [<e185b763>] hub_events+0x343/0x3b0 [usbcore]
 [<e185b805>] hub_thread+0x35/0xf0 [usbcore]
 [<c0115ef0>] default_wake_function+0x0/0x20
 [<e185b7d0>] hub_thread+0x0/0xf0 [usbcore]
 [<c01032e5>] kernel_thread_helper+0x5/0x10

Code: 89 0c 85 c0 5f 9f e1 c7 44 24 04 9c 5c 9f e1 8b 43 10 8b 80 


If HID device connected directly to laptop's USB port I can yank it without
any trouble.

-- 
Dmitry

Re: [OOPS/HACK] atmel_cs and the latest changes in sysfs/symlink.c

[Greg KH]

On Wed, May 05, 2004 at 02:08:11AM -0500, Dmitry Torokhov wrote:
> On Tuesday 04 May 2004 04:04 pm, Greg KH wrote:
> > On Sun, Apr 25, 2004 at 04:48:07PM -0500, Dmitry Torokhov wrote:
> > >  
> > > No, I am still getting the oops.. hmm.. it seems a little bit different,
> > > but still in the hiddev. I investigated further and the oops only happens
> > > if I yank a HID device connected to an USB hub or if I yank entire hub with
> > > a HID device connected to it. Tried with APC UPS and MS Intellimouse
> > > Explorer. If they are connected directly to the laptop's ports everything is
> > > fine, also other devices (USB printer for example) handle hub disconnection
> > > just fine. It does not matter if I have device open or closed for oops to
> > > happen. And, for the record, oops itself:
> > 
> > Are you still getting this in the 2.6.6-rc3 kernel?
> > 
> > How about the latest -mm release?
> > 
> 
> With tonight's bk pull + USB patch from 2.6.6-rc3-mm1 I am still getting the
> following oops when pulling a HID device out of a hub:

Ick, I'll work on trying to duplicate this and see if I can figure it
out...

thanks for letting me know.

greg k-h