[PATCH] pci_ids.h: add new Atheros USB vendor ID

[PATCH] pci_ids.h: add new Atheros USB vendor ID

[Luis R. Rodriguez]

New Atheros USB devices will use 0x0cf3. The first device
will be ar9271, for which we are adding support for.

Cc: zhifeng.cai@atheros.com
Cc: stephen.chen@atheros.com
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---

John, sending this through you as this is only
specific to wireless, at least at the moment.

 include/linux/pci_ids.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 73b46b6..27f4e6b 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2170,6 +2170,7 @@
 #define PCI_DEVICE_ID_BCM1250_HT	0x0002
 
 #define PCI_VENDOR_ID_ATHEROS		0x168c
+#define PCI_VENDOR_ID_ATHEROS_USB	0x0cf3
 
 #define PCI_VENDOR_ID_NETCELL		0x169c
 #define PCI_DEVICE_ID_REVOLUTION	0x0044
-- 
1.6.3.3

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Marcel Holtmann]

Hi Luiz,

> New Atheros USB devices will use 0x0cf3. The first device
> will be ar9271, for which we are adding support for.
> 
> Cc: zhifeng.cai@atheros.com
> Cc: stephen.chen@atheros.com
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
> 
> John, sending this through you as this is only
> specific to wireless, at least at the moment.
> 
>  include/linux/pci_ids.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 73b46b6..27f4e6b 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2170,6 +2170,7 @@
>  #define PCI_DEVICE_ID_BCM1250_HT	0x0002
>  
>  #define PCI_VENDOR_ID_ATHEROS		0x168c
> +#define PCI_VENDOR_ID_ATHEROS_USB	0x0cf3

this is confusing the hell out of me. What are you adding here. A USB
vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?

Regards

Marcel

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Luis R. Rodriguez]

On Wed, Aug 5, 2009 at 9:16 PM, Marcel Holtmann<marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> New Atheros USB devices will use 0x0cf3. The first device
>> will be ar9271, for which we are adding support for.
>>
>> Cc: zhifeng.cai@atheros.com
>> Cc: stephen.chen@atheros.com
>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>> ---
>>
>> John, sending this through you as this is only
>> specific to wireless, at least at the moment.
>>
>>  include/linux/pci_ids.h |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 73b46b6..27f4e6b 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -2170,6 +2170,7 @@
>>  #define PCI_DEVICE_ID_BCM1250_HT     0x0002
>>
>>  #define PCI_VENDOR_ID_ATHEROS                0x168c
>> +#define PCI_VENDOR_ID_ATHEROS_USB    0x0cf3
>
> this is confusing the hell out of me. What are you adding here.

No you're right, my fault.

> A USB
> vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?

Its only USB.

Thanks... hm I see no USB vendor id headers. Guess we won't need to add this.

  Luis

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Marcel Holtmann]

Hi Luiz,

> >> New Atheros USB devices will use 0x0cf3. The first device
> >> will be ar9271, for which we are adding support for.
> >>
> >> Cc: zhifeng.cai@atheros.com
> >> Cc: stephen.chen@atheros.com
> >> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> >> ---
> >>
> >> John, sending this through you as this is only
> >> specific to wireless, at least at the moment.
> >>
> >>  include/linux/pci_ids.h |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> >> index 73b46b6..27f4e6b 100644
> >> --- a/include/linux/pci_ids.h
> >> +++ b/include/linux/pci_ids.h
> >> @@ -2170,6 +2170,7 @@
> >>  #define PCI_DEVICE_ID_BCM1250_HT     0x0002
> >>
> >>  #define PCI_VENDOR_ID_ATHEROS                0x168c
> >> +#define PCI_VENDOR_ID_ATHEROS_USB    0x0cf3
> >
> > this is confusing the hell out of me. What are you adding here.
> 
> No you're right, my fault.
> 
> > A USB
> > vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?
> 
> Its only USB.
> 
> Thanks... hm I see no USB vendor id headers. Guess we won't need to add this.

the closest we have is drivers/hid/hid-ids.h and we might just should
have Greg create a usb-ids.h file.

Regards

Marcel

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Greg KH]

