2010-06-10 22:28:41

by Leann Ogasawara

[permalink] [raw]
Subject: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

The Medion MD40900 device id [0x0cde, 0x0006] is defined twice. Remove
the duplicate.

Originally-by: Ben Collins <[email protected]>
Signed-off-by: Leann Ogasawara <[email protected]>
---
drivers/net/wireless/p54/p54usb.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 7307325..d6d8713 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -69,7 +69,6 @@ static struct usb_device_id p54u_table[] __devinitdata = {
{USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
{USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
{USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
- {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
{USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
{USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
{USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */
--
1.7.0.4





2010-06-15 17:59:17

by Leann Ogasawara

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On Mon, 2010-06-14 at 14:55 -0400, John W. Linville wrote:
> On Thu, Jun 10, 2010 at 06:23:19PM -0500, Larry Finger wrote:
> > On 06/10/2010 05:28 PM, Leann Ogasawara wrote:
> > > The Medion MD40900 device id [0x0cde, 0x0006] is defined twice. Remove
> > > the duplicate.
> > >
> > > Originally-by: Ben Collins <[email protected]>
> > > Signed-off-by: Leann Ogasawara <[email protected]>
> > > ---
> > > drivers/net/wireless/p54/p54usb.c | 1 -
> > > 1 files changed, 0 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
> > > index 7307325..d6d8713 100644
> > > --- a/drivers/net/wireless/p54/p54usb.c
> > > +++ b/drivers/net/wireless/p54/p54usb.c
> > > @@ -69,7 +69,6 @@ static struct usb_device_id p54u_table[] __devinitdata = {
> > > {USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
> > > {USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
> > > {USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
> > > - {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
> > > {USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
> > > {USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
> > > {USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */
> >
> > Do you know for certain that this device is a Version 1 chip, and for
> > that reason, you are removing it from the version 2 list?
> >
> > If not, NACK.
>
> So, I guess you are concerned about the groupings because of the
> different firmwares or something like that? Perhaps a comment that
> says "this could be a version 2 device" is just as handy? Since the
> driver prints the name of the firmware it wants, is there any real
> need for grouping the IDs?
>
> OTOH, is there any actual harm from the duplicate entry? It "seems"
> wrong to me too, but I guess it does no harm...?

I don't believe there is any harm from the duplicate entry, it just
seemed unnecessary.

> Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
> an actual bug involved?

Indeed, this was just a patch we'd been carrying to tidy things up.
There was no actual bug involved. I'd be happy to send a v2 of the
patch which comments out the duplicate entry and adds a note as to why.
Or I'd be fine just leaving the code as is and we'll drop the patch
we're carrying locally in Ubuntu.

Thanks,
Leann


2010-06-10 23:22:52

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On 06/10/2010 05:28 PM, Leann Ogasawara wrote:
> The Medion MD40900 device id [0x0cde, 0x0006] is defined twice. Remove
> the duplicate.
>
> Originally-by: Ben Collins <[email protected]>
> Signed-off-by: Leann Ogasawara <[email protected]>
> ---
> drivers/net/wireless/p54/p54usb.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
> index 7307325..d6d8713 100644
> --- a/drivers/net/wireless/p54/p54usb.c
> +++ b/drivers/net/wireless/p54/p54usb.c
> @@ -69,7 +69,6 @@ static struct usb_device_id p54u_table[] __devinitdata = {
> {USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
> {USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
> {USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
> - {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
> {USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
> {USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
> {USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */

Do you know for certain that this device is a Version 1 chip, and for
that reason, you are removing it from the version 2 list?

If not, NACK.

Larry


2010-06-15 20:00:13

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On Tue, Jun 15, 2010 at 10:59:05AM -0700, Leann Ogasawara wrote:
> On Mon, 2010-06-14 at 14:55 -0400, John W. Linville wrote:

> > So, I guess you are concerned about the groupings because of the
> > different firmwares or something like that? Perhaps a comment that
> > says "this could be a version 2 device" is just as handy? Since the
> > driver prints the name of the firmware it wants, is there any real
> > need for grouping the IDs?
> >
> > OTOH, is there any actual harm from the duplicate entry? It "seems"
> > wrong to me too, but I guess it does no harm...?
>
> I don't believe there is any harm from the duplicate entry, it just
> seemed unnecessary.
>
> > Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
> > an actual bug involved?
>
> Indeed, this was just a patch we'd been carrying to tidy things up.
> There was no actual bug involved. I'd be happy to send a v2 of the
> patch which comments out the duplicate entry and adds a note as to why.
> Or I'd be fine just leaving the code as is and we'll drop the patch
> we're carrying locally in Ubuntu.

FWIW, I like the 'comment-out and add a note' option.

Thanks!

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-06-14 19:00:10

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On Thu, Jun 10, 2010 at 06:23:19PM -0500, Larry Finger wrote:
> On 06/10/2010 05:28 PM, Leann Ogasawara wrote:
> > The Medion MD40900 device id [0x0cde, 0x0006] is defined twice. Remove
> > the duplicate.
> >
> > Originally-by: Ben Collins <[email protected]>
> > Signed-off-by: Leann Ogasawara <[email protected]>
> > ---
> > drivers/net/wireless/p54/p54usb.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
> > index 7307325..d6d8713 100644
> > --- a/drivers/net/wireless/p54/p54usb.c
> > +++ b/drivers/net/wireless/p54/p54usb.c
> > @@ -69,7 +69,6 @@ static struct usb_device_id p54u_table[] __devinitdata = {
> > {USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
> > {USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
> > {USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
> > - {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
> > {USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
> > {USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
> > {USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */
>
> Do you know for certain that this device is a Version 1 chip, and for
> that reason, you are removing it from the version 2 list?
>
> If not, NACK.

So, I guess you are concerned about the groupings because of the
different firmwares or something like that? Perhaps a comment that
says "this could be a version 2 device" is just as handy? Since the
driver prints the name of the firmware it wants, is there any real
need for grouping the IDs?

OTOH, is there any actual harm from the duplicate entry? It "seems"
wrong to me too, but I guess it does no harm...?

Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
an actual bug involved?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-06-14 19:16:46

by Ben Collins

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On Mon, 2010-06-14 at 14:55 -0400, John W. Linville wrote:
> On Thu, Jun 10, 2010 at 06:23:19PM -0500, Larry Finger wrote:
> > On 06/10/2010 05:28 PM, Leann Ogasawara wrote:
> > > The Medion MD40900 device id [0x0cde, 0x0006] is defined twice. Remove
> > > the duplicate.
> > >
> > > Originally-by: Ben Collins <[email protected]>
> > > Signed-off-by: Leann Ogasawara <[email protected]>
> > > ---
> > > drivers/net/wireless/p54/p54usb.c | 1 -
> > > 1 files changed, 0 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
> > > index 7307325..d6d8713 100644
> > > --- a/drivers/net/wireless/p54/p54usb.c
> > > +++ b/drivers/net/wireless/p54/p54usb.c
> > > @@ -69,7 +69,6 @@ static struct usb_device_id p54u_table[] __devinitdata = {
> > > {USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
> > > {USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
> > > {USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
> > > - {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
> > > {USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
> > > {USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
> > > {USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */
> >
> > Do you know for certain that this device is a Version 1 chip, and for
> > that reason, you are removing it from the version 2 list?
> >
> > If not, NACK.
>
> So, I guess you are concerned about the groupings because of the
> different firmwares or something like that? Perhaps a comment that
> says "this could be a version 2 device" is just as handy? Since the
> driver prints the name of the firmware it wants, is there any real
> need for grouping the IDs?
>
> OTOH, is there any actual harm from the duplicate entry? It "seems"
> wrong to me too, but I guess it does no harm...?
>
> Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
> an actual bug involved?

It was flagged by a script I wrote to detect overlapping drivers. This
driver had dual entries for the same device, so the second was basically
giving depmod another entry to worry about. Trivial, yes, but just
seemed cleaner to not do that in general.

I think we had decided awhile back to just comment out this entry with a
note that says something like "already above, just listing for clarity"
or similar.

--
Bluecherry: http://www.bluecherrydvr.com/
SwissDisk : http://www.swissdisk.com/
Ubuntu : http://www.ubuntu.com/
My Blog : http://ben-collins.blogspot.com/


2010-06-15 23:39:15

by Leann Ogasawara

[permalink] [raw]
Subject: [PATCH v2] p54usb: Comment out duplicate Medion MD40900 device id

On Tue, 2010-06-15 at 15:46 -0400, John W. Linville wrote:
> On Tue, Jun 15, 2010 at 10:59:05AM -0700, Leann Ogasawara wrote:
> > On Mon, 2010-06-14 at 14:55 -0400, John W. Linville wrote:
>
> > > So, I guess you are concerned about the groupings because of the
> > > different firmwares or something like that? Perhaps a comment that
> > > says "this could be a version 2 device" is just as handy? Since the
> > > driver prints the name of the firmware it wants, is there any real
> > > need for grouping the IDs?
> > >
> > > OTOH, is there any actual harm from the duplicate entry? It "seems"
> > > wrong to me too, but I guess it does no harm...?
> >
> > I don't believe there is any harm from the duplicate entry, it just
> > seemed unnecessary.
> >
> > > Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
> > > an actual bug involved?
> >
> > Indeed, this was just a patch we'd been carrying to tidy things up.
> > There was no actual bug involved. I'd be happy to send a v2 of the
> > patch which comments out the duplicate entry and adds a note as to why.
> > Or I'd be fine just leaving the code as is and we'll drop the patch
> > we're carrying locally in Ubuntu.
>
> FWIW, I like the 'comment-out and add a note' option.

Here's the 'comment-out and add a note' option. Let me know if you'd
prefer a different style formatting etc.

Thanks,
Leann

>From 5d1b6029ffc0eb770cd34f7dd1a910451c0cda4d Mon Sep 17 00:00:00 2001
From: Leann Ogasawara <[email protected]>
Date: Tue, 15 Jun 2010 14:01:51 -0700
Subject: [PATCH] p54usb: Comment out duplicate Medion MD40900 device id

The Medion MD40900 device id [0x0cde, 0x0006] is defined twice.
Comment out the duplicate.

Originally-by: Ben Collins <[email protected]>
Signed-off-by: Leann Ogasawara <[email protected]>
---
drivers/net/wireless/p54/p54usb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 7307325..2d8d03d 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -69,7 +69,8 @@ static struct usb_device_id p54u_table[] __devinitdata = {
{USB_DEVICE(0x0915, 0x2002)}, /* Cohiba Proto board */
{USB_DEVICE(0x0baf, 0x0118)}, /* U.S. Robotics U5 802.11g Adapter*/
{USB_DEVICE(0x0bf8, 0x1009)}, /* FUJITSU E-5400 USB D1700*/
- {USB_DEVICE(0x0cde, 0x0006)}, /* Medion MD40900 */
+ /* {USB_DEVICE(0x0cde, 0x0006)}, * Medion MD40900 already listed above,
+ * just noting it here for clarity */
{USB_DEVICE(0x0cde, 0x0008)}, /* Sagem XG703A */
{USB_DEVICE(0x0cde, 0x0015)}, /* Zcomax XG-705A */
{USB_DEVICE(0x0d8e, 0x3762)}, /* DLink DWL-G120 Cohiba */
--
1.7.0.4




2010-06-18 15:05:39

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] p54usb: Remove duplicate Medion MD40900 device id

On 06/15/2010 02:46 PM, John W. Linville wrote:
> On Tue, Jun 15, 2010 at 10:59:05AM -0700, Leann Ogasawara wrote:
>> On Mon, 2010-06-14 at 14:55 -0400, John W. Linville wrote:
>
>>> So, I guess you are concerned about the groupings because of the
>>> different firmwares or something like that? Perhaps a comment that
>>> says "this could be a version 2 device" is just as handy? Since the
>>> driver prints the name of the firmware it wants, is there any real
>>> need for grouping the IDs?
>>>
>>> OTOH, is there any actual harm from the duplicate entry? It "seems"
>>> wrong to me too, but I guess it does no harm...?
>>
>> I don't believe there is any harm from the duplicate entry, it just
>> seemed unnecessary.
>>
>>> Leann and/or Ben, was this just tidying-up? I'm guessing there wasn't
>>> an actual bug involved?
>>
>> Indeed, this was just a patch we'd been carrying to tidy things up.
>> There was no actual bug involved. I'd be happy to send a v2 of the
>> patch which comments out the duplicate entry and adds a note as to why.
>> Or I'd be fine just leaving the code as is and we'll drop the patch
>> we're carrying locally in Ubuntu.
>
> FWIW, I like the 'comment-out and add a note' option.

I'm a little late here after being offline, but I too like the comment
out option.

Larry