2009-08-06 01:59:42

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH] 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]
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(-)

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



2009-08-10 16:28:22

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH] Document pci_ids.h addition policy.

On Thu, 6 Aug 2009 15:13:59 -0400
Dave Jones <[email protected]> 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 <[email protected]>
>
> 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

2009-08-06 18:15:55

by Dave Jones

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

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


2009-08-06 19:31:07

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] Document pci_ids.h addition policy.

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 <[email protected]>
>
> 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
[email protected] might be all we have. Be ready.

2009-08-06 19:46:46

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Document pci_ids.h addition policy.

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

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

2009-08-06 04:36:03

by Marcel Holtmann

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

Hi Luiz,

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



2009-08-06 18:31:07

by John W. Linville

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

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
[email protected] might be all we have. Be ready.

2009-08-06 04:16:17

by Marcel Holtmann

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

Hi Luiz,

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



2009-08-06 04:27:14

by Luis R. Rodriguez

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

On Wed, Aug 5, 2009 at 9:16 PM, Marcel Holtmann<[email protected]> wrote:
> Hi Luiz,
>
>> 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(-)
>>
>> 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

2009-08-06 18:52:42

by Joe Perches

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

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 */ }
};



2009-08-06 19:14:07

by Dave Jones

[permalink] [raw]
Subject: [PATCH] Document pci_ids.h addition policy.

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

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

2009-08-06 06:01:05

by Greg KH

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

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

2009-08-06 18:57:49

by Greg KH

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

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

2009-08-06 06:17:34

by Marcel Holtmann

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

Hi Luiz,

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