On Wed, Aug 05, 2009 at 09:35:57PM -0700, Marcel Holtmann wrote:
> Hi Luiz,
> 
> > >> New Atheros USB devices will use 0x0cf3. The first device
> > >> will be ar9271, for which we are adding support for.
> > >>
> > >> Cc: zhifeng.cai@atheros.com
> > >> Cc: stephen.chen@atheros.com
> > >> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> > >> ---
> > >>
> > >> John, sending this through you as this is only
> > >> specific to wireless, at least at the moment.
> > >>
> > >>  include/linux/pci_ids.h |    1 +
> > >>  1 files changed, 1 insertions(+), 0 deletions(-)
> > >>
> > >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > >> index 73b46b6..27f4e6b 100644
> > >> --- a/include/linux/pci_ids.h
> > >> +++ b/include/linux/pci_ids.h
> > >> @@ -2170,6 +2170,7 @@
> > >>  #define PCI_DEVICE_ID_BCM1250_HT     0x0002
> > >>
> > >>  #define PCI_VENDOR_ID_ATHEROS                0x168c
> > >> +#define PCI_VENDOR_ID_ATHEROS_USB    0x0cf3
> > >
> > > this is confusing the hell out of me. What are you adding here.
> > 
> > No you're right, my fault.
> > 
> > > A USB
> > > vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?
> > 
> > Its only USB.
> > 
> > Thanks... hm I see no USB vendor id headers. Guess we won't need to add this.
> 
> the closest we have is drivers/hid/hid-ids.h and we might just should
> have Greg create a usb-ids.h file.

Ick, no, there's no such file for a good reason.  Put your device ids in
the driver files, no need to share it across the whole kernel.

thanks,

greg k-h

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Marcel Holtmann]

Hi Luiz,

> > > >> New Atheros USB devices will use 0x0cf3. The first device
> > > >> will be ar9271, for which we are adding support for.
> > > >>
> > > >> Cc: zhifeng.cai@atheros.com
> > > >> Cc: stephen.chen@atheros.com
> > > >> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> > > >> ---
> > > >>
> > > >> John, sending this through you as this is only
> > > >> specific to wireless, at least at the moment.
> > > >>
> > > >>  include/linux/pci_ids.h |    1 +
> > > >>  1 files changed, 1 insertions(+), 0 deletions(-)
> > > >>
> > > >> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > > >> index 73b46b6..27f4e6b 100644
> > > >> --- a/include/linux/pci_ids.h
> > > >> +++ b/include/linux/pci_ids.h
> > > >> @@ -2170,6 +2170,7 @@
> > > >>  #define PCI_DEVICE_ID_BCM1250_HT     0x0002
> > > >>
> > > >>  #define PCI_VENDOR_ID_ATHEROS                0x168c
> > > >> +#define PCI_VENDOR_ID_ATHEROS_USB    0x0cf3
> > > >
> > > > this is confusing the hell out of me. What are you adding here.
> > > 
> > > No you're right, my fault.
> > > 
> > > > A USB
> > > > vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?
> > > 
> > > Its only USB.
> > > 
> > > Thanks... hm I see no USB vendor id headers. Guess we won't need to add this.
> > 
> > the closest we have is drivers/hid/hid-ids.h and we might just should
> > have Greg create a usb-ids.h file.
> 
> Ick, no, there's no such file for a good reason.  Put your device ids in
> the driver files, no need to share it across the whole kernel.

that is fine with me btw. I have always done this. I do actually prefer
the plain ids and then a comment above their usage :)

Regards

Marcel

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Dave Jones]

On Wed, Aug 05, 2009 at 10:09:32PM -0700, Greg Kroah-Hartman wrote:
 > > the closest we have is drivers/hid/hid-ids.h and we might just should
 > > have Greg create a usb-ids.h file.
 >
 > Ick, no, there's no such file for a good reason.  Put your device ids in
 > the driver files, no need to share it across the whole kernel.

I agree. In fact, I'd go further, and say that the bulk of what we have
in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
entries in that file are used by multiple files in the kernel.

	Dave

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[John W. Linville]

On Thu, Aug 06, 2009 at 02:15:46PM -0400, Dave Jones wrote:
> On Wed, Aug 05, 2009 at 10:09:32PM -0700, Greg Kroah-Hartman wrote:
>  > > the closest we have is drivers/hid/hid-ids.h and we might just should
>  > > have Greg create a usb-ids.h file.
>  >
>  > Ick, no, there's no such file for a good reason.  Put your device ids in
>  > the driver files, no need to share it across the whole kernel.
> 
> I agree. In fact, I'd go further, and say that the bulk of what we have
> in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
> entries in that file are used by multiple files in the kernel.

Not much point in duplicating the vendor IDs among drivers.  I agree
that sharing the device IDs has no obvious benefit.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Greg KH]

On Thu, Aug 06, 2009 at 02:15:46PM -0400, Dave Jones wrote:
> On Wed, Aug 05, 2009 at 10:09:32PM -0700, Greg Kroah-Hartman wrote:
>  > > the closest we have is drivers/hid/hid-ids.h and we might just should
>  > > have Greg create a usb-ids.h file.
>  >
>  > Ick, no, there's no such file for a good reason.  Put your device ids in
>  > the driver files, no need to share it across the whole kernel.
> 
> I agree. In fact, I'd go further, and say that the bulk of what we have
> in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
> entries in that file are used by multiple files in the kernel.

