linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PCI Constants Update for SATA Driver
@ 2005-02-27 17:42 Henning Schmiedehausen
  2005-02-27 18:08 ` Jeff Garzik
  2005-03-02  1:46 ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 9+ messages in thread
From: Henning Schmiedehausen @ 2005-02-27 17:42 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Linux IDE Mailingliste

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

Hi,

having the various constants in pci_ids.h is a really good thing for
grepping through the kernel source. And if they were used, it would be
even better (if you e.g. are looking for which driver claims an SIImage
3112 card...). However, the sata_xxx.c files in the drivers/scsi
directory are notorious for not using the PCI Ids but just hard coded
numbers.

This patch tries to fix this. It is against the 2.6.10 tree from Fedora
Core 3 but should apply to a regular 2.6.10 too.

	Regards
		Henning


-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

      RedHat Certified Engineer -- Jakarta Turbine Development
   Linux, Java, perl, Solaris -- Consulting, Training, Engineering

"Now you can start with implementation and integration and do the
requirements later".  -- Prof. Dr. Dr. h.c. Manfred Broy about the new
german federal software development standard "V-Model XT"
(found at http://de.biz.yahoo.com/050207/299/4en0t.html)

[-- Attachment #2: sata_pci.patch --]
[-- Type: text/x-patch, Size: 10075 bytes --]

--- linux-2.6.10/drivers/scsi/sata_promise.c~	2004-12-24 22:35:23.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_promise.c	2005-02-27 18:06:21.832987233 +0100
@@ -148,17 +148,17 @@
 };
 
 static struct pci_device_id pdc_ata_pci_tbl[] = {
-	{ PCI_VENDOR_ID_PROMISE, 0x3371, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20371, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_2037x },
-	{ PCI_VENDOR_ID_PROMISE, 0x3373, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20378, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_2037x },
-	{ PCI_VENDOR_ID_PROMISE, 0x3375, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20375, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_2037x },
-	{ PCI_VENDOR_ID_PROMISE, 0x3376, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20376, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_2037x },
-	{ PCI_VENDOR_ID_PROMISE, 0x3318, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20318, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_20319 },
-	{ PCI_VENDOR_ID_PROMISE, 0x3319, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20319, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_20319 },
 	{ }	/* terminate list */
 };
--- linux-2.6.10/drivers/scsi/sata_uli.c~	2005-02-27 12:29:10.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_uli.c	2005-02-27 18:06:13.659752507 +0100
@@ -51,9 +51,9 @@
 static void uli_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
 static struct pci_device_id uli_pci_tbl[] = {
-	{ PCI_VENDOR_ID_AL, 0x5289, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5289 },
-	{ PCI_VENDOR_ID_AL, 0x5287, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5287 },
-	{ PCI_VENDOR_ID_AL, 0x5281, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5281 },
+	{ PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_ULI_5287, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5289 },
+	{ PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_ULI_5289, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5287 },
+	{ PCI_VENDOR_ID_AL, PCI_DEVICE_ID_AL_M5281, PCI_ANY_ID, PCI_ANY_ID, 0, 0, uli_5281 },
 	{ }	/* terminate list */
 };
 
--- linux-2.6.10/drivers/scsi/sata_sil.c~	2005-02-27 12:29:10.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_sil.c	2005-02-27 18:06:20.152144613 +0100
@@ -67,12 +67,12 @@
 static void sil_post_set_mode (struct ata_port *ap);
 
 static struct pci_device_id sil_pci_tbl[] = {
-	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1095, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1095, 0x3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1095, 0x3114, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
-	{ 0x1002, 0x436e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
-	{ 0x1002, 0x4379, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ PCI_VENDOR_ID_CMD, PCI_DEVICE_ID_SII_3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ PCI_VENDOR_ID_CMD, PCI_DEVICE_ID_SII_1210SA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ PCI_VENDOR_ID_CMD, PCI_DEVICE_ID_SII_3512, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ PCI_VENDOR_ID_CMD, PCI_DEVICE_ID_SII_3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3114 },
+	{ PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP300_SATA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
+	{ PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_IXP400_SATA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
 	{ }	/* terminate list */
 };
 
--- linux-2.6.10/drivers/scsi/sata_via.c~	2004-12-24 22:34:01.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_via.c	2005-02-27 18:06:11.947912788 +0100
@@ -66,7 +66,7 @@
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
 static struct pci_device_id svia_pci_tbl[] = {
-	{ 0x1106, 0x3149, PCI_ANY_ID, PCI_ANY_ID, 0, 0, via_sata },
+	{ PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237_SATA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, via_sata },
 
 	{ }	/* terminate list */
 };
--- linux-2.6.10/drivers/scsi/sata_sx4.c~	2004-12-24 22:33:48.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_sx4.c	2005-02-27 17:50:24.569601228 +0100
@@ -223,7 +223,7 @@
 };
 
 static struct pci_device_id pdc_sata_pci_tbl[] = {
-	{ PCI_VENDOR_ID_PROMISE, 0x6622, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	{ PCI_VENDOR_ID_PROMISE, PCI_DEVICE_ID_PROMISE_20621, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_20621 },
 	{ }	/* terminate list */
 };
--- linux-2.6.10/drivers/scsi/sata_vsc.c~	2004-12-24 22:34:58.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_vsc.c	2005-02-27 18:06:01.601881495 +0100
@@ -363,8 +363,8 @@
  * compatibility is untested as of yet
  */
 static struct pci_device_id vsc_sata_pci_tbl[] = {
-	{ 0x1725, 0x7174, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
-	{ 0x8086, 0x3200, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
+	{ PCI_VENDOR_ID_VITESSE, PCI_DEVICE_ID_VITESSE_VSC7174, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
+	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_GD31244, PCI_ANY_ID, PCI_ANY_ID, 0x10600, 0xFFFFFF, 0 },
 	{ }
 };
 
--- linux-2.6.10/drivers/scsi/sata_sis.c~	2004-12-24 22:34:57.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_sis.c	2005-02-27 18:06:18.583291508 +0100
@@ -60,8 +60,8 @@
 static void sis_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
 static struct pci_device_id sis_pci_tbl[] = {
-	{ PCI_VENDOR_ID_SI, 0x180, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sis_180 },
-	{ PCI_VENDOR_ID_SI, 0x181, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sis_180 },
+	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_RAID180, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sis_180 },
+	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_RAID180_2, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sis_180 },
 	{ }	/* terminate list */
 };
 
--- linux-2.6.10/drivers/scsi/sata_svw.c~	2004-12-24 22:35:00.000000000 +0100
+++ linux-2.6.10/drivers/scsi/sata_svw.c	2005-02-27 18:06:16.878451135 +0100
@@ -439,9 +439,9 @@
 
 
 static struct pci_device_id k2_sata_pci_tbl[] = {
-	{ 0x1166, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
-	{ 0x1166, 0x0241, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
-	{ 0x1166, 0x0242, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_241, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+	{ PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_242, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
 	{ }
 };
 
--- linux-2.6.10/include/linux/pci_ids.h~	2005-02-27 18:35:28.312973711 +0100
+++ linux-2.6.10/include/linux/pci_ids.h	2005-02-27 18:37:04.623021628 +0100
@@ -598,6 +598,8 @@
 #define PCI_DEVICE_ID_SI_503		0x0008
 #define PCI_DEVICE_ID_SI_ACPI		0x0009
 #define PCI_DEVICE_ID_SI_LPC		0x0018
+#define PCI_DEVICE_ID_SI_RAID180	0x0180
+#define PCI_DEVICE_ID_SI_RAID180_2	0x0181
 #define PCI_DEVICE_ID_SI_5597_VGA	0x0200
 #define PCI_DEVICE_ID_SI_6205		0x0205
 #define PCI_DEVICE_ID_SI_501		0x0406
@@ -793,19 +795,26 @@
 
 #define PCI_VENDOR_ID_PROMISE		0x105a
 #define PCI_DEVICE_ID_PROMISE_20265	0x0d30
+#define PCI_DEVICE_ID_PROMISE_20263	0x0d38
+#define PCI_DEVICE_ID_PROMISE_20275	0x1275
+#define PCI_DEVICE_ID_PROMISE_20318	0x3318
+#define PCI_DEVICE_ID_PROMISE_20319	0x3319
+#define PCI_DEVICE_ID_PROMISE_20371	0x3371
+#define PCI_DEVICE_ID_PROMISE_20378	0x3373
+#define PCI_DEVICE_ID_PROMISE_20375	0x3375
+#define PCI_DEVICE_ID_PROMISE_20376	0x3376
 #define PCI_DEVICE_ID_PROMISE_20267	0x4d30
 #define PCI_DEVICE_ID_PROMISE_20246	0x4d33
 #define PCI_DEVICE_ID_PROMISE_20262	0x4d38
-#define PCI_DEVICE_ID_PROMISE_20263	0x0D38
 #define PCI_DEVICE_ID_PROMISE_20268	0x4d68
-#define PCI_DEVICE_ID_PROMISE_20268R	0x6268
 #define PCI_DEVICE_ID_PROMISE_20269	0x4d69
+#define PCI_DEVICE_ID_PROMISE_20276	0x5275
+#define PCI_DEVICE_ID_PROMISE_5300	0x5300
+#define PCI_DEVICE_ID_PROMISE_20268R	0x6268
 #define PCI_DEVICE_ID_PROMISE_20270	0x6268
 #define PCI_DEVICE_ID_PROMISE_20271	0x6269
-#define PCI_DEVICE_ID_PROMISE_20275	0x1275
-#define PCI_DEVICE_ID_PROMISE_20276	0x5275
+#define PCI_DEVICE_ID_PROMISE_20621	0x6622
 #define PCI_DEVICE_ID_PROMISE_20277	0x7275
-#define PCI_DEVICE_ID_PROMISE_5300	0x5300
 
 #define PCI_VENDOR_ID_N9		0x105d
 #define PCI_DEVICE_ID_N9_I128		0x2309
@@ -927,6 +936,7 @@
 #define PCI_DEVICE_ID_SUN_TOMATILLO	0xa801
 
 #define PCI_VENDOR_ID_CMD		0x1095
+#define PCI_DEVICE_ID_SII_1210SA	0x0240
 #define PCI_DEVICE_ID_CMD_640		0x0640
 #define PCI_DEVICE_ID_CMD_643		0x0643
 #define PCI_DEVICE_ID_CMD_646		0x0646
@@ -938,7 +948,8 @@
 
 #define PCI_DEVICE_ID_SII_680		0x0680
 #define PCI_DEVICE_ID_SII_3112		0x3112
-#define PCI_DEVICE_ID_SII_1210SA	0x0240
+#define PCI_DEVICE_ID_SII_3114		0x3114
+#define PCI_DEVICE_ID_SII_3512		0x3512
 
 #define PCI_VENDOR_ID_VISION		0x1098
 #define PCI_DEVICE_ID_VISION_QD8500	0x0001
@@ -1050,6 +1061,9 @@
 #define PCI_DEVICE_ID_AL_M5228		0x5228
 #define PCI_DEVICE_ID_AL_M5229		0x5229
 #define PCI_DEVICE_ID_AL_M5237		0x5237
+#define PCI_DEVICE_ID_AL_M5281		0x5281
+#define PCI_DEVICE_ID_AL_ULI_5287	0x5287
+#define PCI_DEVICE_ID_AL_ULI_5289	0x5289
 #define PCI_DEVICE_ID_AL_M5243		0x5243
 #define PCI_DEVICE_ID_AL_M5451		0x5451
 #define PCI_DEVICE_ID_AL_M7101		0x7101
@@ -1447,6 +1461,9 @@
 #define PCI_DEVICE_ID_SERVERWORKS_GCLE    0x0225
 #define PCI_DEVICE_ID_SERVERWORKS_GCLE2   0x0227
 #define PCI_DEVICE_ID_SERVERWORKS_CSB5ISA 0x0230
+#define PCI_DEVICE_ID_SERVERWORKS_K2_240  0x0240
+#define PCI_DEVICE_ID_SERVERWORKS_K2_241  0x0241
+#define PCI_DEVICE_ID_SERVERWORKS_K2_242  0x0242
 
 #define PCI_VENDOR_ID_SBE		0x1176
 #define PCI_DEVICE_ID_SBE_WANXL100	0x0301
@@ -2015,6 +2032,9 @@
 #define PCI_DEVICE_ID_FARSITE_TE1       0x1610
 #define PCI_DEVICE_ID_FARSITE_TE1C      0x1612
 
+#define PCI_VENDOR_ID_VITESSE           0x1725
+#define PCI_DEVICE_ID_VITESSE_VSC7174   0x7174
+
 #define PCI_VENDOR_ID_LINKSYS		0x1737
 #define PCI_DEVICE_ID_LINKSYS_EG1032	0x1032
 #define PCI_DEVICE_ID_LINKSYS_EG1064	0x1064
@@ -2275,6 +2295,7 @@
 #define PCI_DEVICE_ID_INTEL_ICH7_21	0x27df
 #define PCI_DEVICE_ID_INTEL_ICH7_22	0x27e0
 #define PCI_DEVICE_ID_INTEL_ICH7_23	0x27e2
+#define PCI_DEVICE_ID_INTEL_GD31244	0x3200
 #define PCI_DEVICE_ID_INTEL_82855PM_HB	0x3340
 #define PCI_DEVICE_ID_INTEL_82830_HB	0x3575
 #define PCI_DEVICE_ID_INTEL_82830_CGC	0x3577

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 17:42 PCI Constants Update for SATA Driver Henning Schmiedehausen
@ 2005-02-27 18:08 ` Jeff Garzik
  2005-02-27 19:20   ` Henning Schmiedehausen
  2005-03-02  1:47   ` Benjamin Herrenschmidt
  2005-03-02  1:46 ` Benjamin Herrenschmidt
  1 sibling, 2 replies; 9+ messages in thread
From: Jeff Garzik @ 2005-02-27 18:08 UTC (permalink / raw)
  To: Henning Schmiedehausen; +Cc: Linux IDE Mailingliste

Henning Schmiedehausen wrote:
> Hi,
> 
> having the various constants in pci_ids.h is a really good thing for
> grepping through the kernel source. And if they were used, it would be
> even better (if you e.g. are looking for which driver claims an SIImage
> 3112 card...). However, the sata_xxx.c files in the drivers/scsi
> directory are notorious for not using the PCI Ids but just hard coded
> numbers.
> 
> This patch tries to fix this. It is against the 2.6.10 tree from Fedora
> Core 3 but should apply to a regular 2.6.10 too.

"Notorious", heh.  This is quite intentional.

PCI device ids are just random numbers that vendors pick out of thin 
air.  Device id symbolic constants have little value, and creates churn 
whereby every kernel hacker is patching include/linux/pci_ids.h.

	Jeff



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 18:08 ` Jeff Garzik
@ 2005-02-27 19:20   ` Henning Schmiedehausen
  2005-02-27 19:52     ` Jeff Garzik
  2005-03-02  1:47   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 9+ messages in thread
