pci_ids.h: add new Atheros USB vendor ID

PATCH In review Luis R. Rodriguez 7 participants 14 messages Aug 6, 2009 – Aug 10, 2009

Subsystems (3)

USB PCI USB ohci driver

lore

Share & tools

lore

Luis R. Rodriguez

author

2009-08-06 01:59 UTC #0 lore raw

PATCH 1/1 pci_ids.h: add new Atheros USB vendor ID

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

Cc [email protected]

Signed-off-by Luis R. Rodriguez <[email protected]>

---

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(-)

include/linux/pci_ids.h +1 -0

index 73b46b6..27f4e6b 100644
@@ -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

Signature · 1 line

1.6.3.3

Marcel Holtmann

2009-08-06 04:16 UTC #1 lore raw

Hi Luiz,

Quoted 19 lines

New Atheros USB devices will use 0x0cf3. The first device will be ar9271, for which we are adding support for.
Cc [email protected]
Cc [email protected]
Signed-off-by Luis R. Rodriguez <[email protected]>
---
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(-)
include/linux/pci_ids.h +0 -0
index 73b46b6..27f4e6b 100644

include/linux/pci_ids.h line 2173

Quoted 4 lines

include/linux/pci_ids.h +1 -0

@@ -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

Luis R. Rodriguez

author

2009-08-06 04:27 UTC #2 lore raw

Quoted 23 lines · Marcel Holtmann

Hi Luiz,
Quoted 21 lines
New Atheros USB devices will use 0x0cf3. The first device will be ar9271, for which we are adding support for.
Cc [email protected]
Cc [email protected]
Signed-off-by Luis R. Rodriguez <[email protected]>
---
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(-)
include/linux/pci_ids.h +0 -0
index 73b46b6..27f4e6b 100644
@@ -2170,6 +2170,7 @@
  #define PCI_DEVICE_ID_BCM1250_HT     0x0002
 

Quoted 4 lines

include/linux/pci_ids.h line 2173
Quoted 2 lines
include/linux/pci_ids.h +1 -0
  #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.

Quoted 2 lines

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

Marcel Holtmann

2009-08-06 04:36 UTC #3 lore raw

Hi Luiz,

Quoted 26 lines

Quoted 21 lines
Quoted 21 lines
New Atheros USB devices will use 0x0cf3. The first device will be ar9271, for which we are adding support for.
Cc [email protected]
Cc [email protected]
Signed-off-by Luis R. Rodriguez <[email protected]>
---
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(-)
include/linux/pci_ids.h +0 -0
index 73b46b6..27f4e6b 100644
@@ -2170,6 +2170,7 @@
 #define PCI_DEVICE_ID_BCM1250_HT     0x0002
 
Quoted 4 lines
include/linux/pci_ids.h line 2173
Quoted 2 lines
include/linux/pci_ids.h +1 -0
 #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.

Quoted 8 lines

No you're right, my fault.
Quoted 2 lines
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

Greg KH

2009-08-06 06:01 UTC #4 lore raw

Quoted 33 lines · Marcel Holtmann

Hi Luiz,
Quoted 31 lines
Quoted 21 lines
Quoted 21 lines
New Atheros USB devices will use 0x0cf3. The first device will be ar9271, for which we are adding support for.
Cc [email protected]
Cc [email protected]
Signed-off-by Luis R. Rodriguez <[email protected]>
---
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(-)
include/linux/pci_ids.h +0 -0
index 73b46b6..27f4e6b 100644
@@ -2170,6 +2170,7 @@
 #define PCI_DEVICE_ID_BCM1250_HT     0x0002
 
Quoted 4 lines
include/linux/pci_ids.h line 2173
Quoted 2 lines
include/linux/pci_ids.h +1 -0
 #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.
Quoted 2 lines
A USB
vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?

Quoted 6 lines

Quoted 3 lines
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

Marcel Holtmann