I agree with that as well, which is why when I was PCI maintainer, I
didn't allow any new additions to the pci_ids.h file, unless it was
going to be an id that was shared by different drivers.  That cut the
new additions down to about none :)

thanks,

greg k-h

[PATCH] Document pci_ids.h addition policy.

[Dave Jones]

On Thu, Aug 06, 2009 at 11:33:26AM -0700, Greg Kroah-Hartman wrote:
 
 > > I agree. In fact, I'd go further, and say that the bulk of what we have
 > > in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
 > > entries in that file are used by multiple files in the kernel.
 > 
 > I agree with that as well, which is why when I was PCI maintainer, I
 > didn't allow any new additions to the pci_ids.h file, unless it was
 > going to be an id that was shared by different drivers.  That cut the
 > new additions down to about none :)

Perhaps documenting this is a good idea to prevent things being moved there,
as it still seems to be happening judging from the git log.

Signed-off-by: Dave Jones <davej@redhat.com>
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 73b46b6..f9156fd 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2,6 +2,9 @@
  *	PCI Class, Vendor and Device IDs
  *
  *	Please keep sorted.
+ *
+ *	Do not add new entries to this file unless the definitions
+ *	are shared between multiple drivers.
  */
 
 /* Device classes and subclasses */

Re: [PATCH] Document pci_ids.h addition policy.

[John W. Linville]

On Thu, Aug 06, 2009 at 03:13:59PM -0400, Dave Jones wrote:
> On Thu, Aug 06, 2009 at 11:33:26AM -0700, Greg Kroah-Hartman wrote:
>  
>  > > I agree. In fact, I'd go further, and say that the bulk of what we have
>  > > in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
>  > > entries in that file are used by multiple files in the kernel.
>  > 
>  > I agree with that as well, which is why when I was PCI maintainer, I
>  > didn't allow any new additions to the pci_ids.h file, unless it was
>  > going to be an id that was shared by different drivers.  That cut the
>  > new additions down to about none :)
> 
> Perhaps documenting this is a good idea to prevent things being moved there,
> as it still seems to be happening judging from the git log.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 73b46b6..f9156fd 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2,6 +2,9 @@
>   *	PCI Class, Vendor and Device IDs
>   *
>   *	Please keep sorted.
> + *
> + *	Do not add new entries to this file unless the definitions
> + *	are shared between multiple drivers.
>   */
>  
>  /* Device classes and subclasses */
> 

ACK

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] Document pci_ids.h addition policy.

[Greg KH]

On Thu, Aug 06, 2009 at 03:13:59PM -0400, Dave Jones wrote:
> On Thu, Aug 06, 2009 at 11:33:26AM -0700, Greg Kroah-Hartman wrote:
>  
>  > > I agree. In fact, I'd go further, and say that the bulk of what we have
>  > > in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
>  > > entries in that file are used by multiple files in the kernel.
>  > 
>  > I agree with that as well, which is why when I was PCI maintainer, I
>  > didn't allow any new additions to the pci_ids.h file, unless it was
>  > going to be an id that was shared by different drivers.  That cut the
>  > new additions down to about none :)
> 
> Perhaps documenting this is a good idea to prevent things being moved there,
> as it still seems to be happening judging from the git log.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

Re: [PATCH] Document pci_ids.h addition policy.

[Jesse Barnes]

On Thu, 6 Aug 2009 15:13:59 -0400
Dave Jones <davej@redhat.com> wrote:

> On Thu, Aug 06, 2009 at 11:33:26AM -0700, Greg Kroah-Hartman wrote:
>  
>  > > I agree. In fact, I'd go further, and say that the bulk of what
>  > > we have in pci_ids.h is also pointlessly shared. Only a tiny
>  > > fraction of the device entries in that file are used by multiple
>  > > files in the kernel.
>  > 
>  > I agree with that as well, which is why when I was PCI maintainer,
>  > I didn't allow any new additions to the pci_ids.h file, unless it
>  > was going to be an id that was shared by different drivers.  That
>  > cut the new additions down to about none :)
> 
> Perhaps documenting this is a good idea to prevent things being moved
> there, as it still seems to be happening judging from the git log.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 73b46b6..f9156fd 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2,6 +2,9 @@
>   *	PCI Class, Vendor and Device IDs
>   *
>   *	Please keep sorted.
> + *
> + *	Do not add new entries to this file unless the definitions
> + *	are shared between multiple drivers.
>   */
>  
>  /* Device classes and subclasses */
> 

Applied to linux-next, thanks.  I'll try to be more strict about this
too (I've reminded a few people of this informal policy already).

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] pci_ids.h: add new Atheros USB vendor ID

[Joe Perches]

