[PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[Seth Heasley]

This patch adds the LPC Controller DeviceIDs for the Intel DH89xxCC PCH.

Signed-off-by: Seth Heasley <seth.heasley@intel.com>

--- linux-2.6.37/include/linux/pci_ids.h.orig	2011-01-04 16:50:19.000000000 -0800
+++ linux-2.6.37/include/linux/pci_ids.h	2011-01-06 14:25:05.000000000 -0800
@@ -2469,6 +2469,9 @@
 #define PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX	0x1c5f
 #define PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS	0x1d22
 #define PCI_DEVICE_ID_INTEL_PATSBURG_LPC	0x1d40
+#define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN	0x2310
+#define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX	0x231f
+#define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS	0x2330
 #define PCI_DEVICE_ID_INTEL_82801AA_0	0x2410
 #define PCI_DEVICE_ID_INTEL_82801AA_1	0x2411
 #define PCI_DEVICE_ID_INTEL_82801AA_3	0x2413
--- linux-2.6.37/arch/x86/pci/irq.c.orig	2011-01-04 16:50:19.000000000 -0800
+++ linux-2.6.37/arch/x86/pci/irq.c	2011-01-06 14:16:00.000000000 -0800
@@ -611,6 +611,15 @@
 		r->set = pirq_piix_set;
 		return 1;
 	}
+
+	if ((device >= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN) &&
+		(device <= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX)) {
+		r->name = "PIIX/ICH";
+		r->get = pirq_piix_get;
+		r->set = pirq_piix_set;
+		return 1;
+	}
+
 	return 0;
 }

Re: [PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[Jean Delvare]

On Mon, 10 Jan 2011 10:58:13 -0800, Seth Heasley wrote:
> This patch adds the LPC Controller DeviceIDs for the Intel DH89xxCC PCH.
> 
> Signed-off-by: Seth Heasley <seth.heasley@intel.com>
> 
> --- linux-2.6.37/include/linux/pci_ids.h.orig	2011-01-04 16:50:19.000000000 -0800
> +++ linux-2.6.37/include/linux/pci_ids.h	2011-01-06 14:25:05.000000000 -0800
> @@ -2469,6 +2469,9 @@
>  #define PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX	0x1c5f
>  #define PCI_DEVICE_ID_INTEL_PATSBURG_SMBUS	0x1d22
>  #define PCI_DEVICE_ID_INTEL_PATSBURG_LPC	0x1d40
> +#define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN	0x2310
> +#define PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX	0x231f
> +#define PCI_DEVICE_ID_INTEL_DH89XXCC_SMBUS	0x2330
>  #define PCI_DEVICE_ID_INTEL_82801AA_0	0x2410
>  #define PCI_DEVICE_ID_INTEL_82801AA_1	0x2411
>  #define PCI_DEVICE_ID_INTEL_82801AA_3	0x2413
> --- linux-2.6.37/arch/x86/pci/irq.c.orig	2011-01-04 16:50:19.000000000 -0800
> +++ linux-2.6.37/arch/x86/pci/irq.c	2011-01-06 14:16:00.000000000 -0800
> @@ -611,6 +611,15 @@
>  		r->set = pirq_piix_set;
>  		return 1;
>  	}
> +
> +	if ((device >= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN) &&
> +		(device <= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX)) {
> +		r->name = "PIIX/ICH";
> +		r->get = pirq_piix_get;
> +		r->set = pirq_piix_set;
> +		return 1;
> +	}
> +

I'm curious why we have separate sections for INTEL_5_3400_SERIES,
INTEL_COUGARPOINT and now INTEL_DH89XXCC? We do the same thing for all
3, so it would make sense to merge the tests to avoid duplicating the
code.

Other than this, the code looks good.

>  	return 0;
>  }


-- 
Jean Delvare

RE: [PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[Heasley, Seth]

>> --- linux-2.6.37/arch/x86/pci/irq.c.orig	2011-01-04
>16:50:19.000000000 -0800
>> +++ linux-2.6.37/arch/x86/pci/irq.c	2011-01-06 14:16:00.000000000
>-0800
>> @@ -611,6 +611,15 @@
>>  		r->set = pirq_piix_set;
>>  		return 1;
>>  	}
>> +
>> +	if ((device >= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN) &&
>> +		(device <= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX)) {
>> +		r->name = "PIIX/ICH";
>> +		r->get = pirq_piix_get;
>> +		r->set = pirq_piix_set;
>> +		return 1;
>> +	}
>> +
>
>I'm curious why we have separate sections for INTEL_5_3400_SERIES,
>INTEL_COUGARPOINT and now INTEL_DH89XXCC? We do the same thing for all
>3, so it would make sense to merge the tests to avoid duplicating the
>code.

Some products are defining a range of DeviceIDs rather than one or two specific IDs, so these blocks are for catching any DeviceID in the range.  I'm certainly open to suggestion for a better way to handle these, and I agree that the current way is awkward.

-Seth  

Re: [PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[Jean Delvare]

On Mon, 10 Jan 2011 11:19:15 -0800, Heasley, Seth wrote:
> >> --- linux-2.6.37/arch/x86/pci/irq.c.orig	2011-01-04
> >16:50:19.000000000 -0800
> >> +++ linux-2.6.37/arch/x86/pci/irq.c	2011-01-06 14:16:00.000000000
> >-0800
> >> @@ -611,6 +611,15 @@
> >>  		r->set = pirq_piix_set;
> >>  		return 1;
> >>  	}
> >> +
> >> +	if ((device >= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN) &&
> >> +		(device <= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX)) {
> >> +		r->name = "PIIX/ICH";
> >> +		r->get = pirq_piix_get;
> >> +		r->set = pirq_piix_set;
> >> +		return 1;
> >> +	}
> >> +
> >
> >I'm curious why we have separate sections for INTEL_5_3400_SERIES,
> >INTEL_COUGARPOINT and now INTEL_DH89XXCC? We do the same thing for all
> >3, so it would make sense to merge the tests to avoid duplicating the
> >code.
> 
> Some products are defining a range of DeviceIDs rather than one or two specific IDs, so these blocks are for catching any DeviceID in the range.  I'm certainly open to suggestion for a better way to handle these, and I agree that the current way is awkward.

I understand the need to have separate blocks for separate IDs and for
ID ranges. But all the ranges could be covered by the same block:

	if ((device >= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MIN && 
	     device <= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MAX)
	 || (device >= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN && 
	     device <= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX)) {
		r->name = "PIIX/ICH";
		r->get = pirq_piix_get;
		r->set = pirq_piix_set;
		return 1;
	}

-- 
Jean Delvare

RE: [PATCH 2.6.37] irq: irq and pci_ids patch for Intel DH89xxCC DeviceIDs

[Heasley, Seth]

>> Some products are defining a range of DeviceIDs rather than one or two
>specific IDs, so these blocks are for catching any DeviceID in the
>range.  I'm certainly open to suggestion for a better way to handle
>these, and I agree that the current way is awkward.
>
>I understand the need to have separate blocks for separate IDs and for
>ID ranges. But all the ranges could be covered by the same block:
>
>	if ((device >= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MIN &&
>	     device <= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MAX)
>	 || (device >= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN &&
>	     device <= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX)) {
>		r->name = "PIIX/ICH";
>		r->get = pirq_piix_get;
>		r->set = pirq_piix_set;
>		return 1;
>	}

Not sure why I didn't do it this way.  Thanks for the help!

-Seth