From: Henning Schmiedehausen @ 2005-02-27 19:20 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Linux IDE Mailingliste

Hi Jeff,

then what sense does it make to have pci_ids.h at all?

	Regards
		Henning

On Sun, 2005-02-27 at 13:08 -0500, Jeff Garzik wrote:
> Henning Schmiedehausen wrote:
> > Hi,
> > 
> > having the various constants in pci_ids.h is a really good thing for
> > grepping through the kernel source. And if they were used, it would be
> > even better (if you e.g. are looking for which driver claims an SIImage
> > 3112 card...). However, the sata_xxx.c files in the drivers/scsi
> > directory are notorious for not using the PCI Ids but just hard coded
> > numbers.
> > 
> > This patch tries to fix this. It is against the 2.6.10 tree from Fedora
> > Core 3 but should apply to a regular 2.6.10 too.
> 
> "Notorious", heh.  This is quite intentional.
> 
> PCI device ids are just random numbers that vendors pick out of thin 
> air.  Device id symbolic constants have little value, and creates churn 
> whereby every kernel hacker is patching include/linux/pci_ids.h.
> 
> 	Jeff
> 
> 
-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

      RedHat Certified Engineer -- Jakarta Turbine Development
   Linux, Java, perl, Solaris -- Consulting, Training, Engineering