On Thu, 2009-08-06 at 14:15 -0400, Dave Jones wrote:
> On Wed, Aug 05, 2009 at 10:09:32PM -0700, Greg Kroah-Hartman wrote:
>  > > the closest we have is drivers/hid/hid-ids.h and we might just should
>  > > have Greg create a usb-ids.h file.
>  > Ick, no, there's no such file for a good reason.  Put your device ids in
>  > the driver files, no need to share it across the whole kernel.
> I agree. In fact, I'd go further, and say that the bulk of what we have
> in pci_ids.h is also pointlessly shared. Only a tiny fraction of the device
> entries in that file are used by multiple files in the kernel.

I created some scripts a while ago to go through the
kernel to convert struct pci_device definitions to use
PCI_VDEVICE and PCI_DEVICE.

I could move definitions out of the pci_ids.h file at
the same time and closer to their actual use if those
definitions were not globally used.  Maybe move them
by module or by specific file.

Perhaps like this example unsigned, not to be applied,
patch below:

diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index d2ba04d..e978eb8 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -264,30 +264,12 @@ static void amd_iso_dev_put(void)
 
 /* List of quirks for OHCI */
 static const struct pci_device_id ohci_pci_quirks[] = {
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x740c),
-		.driver_data = (unsigned long)ohci_quirk_amd756,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_OPTI, 0xc861),
-		.driver_data = (unsigned long)ohci_quirk_opti,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_NS, PCI_ANY_ID),
-		.driver_data = (unsigned long)ohci_quirk_ns,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xa0f8),
-		.driver_data = (unsigned long)ohci_quirk_zfmicro,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA_2, 0x01b6),
-		.driver_data = (unsigned long)ohci_quirk_toshiba_scc,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_NEC, PCI_DEVICE_ID_NEC_USB),
-		.driver_data = (unsigned long)ohci_quirk_nec,
-	},
+	{ PCI_UNNAMED_DEVICE(AMD, 0x740c), .driver_data = (unsigned long)ohci_quirk_amd756 },
+	{ PCI_UNNAMED_DEVICE(OPTI, 0xc861), .driver_data = (unsigned long)ohci_quirk_opti },
+	{ PCI_UNNAMED_DEVICE(NS, PCI_ANY_ID), .driver_data = (unsigned long)ohci_quirk_ns },
+	{ PCI_UNNAMED_DEVICE(COMPAQ, 0xa0f8), .driver_data = (unsigned long)ohci_quirk_zfmicro },
+	{ PCI_UNNAMED_DEVICE(TOSHIBA_2, 0x01b6), .driver_data = (unsigned long)ohci_quirk_toshiba_scc },
+	{ PCI_NAMED_DEVICE(NEC, NEC_USB), .driver_data = (unsigned long)ohci_quirk_nec },
 	{
 		/* Toshiba portege 4000 */
 		.vendor		= PCI_VENDOR_ID_AL,
@@ -296,22 +278,10 @@ static const struct pci_device_id ohci_pci_quirks[] = {
 		.subdevice	= 0x0004,
 		.driver_data	= (unsigned long) broken_suspend,
 	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_ITE, 0x8152),
-		.driver_data = (unsigned long) broken_suspend,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4397),
-		.driver_data = (unsigned long)ohci_quirk_amd700,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4398),
-		.driver_data = (unsigned long)ohci_quirk_amd700,
-	},
-	{
-		PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
-		.driver_data = (unsigned long)ohci_quirk_amd700,
-	},
+	{ PCI_UNNAMED_DEVICE(ITE, 0x8152), .driver_data = (kernel_ulong_t)broken_suspend },
+	{ PCI_UNNAMED_DEVICE(ATI, 0x4397), .driver_data = (kernel_ulong_t)ohci_quirk_amd700 },
+	{ PCI_UNNAMED_DEVICE(ATI, 0x4398), .driver_data = (kernel_ulong_t)ohci_quirk_amd700 },
+	{ PCI_UNNAMED_DEVICE(ATI, 0x4399), .driver_data = (kernel_ulong_t)ohci_quirk_amd700 },
 
 	/* FIXME for some of the early AMD 760 southbridges, OHCI
 	 * won't work at all.  blacklist them.
diff --git a/drivers/usb/host/whci/hcd.c b/drivers/usb/host/whci/hcd.c
index e019a50..91e6563 100644
--- a/drivers/usb/host/whci/hcd.c
+++ b/drivers/usb/host/whci/hcd.c
@@ -351,7 +351,7 @@ static void __exit whci_hc_driver_exit(void)
 module_exit(whci_hc_driver_exit);
 
 /* PCI device ID's that we handle (so it gets loaded) */
-static struct pci_device_id whci_hcd_id_table[] = {
+static const struct pci_device_id whci_hcd_id_table[]  = {
 	{ PCI_DEVICE_CLASS(PCI_CLASS_WIRELESS_WHCI, ~0) },
 	{ /* empty last entry */ }
 };