2009-08-06 06:17 UTC #5 lore raw

Hi Luiz,

Quoted 33 lines

Quoted 33 lines
Quoted 33 lines
Quoted 21 lines
Quoted 21 lines
New Atheros USB devices will use 0x0cf3. The first device will be ar9271, for which we are adding support for.
Cc [email protected]
Cc [email protected]
Signed-off-by Luis R. Rodriguez <[email protected]>
---
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(-)
include/linux/pci_ids.h +0 -0
index 73b46b6..27f4e6b 100644
@@ -2170,6 +2170,7 @@
 #define PCI_DEVICE_ID_BCM1250_HT     0x0002
 
Quoted 4 lines
include/linux/pci_ids.h line 2173
Quoted 2 lines
include/linux/pci_ids.h +1 -0
 #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.
Quoted 2 lines
A USB
vendor id or a PCI one. Is the ar9271 a PCI device or a USB device?
Its only USB.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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

Dave Jones

2009-08-06 18:15 UTC #6 lore raw

In reply to Greg KH

Quoted 5 lines · Greg Kroah-Hartman

Quoted 2 lines
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

John W. Linville

2009-08-06 18:31 UTC #7 lore raw

Quoted 10 lines · Dave Jones

Quoted 5 lines · Greg Kroah-Hartman
Quoted 2 lines
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

Signature · 2 lines

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

Joe Perches

2009-08-06 18:52 UTC #8 lore raw

In reply to Dave Jones

Quoted 8 lines · Dave Jones

Quoted 4 lines · Greg Kroah-Hartman
Quoted 2 lines
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:

drivers/usb/host/ohci-pci.c +10 ~~-40~~

index d2ba04d..e978eb8 100644
@@ -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.

drivers/usb/host/whci/hcd.c +1 -1

index e019a50..91e6563 100644
@@ -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 */ }
 };

Greg KH

2009-08-06 18:57 UTC #9 lore raw

In reply to Dave Jones

Quoted 10 lines · Dave Jones

Quoted 5 lines · Greg Kroah-Hartman
Quoted 2 lines
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

Dave Jones

2009-08-06 19:14 UTC #10 lore raw

PATCH 1/1 Document pci_ids.h addition policy.

Quoted 8 lines · Greg Kroah-Hartman

Quoted 3 lines
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 <[email protected]>

include/linux/pci_ids.h +3 -0

index 73b46b6..f9156fd 100644
@@ -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 */

John W. Linville

2009-08-06 19:31 UTC #11 lore raw

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

Quoted 22 lines · Dave Jones

Quoted 8 lines · Greg Kroah-Hartman
Quoted 3 lines
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 <[email protected]>
include/linux/pci_ids.h +0 -0
index 73b46b6..f9156fd 100644

include/linux/pci_ids.h line 10

Quoted 9 lines

include/linux/pci_ids.h +3 -0

@@ -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

Signature · 2 lines

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

Greg KH

Acked-by Greg Kroah-Hartman on v1 1/1

2009-08-06 19:46 UTC #12 lore raw

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

In reply to Dave Jones

Quoted 11 lines · Dave Jones

Quoted 8 lines · Greg Kroah-Hartman
Quoted 3 lines
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 :)

Quoted 4 lines

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 <[email protected]>

Acked-by Greg Kroah-Hartman <[email protected]>

Jesse Barnes

Applied

2009-08-10 16:28 UTC #13 lore raw

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

In reply to Dave Jones

Quoted 23 lines · On Thu, 6 Aug 2009 15:13:59 -0400 Dave Jones <[email protected]> wrote:

Quoted 9 lines · Greg Kroah-Hartman
Quoted 4 lines
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 <[email protected]>
include/linux/pci_ids.h +0 -0
index 73b46b6..f9156fd 100644

include/linux/pci_ids.h line 10

Quoted 9 lines

include/linux/pci_ids.h +3 -0

@@ -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).

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center