Skip to contentSkip to conversation

Remove duplicate ID in ipaq driver

PATCH In review Ben Collins 5 participants 8 messages May 16, 2007 – May 17, 2007

Subsystems (2)
USB USB serial
lore
Share & tools
lore

Ben Collins

author
#0 lore raw

PATCH 1/1 Remove duplicate ID in ipaq driver

Cc Ganesh Varadarajan <[email protected]>
Cc [email protected]
Signed-off-by Ben Collins <[email protected]>
drivers/usb/serial/ipaq.c +0 -1
index 4df0ec7..7c85be4 100644
@@ -92,7 +92,6 @@ static void ipaq_destroy_lists(struct usb_serial_port *port);
 
 static struct usb_device_id ipaq_id_table [] = {
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },
 	{ USB_DEVICE(0x0104, 0x00BE) }, /* Socket USB Sync */
 	{ USB_DEVICE(0x03F0, 0x1016) }, /* HP USB Sync */
 	{ USB_DEVICE(0x03F0, 0x1116) }, /* HP USB Sync 1611 */
Signature · 2 lines
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/
#1 lore raw

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 95

Quoted 2 lines
drivers/usb/serial/ipaq.c +0 -1
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },

Is it obvious why this patch is correct? Especially given the comment just before the line you delete, and the code

	if (vendor) {
		ipaq_id_table[0].idVendor = vendor;
		ipaq_id_table[0].idProduct = product;
	}

in ipaq_init()?

- R.

author
#2 lore raw
Quoted 6 lines · Roland Dreier

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 95

Quoted 2 lines
drivers/usb/serial/ipaq.c +0 -1
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },

Is it obvious why this patch is correct? Especially given the comment just before the line you delete, and the code

Quoted 6 lines
	if (vendor) {
		ipaq_id_table[0].idVendor = vendor;
		ipaq_id_table[0].idProduct = product;
	}

in ipaq_init()?

My mistake, quick on the patching going through this dupe list.

Might I add that this is terrible use of the device table, though. Clutters userspace, and adds processing to module-init-tools programs.

Signature · 2 lines
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/
#3 lore raw
Quoted 12 lines · Ben Collins
Quoted 11 lines · Roland Dreier

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 95

Quoted 2 lines
drivers/usb/serial/ipaq.c +0 -1
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },

Is it obvious why this patch is correct? Especially given the comment just before the line you delete, and the code

	if (vendor) {
		ipaq_id_table[0].idVendor = vendor;
		ipaq_id_table[0].idProduct = product;
	}
Quoted 6 lines
Quoted 1 line

in ipaq_init()?

My mistake, quick on the patching going through this dupe list.

Might I add that this is terrible use of the device table, though. Clutters userspace, and adds processing to module-init-tools programs.

It's a hold-over from the times when we didn't have the sysfs "add a new id" interface for usb-serial drivers, which only recently was created.

So we just have to live with it, and the infinitesimal speed hit it creates :)

thanks,

greg k-h

author
#4 lore raw
Quoted 17 lines · Greg KH
Quoted 16 lines · Ben Collins
Quoted 6 lines · Roland Dreier

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 95

Quoted 2 lines
drivers/usb/serial/ipaq.c +0 -1
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },

Is it obvious why this patch is correct? Especially given the comment just before the line you delete, and the code

Quoted 6 lines
	if (vendor) {
		ipaq_id_table[0].idVendor = vendor;
		ipaq_id_table[0].idProduct = product;
	}

in ipaq_init()?

My mistake, quick on the patching going through this dupe list.

Quoted 8 lines
Quoted 2 lines

Might I add that this is terrible use of the device table, though. Clutters userspace, and adds processing to module-init-tools programs.

It's a hold-over from the times when we didn't have the sysfs "add a new id" interface for usb-serial drivers, which only recently was created.

So we just have to live with it, and the infinitesimal speed hit it creates :)

Any objection to adding it to planned-for-removal and spitting out a printk when someone uses the "feature"?

Signature · 2 lines
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/
#5 lore raw
Quoted 5 lines · Ben Collins
Quoted 2 lines

So we just have to live with it, and the infinitesimal speed hit it creates :)

Any objection to adding it to planned-for-removal and spitting out a printk when someone uses the "feature"?

Do we have any good reason to believe that this will eventually lead to a clean-up? ISTR that we did a similar printk for sysctl (or as Dr. Phil likes to say, "How's that working for you?")

#6 lore raw
In reply to Ben Collins
Quoted 21 lines · Ben Collins
Quoted 20 lines · Greg KH
Quoted 18 lines · Ben Collins
Quoted 6 lines · Roland Dreier

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 95

Quoted 2 lines
drivers/usb/serial/ipaq.c +0 -1
 	/* The first entry is a placeholder for the insmod-specified device */
-	{ USB_DEVICE(0x049F, 0x0003) },

Is it obvious why this patch is correct? Especially given the comment just before the line you delete, and the code

Quoted 6 lines
	if (vendor) {
		ipaq_id_table[0].idVendor = vendor;
		ipaq_id_table[0].idProduct = product;
	}

in ipaq_init()?

My mistake, quick on the patching going through this dupe list.

Might I add that this is terrible use of the device table, though. Clutters userspace, and adds processing to module-init-tools programs.

Quoted 8 lines
Quoted 5 lines

It's a hold-over from the times when we didn't have the sysfs "add a new id" interface for usb-serial drivers, which only recently was created.

So we just have to live with it, and the infinitesimal speed hit it creates :)

Any objection to adding it to planned-for-removal and spitting out a printk when someone uses the "feature"?

No, it's a module parameter and it's much easier to use for some systems than the sysfs file way. It's just not worth breaking userspace APIs for no good reason.

thanks,

greg k-h

#7 lore raw
In reply to Ben Collins

Ben Collins napsal(a):

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 94

Quoted 12 lines
Cc Ganesh Varadarajan <[email protected]>
Cc [email protected]
Signed-off-by Ben Collins <[email protected]>
drivers/usb/serial/ipaq.c +0 -0
index 4df0ec7..7c85be4 100644
@@ -92,7 +92,6 @@ static void ipaq_destroy_lists(struct usb_serial_port *port);
 
 static struct usb_device_id ipaq_id_table [] = {
 	/* The first entry is a placeholder for the insmod-specified device */

I wonder if the people ever read the comments? ^^^^^^^^^
It's used for parameter from command line when modprobing. See ipaq_init.

drivers/usb/serial/ipaq.c static void ipaq_destroy_lists(struct usb_serial_port *port); · line 97

Quoted 5 lines
drivers/usb/serial/ipaq.c +0 -1
-	{ USB_DEVICE(0x049F, 0x0003) },
 	{ USB_DEVICE(0x0104, 0x00BE) }, /* Socket USB Sync */
 	{ USB_DEVICE(0x03F0, 0x1016) }, /* HP USB Sync */
 	{ USB_DEVICE(0x03F0, 0x1116) }, /* HP USB Sync 1611 */

regards,

Signature · 4 lines
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E