[PATCH] Remove duplicate ID in ipaq driver

[PATCH] Remove duplicate ID in ipaq driver

[Ben Collins]

Cc: Ganesh Varadarajan <ganesh@veritas.com>
Cc: linux-usb-devel@lists.sourceforge.net
Signed-off-by: Ben Collins <bcollins@ubuntu.com>

diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
index 4df0ec7..7c85be4 100644
--- a/drivers/usb/serial/ipaq.c
+++ b/drivers/usb/serial/ipaq.c
@@ -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 */

-- 
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/

Re: [PATCH] Remove duplicate ID in ipaq driver

[Roland Dreier]

 >  	/* 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.

Re: [PATCH] Remove duplicate ID in ipaq driver

[Ben Collins]

On Wed, 2007-05-16 at 13:59 -0700, Roland Dreier wrote:
> >  	/* 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()?

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.

-- 
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/

Re: [linux-usb-devel] [PATCH] Remove duplicate ID in ipaq driver

[Greg KH]

On Wed, May 16, 2007 at 05:36:48PM -0400, Ben Collins wrote:
> On Wed, 2007-05-16 at 13:59 -0700, Roland Dreier wrote:
> > >  	/* 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()?
> 
> 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

Re: [linux-usb-devel] [PATCH] Remove duplicate ID in ipaq driver

[Ben Collins]

On Thu, 2007-05-17 at 05:43 -0700, Greg KH wrote:
> On Wed, May 16, 2007 at 05:36:48PM -0400, Ben Collins wrote:
> > On Wed, 2007-05-16 at 13:59 -0700, Roland Dreier wrote:
> > > >  	/* 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()?
> > 
> > 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 :)

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

-- 
Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/

Re: [linux-usb-devel] [PATCH] Remove duplicate ID in ipaq driver

[Valdis.Kletnieks]

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

On Thu, 17 May 2007 09:02:20 EDT, Ben Collins said:
> > 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?")


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [linux-usb-devel] [PATCH] Remove duplicate ID in ipaq driver

[Greg KH]

On Thu, May 17, 2007 at 09:02:20AM -0400, Ben Collins wrote:
> On Thu, 2007-05-17 at 05:43 -0700, Greg KH wrote:
> > On Wed, May 16, 2007 at 05:36:48PM -0400, Ben Collins wrote:
> > > On Wed, 2007-05-16 at 13:59 -0700, Roland Dreier wrote:
> > > > >  	/* 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()?
> > > 
> > > 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 :)
> 
> 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

Re: [PATCH] Remove duplicate ID in ipaq driver

[Jiri Slaby]

Ben Collins napsal(a):
> Cc: Ganesh Varadarajan <ganesh@veritas.com>
> Cc: linux-usb-devel@lists.sourceforge.net
> Signed-off-by: Ben Collins <bcollins@ubuntu.com>
> 
> diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
> index 4df0ec7..7c85be4 100644
> --- a/drivers/usb/serial/ipaq.c
> +++ b/drivers/usb/serial/ipaq.c
> @@ -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.

> -	{ 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,
-- 
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