2010-02-04 23:28:53

by Ha, Tristram

[permalink] [raw]
Subject: [PATCH 2.6.33 4/4] net: Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices

From: Tristram Ha <[email protected]>

Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices.

Signed-off-by: Tristram Ha <[email protected]>
---
This is a resubmission of the Micrel KSZ8841/2 PCI Ethernet driver.

--- linux-2.6.33-rc5.old/include/linux/pci_ids.h 2010-01-21 15:31:35.000000000 -0800
+++ linux-2.6.33-rc5.new/include/linux/pci_ids.h 2010-01-29 10:38:53.000000000 -0800
@@ -2199,6 +2199,12 @@
#define PCI_VENDOR_ID_NETCELL 0x169c
#define PCI_DEVICE_ID_REVOLUTION 0x0044

+#define PCI_VENDOR_ID_MICREL_KS 0x16c6
+#define PCI_DEVICE_ID_MICREL_KS8692 0x8692
+#define PCI_DEVICE_ID_MICREL_KS8695 0x8695
+#define PCI_DEVICE_ID_MICREL_KS8841 0x8841
+#define PCI_DEVICE_ID_MICREL_KS8842 0x8842
+
#define PCI_VENDOR_ID_CENATEK 0x16CA
#define PCI_DEVICE_ID_CENATEK_IDE 0x0001


2010-02-04 23:40:21

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2.6.33 4/4] net: Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices

From: "Ha, Tristram" <[email protected]>
Date: Thu, 4 Feb 2010 15:28:29 -0800

> From: Tristram Ha <[email protected]>
>
> Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices.
>
> Signed-off-by: Tristram Ha <[email protected]>

We no longer add device ID defines to the linux/pci_ids.h
file, in fact it's been this way for several years.

Just use constants or local devices in your driver.

Furthermore, because you made this the last patch, the
tree would fail to build at the point where the first
three of your patches are applied.

This breaks GIT bisection, and is severely frowned upon. The tree
must build with all available kernel config options enabled at every
step of applying your set of patches.

2010-02-04 23:40:30

by Stephen Hemminger

[permalink] [raw]
Subject: Re: [PATCH 2.6.33 4/4] net: Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices

On Thu, 4 Feb 2010 15:28:29 -0800
"Ha, Tristram" <[email protected]> wrote:

> From: Tristram Ha <[email protected]>
>
> Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices.
>
> Signed-off-by: Tristram Ha <[email protected]>
> ---
> This is a resubmission of the Micrel KSZ8841/2 PCI Ethernet driver.
>
> --- linux-2.6.33-rc5.old/include/linux/pci_ids.h 2010-01-21 15:31:35.000000000 -0800
> +++ linux-2.6.33-rc5.new/include/linux/pci_ids.h 2010-01-29 10:38:53.000000000 -0800
> @@ -2199,6 +2199,12 @@
> #define PCI_VENDOR_ID_NETCELL 0x169c
> #define PCI_DEVICE_ID_REVOLUTION 0x0044
>
> +#define PCI_VENDOR_ID_MICREL_KS 0x16c6
> +#define PCI_DEVICE_ID_MICREL_KS8692 0x8692
> +#define PCI_DEVICE_ID_MICREL_KS8695 0x8695
> +#define PCI_DEVICE_ID_MICREL_KS8841 0x8841
> +#define PCI_DEVICE_ID_MICREL_KS8842 0x8842
> +
> #define PCI_VENDOR_ID_CENATEK 0x16CA
> #define PCI_DEVICE_ID_CENATEK_IDE 0x0001
>

Current practice is to NOT update this file and instead
keep constants in the individual driver.

2010-02-05 02:57:49

by Ha, Tristram

[permalink] [raw]
Subject: RE: [PATCH 2.6.33 4/4] net: Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices

Stephen Hemminger wrote:
> On Thu, 4 Feb 2010 15:28:29 -0800
> "Ha, Tristram" <[email protected]> wrote:
>
>> From: Tristram Ha <[email protected]>
>>
>> Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices.
>>
>> Signed-off-by: Tristram Ha <[email protected]> ---
>> This is a resubmission of the Micrel KSZ8841/2 PCI Ethernet driver.
>>
>> --- linux-2.6.33-rc5.old/include/linux/pci_ids.h 2010-01-21
15:31:35.000000000 -0800
>> +++ linux-2.6.33-rc5.new/include/linux/pci_ids.h 2010-01-29
10:38:53.000000000 -0800 @@ -2199,6
>> +2199,12 @@ #define PCI_VENDOR_ID_NETCELL 0x169c
>> #define PCI_DEVICE_ID_REVOLUTION 0x0044
>>
>> +#define PCI_VENDOR_ID_MICREL_KS 0x16c6
>> +#define PCI_DEVICE_ID_MICREL_KS8692 0x8692
>> +#define PCI_DEVICE_ID_MICREL_KS8695 0x8695
>> +#define PCI_DEVICE_ID_MICREL_KS8841 0x8841
>> +#define PCI_DEVICE_ID_MICREL_KS8842 0x8842
>> +
>> #define PCI_VENDOR_ID_CENATEK 0x16CA
>> #define PCI_DEVICE_ID_CENATEK_IDE 0x0001
>>
>
> Current practice is to NOT update this file and instead keep constants
in the individual driver.

It seems I received conflicted recommendation from Alan Cox:

>> >> +#define PCI_VENDOR_ID_KS884X 0x16C6
>> >> +#define PCI_DEVICE_ID_KS8841 0x8841
>> >> +#define PCI_DEVICE_ID_KS8842 0x8842
>> >
>> > Those belong in the pci device id header.
>> >
>> >
>>
>> I do not quite understand your suggestion. Do I need to put those
IDs
>> in one of the kernel headers?
>
> Into include/linux/pci_ids.h

So I do not need to update pci_ids.h with even the PCI vendor ID?

2010-02-05 04:27:42

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2.6.33 4/4] net: Add PCI vendor and device ids for Micrel KSZ8841/2 PCI devices

From: "Ha, Tristram" <[email protected]>
Date: Thu, 4 Feb 2010 18:55:31 -0800

> Stephen Hemminger wrote:
>> Current practice is to NOT update this file and instead keep constants
> in the individual driver.
>
> It seems I received conflicted recommendation from Alan Cox:

Alan's understanding is several years out of date.

Stephen's is correct.

But you don't need these stupid defines at all, all you
use them for is the PCI device ID table in your driver
so just use constants.