"Now you can start with implementation and integration and do the
requirements later".  -- Prof. Dr. Dr. h.c. Manfred Broy about the new
german federal software development standard "V-Model XT"
(found at http://de.biz.yahoo.com/050207/299/4en0t.html)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 19:20   ` Henning Schmiedehausen
@ 2005-02-27 19:52     ` Jeff Garzik
  2005-02-27 20:05       ` Henning Schmiedehausen
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Garzik @ 2005-02-27 19:52 UTC (permalink / raw)
  To: Henning Schmiedehausen; +Cc: Linux IDE Mailingliste

Henning Schmiedehausen wrote:
> Hi Jeff,
> 
> then what sense does it make to have pci_ids.h at all?

pci_ids.h does not only contain PCI_DEVICE_ID_xxx constants.

	Jeff




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 19:52     ` Jeff Garzik
@ 2005-02-27 20:05       ` Henning Schmiedehausen
  2005-02-27 20:07         ` Jeff Garzik
  0 siblings, 1 reply; 9+ messages in thread
From: Henning Schmiedehausen @ 2005-02-27 20:05 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Linux IDE Mailingliste

Let me rephrase:

What sense does it make to have PCI_VENDOR_ID and PCI_DEVICE_ID
constants if using them is optional/discouraged?

	Regards
		Henning



On Sun, 2005-02-27 at 14:52 -0500, Jeff Garzik wrote:
> Henning Schmiedehausen wrote:
> > Hi Jeff,
> > 
> > then what sense does it make to have pci_ids.h at all?
> 
> pci_ids.h does not only contain PCI_DEVICE_ID_xxx constants.
> 
> 	Jeff
> 
> 
> 
-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

      RedHat Certified Engineer -- Jakarta Turbine Development
   Linux, Java, perl, Solaris -- Consulting, Training, Engineering

"Now you can start with implementation and integration and do the
requirements later".  -- Prof. Dr. Dr. h.c. Manfred Broy about the new
german federal software development standard "V-Model XT"
(found at http://de.biz.yahoo.com/050207/299/4en0t.html)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 20:05       ` Henning Schmiedehausen
@ 2005-02-27 20:07         ` Jeff Garzik
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Garzik @ 2005-02-27 20:07 UTC (permalink / raw)
  To: Henning Schmiedehausen; +Cc: Linux IDE Mailingliste

On Sun, Feb 27, 2005 at 09:05:06PM +0100, Henning Schmiedehausen wrote:
> Let me rephrase:
> 
> What sense does it make to have PCI_VENDOR_ID and PCI_DEVICE_ID
> constants if using them is optional/discouraged?

Only PCI_DEVICE_ID_xxx is discouraged.  Not PCI_VENDOR_ID_xxx.

	Jeff




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 17:42 PCI Constants Update for SATA Driver Henning Schmiedehausen
  2005-02-27 18:08 ` Jeff Garzik
@ 2005-03-02  1:46 ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2005-03-02  1:46 UTC (permalink / raw)
  To: Henning Schmiedehausen; +Cc: Jeff Garzik, Linux IDE Mailingliste

On Sun, 2005-02-27 at 18:42 +0100, Henning Schmiedehausen wrote:
 static struct pci_device_id k2_sata_pci_tbl[] = {
-       { 0x1166, 0x0240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
-       { 0x1166, 0x0241, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
-       { 0x1166, 0x0242, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+       { PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_240, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+       { PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_241, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
+       { PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_K2_242, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
        { }

I'm not sure the "K2" in the names here it correct. K2 is Apple's G5 IO
chip, which happens to include a serverworks controller. Jeff should
probably fix the constants to the real cell names :) Besides, K2 afaik
only ever contains a 240...
Ben.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-02-27 18:08 ` Jeff Garzik
  2005-02-27 19:20   ` Henning Schmiedehausen
@ 2005-03-02  1:47   ` Benjamin Herrenschmidt
  2005-03-02  5:49     ` Jeff Garzik
  1 sibling, 1 reply; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2005-03-02  1:47 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Henning Schmiedehausen, Linux IDE Mailingliste

On Sun, 2005-02-27 at 13:08 -0500, Jeff Garzik wrote:
> Henning Schmiedehausen wrote:
> > Hi,
> > 
> > having the various constants in pci_ids.h is a really good thing for
> > grepping through the kernel source. And if they were used, it would be
> > even better (if you e.g. are looking for which driver claims an SIImage
> > 3112 card...). However, the sata_xxx.c files in the drivers/scsi
> > directory are notorious for not using the PCI Ids but just hard coded
> > numbers.
> > 
> > This patch tries to fix this. It is against the 2.6.10 tree from Fedora
> > Core 3 but should apply to a regular 2.6.10 too.
> 
> "Notorious", heh.  This is quite intentional.
> 
> PCI device ids are just random numbers that vendors pick out of thin 
> air.  Device id symbolic constants have little value, and creates churn 
> whereby every kernel hacker is patching include/linux/pci_ids.h.

I still agree that it's handy to use the constants in pci_ids.h when
available, I do that for grepping regulary.

What I usually do is that I separately send patches updating pci_ids.h
and driver patches. In some cases, I leave the numeric constant in the
driver until I'm sure the pci_ids.h patch got in, then eventually fix up
the driver to use the constant.

Ben.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: PCI Constants Update for SATA Driver
  2005-03-02  1:47   ` Benjamin Herrenschmidt
@ 2005-03-02  5:49     ` Jeff Garzik
  0 siblings, 0 replies; 9+ messages in thread
From: Jeff Garzik @ 2005-03-02  5:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Henning Schmiedehausen, Linux IDE Mailingliste

Benjamin Herrenschmidt wrote:
> On Sun, 2005-02-27 at 13:08 -0500, Jeff Garzik wrote:
> 
>>Henning Schmiedehausen wrote:
>>
>>>Hi,
>>>
>>>having the various constants in pci_ids.h is a really good thing for
>>>grepping through the kernel source. And if they were used, it would be
>>>even better (if you e.g. are looking for which driver claims an SIImage
>>>3112 card...). However, the sata_xxx.c files in the drivers/scsi
>>>directory are notorious for not using the PCI Ids but just hard coded
>>>numbers.
>>>
>>>This patch tries to fix this. It is against the 2.6.10 tree from Fedora
>>>Core 3 but should apply to a regular 2.6.10 too.
>>
>>"Notorious", heh.  This is quite intentional.
>>
>>PCI device ids are just random numbers that vendors pick out of thin 
>>air.  Device id symbolic constants have little value, and creates churn 
>>whereby every kernel hacker is patching include/linux/pci_ids.h.
> 
> 
> I still agree that it's handy to use the constants in pci_ids.h when
> available, I do that for grepping regulary.
> 
> What I usually do is that I separately send patches updating pci_ids.h
> and driver patches. In some cases, I leave the numeric constant in the
> driver until I'm sure the pci_ids.h patch got in, then eventually fix up
> the driver to use the constant.

Well, for SATA and net drivers, I strongly prefer

	{ PCI_VENDOR_ID_xxx, 0x1234, PCI_ANY_ID, PCI_ANY_ID, ... }
		or
	{ PCI_DEVICE(PCI_VENDOR_ID_xxx, 0x1234), ... }

IFF [1] the PCI device id is used in more than one place in the entire 
kernel, then using PCI_DEVICE_ID_xxx is OK.

Regards,

	Jeff


[1] - that's "if and only if", for the non-math types


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2005-03-02  5:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-27 17:42 PCI Constants Update for SATA Driver Henning Schmiedehausen
2005-02-27 18:08 ` Jeff Garzik
2005-02-27 19:20   ` Henning Schmiedehausen
2005-02-27 19:52     ` Jeff Garzik
2005-02-27 20:05       ` Henning Schmiedehausen
2005-02-27 20:07         ` Jeff Garzik
2005-03-02  1:47   ` Benjamin Herrenschmidt
2005-03-02  5:49     ` Jeff Garzik
2005-03-02  1:46 ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).