Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-23 Thread Adam J. Richter

>Keith Owens wrote:
>> 
>> [Adam J. Richter]
>> > +static struct pci_device_id atp870u_pci_tbl[] __initdata = {
>> > +{vendor: 0x1191, device: 0x8002, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
>> > +{vendor: 0x1191, device: 0x8010, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
>> 
>> It would make it easier to read and safer to type if you used a macro
>> to generate the fields.
>> 
>> #define PCITBL(v,d,sv,sd) \
>>  { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
>>PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }
>> 
>> #define PCITBL_END {0,0,0,0}
>> 
>> static struct pci_device_id foo_pci_tbl[] __initdata = {
>>   PCITBL(INTEL, INTEL_82437VX, ANY, ANY),
>>   PCITBL_END
>> }

>* your macro fails for the 'ANY' case, because the proper macro is
>PCI_ANY_ID not PCI_{VENDOR,DEVICE}_ID_ANY.
>* many drivers need to set the driver_data field

>That said, the general idea presented above is quite good.  The PCITBL
>macro will probably change on a per-driver basis... I don't think it
>belongs in a common header.  For example, ethernet drivers might want to
>have a macro that always checks for PCI-CLASS-ETHERNET under the hood,
>while visibly looking like

>PCITBL(INTEL, 82437VX, {a driver_data value}),
>PCITBL(INTEL, 82987VX, {another driver_data value}),
>PCITBL_END

>   Jeff


Jaroslav Kysela's isapnp MODULE_DEVICE_TABLE support uses a
a similar scheme (see Documentation/isapnp.txt and
include/linux/isapnp.h).  I also used similar macros to add
support usb MODULE_DEVICE_TABLE support for usb/storage/
(not yet in 2.4.0-test11 yet) and usb/pegasus.c.

Note that, in all of these cases, the macros expand to field:value
form, which addresses my concerns about modifiability.

The cases where I implemented this sort of thing in the USB code
were where there was a big tables of values that included device
ID information plus more information than could comfortably fit in
in id->driver_data.  Using a trick used in the gcc sources, I changed
the table entries into a calls to a macro and moved them into a
separate .h file, which was included multiple times with different
definitions of the macro to construct the usb_device_id table and a
parallel table of additional driver specific data.  There are a bunch
of cases in the code that I have been adding pci_device_id tables
to where an approach like this will probably end up being used
when somebody gets around to eliminating the duplication of data
between the pci_device_id table and the "old" way the driver
organized the PCI ID information.

In the specific case of the current drivers/scsi/atp870u.c, 
I think a macro would not help yet becuase the pci entries
still fit on one line without the macro.  So, it would just
make the code longer and perhaps less clear.  However, when the
devid[] array in that driver is eliminated and perhaps the
driver is ported to the new PCI interface, then the
vendor_data field will probably be used and it will probably be
worthwhile to define a macro as you describe (at least that's
my prediction; I don't claim to be the maintainer of any of these
drivers).

Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-23 Thread Adam J. Richter

Keith Owens wrote:
 
 [Adam J. Richter]
  +static struct pci_device_id atp870u_pci_tbl[] __initdata = {
  +{vendor: 0x1191, device: 0x8002, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
  +{vendor: 0x1191, device: 0x8010, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
 
 It would make it easier to read and safer to type if you used a macro
 to generate the fields.
 
 #define PCITBL(v,d,sv,sd) \
  { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }
 
 #define PCITBL_END {0,0,0,0}
 
 static struct pci_device_id foo_pci_tbl[] __initdata = {
   PCITBL(INTEL, INTEL_82437VX, ANY, ANY),
   PCITBL_END
 }

* your macro fails for the 'ANY' case, because the proper macro is
PCI_ANY_ID not PCI_{VENDOR,DEVICE}_ID_ANY.
* many drivers need to set the driver_data field

That said, the general idea presented above is quite good.  The PCITBL
macro will probably change on a per-driver basis... I don't think it
belongs in a common header.  For example, ethernet drivers might want to
have a macro that always checks for PCI-CLASS-ETHERNET under the hood,
while visibly looking like

PCITBL(INTEL, 82437VX, {a driver_data value}),
PCITBL(INTEL, 82987VX, {another driver_data value}),
PCITBL_END

   Jeff


Jaroslav Kysela's isapnp MODULE_DEVICE_TABLE support uses a
a similar scheme (see Documentation/isapnp.txt and
include/linux/isapnp.h).  I also used similar macros to add
support usb MODULE_DEVICE_TABLE support for usb/storage/
(not yet in 2.4.0-test11 yet) and usb/pegasus.c.

Note that, in all of these cases, the macros expand to field:value
form, which addresses my concerns about modifiability.

The cases where I implemented this sort of thing in the USB code
were where there was a big tables of values that included device
ID information plus more information than could comfortably fit in
in id-driver_data.  Using a trick used in the gcc sources, I changed
the table entries into a calls to a macro and moved them into a
separate .h file, which was included multiple times with different
definitions of the macro to construct the usb_device_id table and a
parallel table of additional driver specific data.  There are a bunch
of cases in the code that I have been adding pci_device_id tables
to where an approach like this will probably end up being used
when somebody gets around to eliminating the duplication of data
between the pci_device_id table and the "old" way the driver
organized the PCI ID information.

In the specific case of the current drivers/scsi/atp870u.c, 
I think a macro would not help yet becuase the pci entries
still fit on one line without the macro.  So, it would just
make the code longer and perhaps less clear.  However, when the
devid[] array in that driver is eliminated and perhaps the
driver is ported to the new PCI interface, then the
vendor_data field will probably be used and it will probably be
worthwhile to define a macro as you describe (at least that's
my prediction; I don't claim to be the maintainer of any of these
drivers).

Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Keith Owens

On Thu, 23 Nov 2000 02:25:33 -0500, 
Jeff Garzik <[EMAIL PROTECTED]> wrote:
>Keith Owens wrote:
>> #define PCITBL(v,d,sv,sd) \
>>  { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
>>PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }
>
>* your macro fails for the 'ANY' case, because the proper macro is
>PCI_ANY_ID not PCI_{VENDOR,DEVICE}_ID_ANY.

It was just a suggestion.  Actually getting it working was left as an
exercise for the reader ;).

#define PCI_VENDOR_ID_ANY PCI_ANY_ID
#define PCI_DEVICE_ID_ANY PCI_ANY_ID

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Keith Owens

[Adam J. Richter]
> +static struct pci_device_id atp870u_pci_tbl[] __initdata = {
> +{vendor: 0x1191, device: 0x8002, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
> +{vendor: 0x1191, device: 0x8010, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},

It would make it easier to read and safer to type if you used a macro
to generate the fields.

#define PCITBL(v,d,sv,sd) \
 { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
   PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }

#define PCITBL_END {0,0,0,0}

static struct pci_device_id foo_pci_tbl[] __initdata = {
  PCITBL(INTEL, INTEL_82437VX, ANY, ANY),
  PCITBL_END
}

Shorter is easier to read.  Using a prefix on the fields makes it much
harder for somebody to accidentally swap device and vendor codes.  If
they swap the parameters and type
  PCITBL(INTEL_82437VX, INTEL, ANY, ANY),
by mistake then they get compile errors, PCI_VENDOR_ID_INTEL_82437VX
and PCI_DEVICE_ID_INTEL are undefined.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Adam J. Richter

Christoph Hellwig <[EMAIL PROTECTED]> writes:

>Neither there are lots of NULL-initilized fields nor is
>there any reason to add new fields (the pci tables are external
>API, because of MODULE_DEVICE_TABLE).

PCI ID matching relies on the zeros being filled in for
an empty value in the case of class_mask.  depmod deliberately
includes a format comment at the start of modules.pcimap so that the
structure can be changed in the future.

However, thanks for your feedback.  I will take it into
consideration.

Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Christoph Hellwig

In article <[EMAIL PROTECTED]> you wrote:

> --opJtzjQTFsWo+cga
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline


>   Here is my first pass at adding pci_device_id tables to all
> PCI scsi drivers in linux-2.4.0-test11.  It implements a compromise
> regarding named initializers for pci_device_id table entries: shorter
> tables or tables that contain anonymous constants use the named fields,
> but the few longer tables where the purpose of the constants are more
> clearly labelled do not use named fields, because those tables would
> be really big otherwise (in terms of lines of source code, not what
> they compile into).

IMHO the pci tables look much cleaner without named initiliters.
They are really ugly if we nest structures and arrays.
The other argument for named initilizers don't aren't true in this
case too.  Neither there are lots of NULL-initilized fields nor is
there any reason to add new fields (the pci tables are external
API, because of MODULE_DEVICE_TABLE).

Christoph

-- 
Always remember that you are unique.  Just like everyone else.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Adam J. Richter


Here is my first pass at adding pci_device_id tables to all
PCI scsi drivers in linux-2.4.0-test11.  It implements a compromise
regarding named initializers for pci_device_id table entries: shorter
tables or tables that contain anonymous constants use the named fields,
but the few longer tables where the purpose of the constants are more
clearly labelled do not use named fields, because those tables would
be really big otherwise (in terms of lines of source code, not what
they compile into).  The theory in this is that the number of tables
that use the unlabelled structure initializers is still kept relatively
small, so a change to pci_device_id that would require changing the
initializers would involve changing only a relatively small number
of drivers, as opposed to potentially all ~150 PCI drivers.

-- 
Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."


--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/3w-.c Wed Nov  8 17:09:50 2000
+++ 3w-.c   Wed Nov 22 04:04:27 2000
@@ -87,6 +87,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -100,6 +101,17 @@
 #include "hosts.h"
 
 #include "3w-.h"
+
+static struct pci_device_id tw_scsi_pci_tbl[] __initdata = {
+   {
+ vendor: TW_VENDOR_ID,
+ device: TW_DEVICE_ID,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, tw_scsi_pci_tbl);
 
 static int tw_copy_info(TW_Info *info, char *fmt, ...);
 static void tw_copy_mem_info(TW_Info *info, char *data, int len);
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/AM53C974.cSun Nov 12 20:40:42 
2000
+++ AM53C974.c  Wed Nov 22 04:28:10 2000
@@ -340,6 +340,18 @@
 #define TAG_NONE   -2  /* Establish I_T_L nexus instead of I_T_L_Q
   * even on SCSI-II devices */
 
+static struct pci_device_id am53c974_pci_tbl[] __initdata = {
+   {
+ vendor: PCI_VENDOR_ID_AMD,
+ device: PCI_DEVICE_ID_AMD_SCSI,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, am53c974_pci_tbl);
+
+
 / LILO overrides */
 typedef struct _override_t {
int host_scsi_id;   /* SCSI id of the bus controller */
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/BusLogic.cSat Nov 11 19:01:11 
2000
+++ BusLogic.c  Wed Nov 22 04:07:16 2000
@@ -54,6 +54,29 @@
 #include "FlashPoint.c"
 
 
+static struct pci_device_id buslogic_pci_tbl[] __initdata = {
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER_NC,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, buslogic_pci_tbl);
+
 /*
   BusLogic_DriverOptionsCount is a count of the number of BusLogic Driver
   Options specifications provided via the Linux Kernel Command Line or via
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/NCR53C9x.cMon Jun 19 17:59:41 
2000
+++ NCR53C9x.c  Sat Nov 18 03:22:01 2000
@@ -21,10 +21,7 @@
  * 4) Maybe change use of "esp" to something more "NCR"'ish.
  */
 
-#ifdef MODULE
 #include 
-#endif
-
 #include 
 #include 
 #include 
@@ -3604,6 +3601,15 @@
spin_unlock_irqrestore(_request_lock, flags);
 }
 #endif
+
+EXPORT_SYMBOL(esp_intr);
+EXPORT_SYMBOL(esp_allocate);
+EXPORT_SYMBOL(esp_initialize);
+EXPORT_SYMBOL(esp_reset);
+EXPORT_SYMBOL(esp_abort);
+EXPORT_SYMBOL(esps_in_use);
+EXPORT_SYMBOL(esp_deallocate);
+EXPORT_SYMBOL(esp_queue);
 
 #ifdef MODULE
 int init_module(void) { return 0; }
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/advansys.cSat Nov 11 19:01:11 
2000
+++ advansys.c  Wed Nov 22 04:02:15 2000
@@ -4071,6 +4071,18 @@
 #endif /* ASC_CONFIG_PCI */
 #endif /* version < v2.1.93 */
 
+#if LINUX_VERSION_CODE >= ASC_LINUX_VERSION(2,4,0)
+static struct pci_device_id advansys_pci_tbl[] __initdata = {
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1100, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1200, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1300, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_2300, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, 

Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Adam J. Richter


Here is my first pass at adding pci_device_id tables to all
PCI scsi drivers in linux-2.4.0-test11.  It implements a compromise
regarding named initializers for pci_device_id table entries: shorter
tables or tables that contain anonymous constants use the named fields,
but the few longer tables where the purpose of the constants are more
clearly labelled do not use named fields, because those tables would
be really big otherwise (in terms of lines of source code, not what
they compile into).  The theory in this is that the number of tables
that use the unlabelled structure initializers is still kept relatively
small, so a change to pci_device_id that would require changing the
initializers would involve changing only a relatively small number
of drivers, as opposed to potentially all ~150 PCI drivers.

-- 
Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."


--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/3w-.c Wed Nov  8 17:09:50 2000
+++ 3w-.c   Wed Nov 22 04:04:27 2000
@@ -87,6 +87,7 @@
 #include linux/smp.h
 #include linux/reboot.h
 #include linux/spinlock.h
+#include linux/init.h
 
 #include asm/errno.h
 #include asm/io.h
@@ -100,6 +101,17 @@
 #include "hosts.h"
 
 #include "3w-.h"
+
+static struct pci_device_id tw_scsi_pci_tbl[] __initdata = {
+   {
+ vendor: TW_VENDOR_ID,
+ device: TW_DEVICE_ID,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, tw_scsi_pci_tbl);
 
 static int tw_copy_info(TW_Info *info, char *fmt, ...);
 static void tw_copy_mem_info(TW_Info *info, char *data, int len);
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/AM53C974.cSun Nov 12 20:40:42 
2000
+++ AM53C974.c  Wed Nov 22 04:28:10 2000
@@ -340,6 +340,18 @@
 #define TAG_NONE   -2  /* Establish I_T_L nexus instead of I_T_L_Q
   * even on SCSI-II devices */
 
+static struct pci_device_id am53c974_pci_tbl[] __initdata = {
+   {
+ vendor: PCI_VENDOR_ID_AMD,
+ device: PCI_DEVICE_ID_AMD_SCSI,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, am53c974_pci_tbl);
+
+
 / LILO overrides */
 typedef struct _override_t {
int host_scsi_id;   /* SCSI id of the bus controller */
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/BusLogic.cSat Nov 11 19:01:11 
2000
+++ BusLogic.c  Wed Nov 22 04:07:16 2000
@@ -54,6 +54,29 @@
 #include "FlashPoint.c"
 
 
+static struct pci_device_id buslogic_pci_tbl[] __initdata = {
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER_NC,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   {
+ vendor: PCI_VENDOR_ID_BUSLOGIC,
+ device: PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT,
+ subvendor: PCI_ANY_ID,
+ subdevice: PCI_ANY_ID,
+   },
+   { } /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(pci, buslogic_pci_tbl);
+
 /*
   BusLogic_DriverOptionsCount is a count of the number of BusLogic Driver
   Options specifications provided via the Linux Kernel Command Line or via
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/NCR53C9x.cMon Jun 19 17:59:41 
2000
+++ NCR53C9x.c  Sat Nov 18 03:22:01 2000
@@ -21,10 +21,7 @@
  * 4) Maybe change use of "esp" to something more "NCR"'ish.
  */
 
-#ifdef MODULE
 #include linux/module.h
-#endif
-
 #include linux/config.h
 #include linux/kernel.h
 #include linux/delay.h
@@ -3604,6 +3601,15 @@
spin_unlock_irqrestore(io_request_lock, flags);
 }
 #endif
+
+EXPORT_SYMBOL(esp_intr);
+EXPORT_SYMBOL(esp_allocate);
+EXPORT_SYMBOL(esp_initialize);
+EXPORT_SYMBOL(esp_reset);
+EXPORT_SYMBOL(esp_abort);
+EXPORT_SYMBOL(esps_in_use);
+EXPORT_SYMBOL(esp_deallocate);
+EXPORT_SYMBOL(esp_queue);
 
 #ifdef MODULE
 int init_module(void) { return 0; }
--- /tmp/adam/linux-2.4.0-test11/drivers/scsi/advansys.cSat Nov 11 19:01:11 
2000
+++ advansys.c  Wed Nov 22 04:02:15 2000
@@ -4071,6 +4071,18 @@
 #endif /* ASC_CONFIG_PCI */
 #endif /* version  v2.1.93 */
 
+#if LINUX_VERSION_CODE = ASC_LINUX_VERSION(2,4,0)
+static struct pci_device_id advansys_pci_tbl[] __initdata = {
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1100, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1200, PCI_ANY_ID, PCI_ANY_ID },
+{ ADV_PCI_VENDOR_ID, ASC_PCI_DEVICE_ID_1300, PCI_ANY_ID, 

Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Christoph Hellwig

In article [EMAIL PROTECTED] you wrote:

 --opJtzjQTFsWo+cga
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline


   Here is my first pass at adding pci_device_id tables to all
 PCI scsi drivers in linux-2.4.0-test11.  It implements a compromise
 regarding named initializers for pci_device_id table entries: shorter
 tables or tables that contain anonymous constants use the named fields,
 but the few longer tables where the purpose of the constants are more
 clearly labelled do not use named fields, because those tables would
 be really big otherwise (in terms of lines of source code, not what
 they compile into).

IMHO the pci tables look much cleaner without named initiliters.
They are really ugly if we nest structures and arrays.
The other argument for named initilizers don't aren't true in this
case too.  Neither there are lots of NULL-initilized fields nor is
there any reason to add new fields (the pci tables are external
API, because of MODULE_DEVICE_TABLE).

Christoph

-- 
Always remember that you are unique.  Just like everyone else.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Adam J. Richter

Christoph Hellwig [EMAIL PROTECTED] writes:

Neither there are lots of NULL-initilized fields nor is
there any reason to add new fields (the pci tables are external
API, because of MODULE_DEVICE_TABLE).

PCI ID matching relies on the zeros being filled in for
an empty value in the case of class_mask.  depmod deliberately
includes a format comment at the start of modules.pcimap so that the
structure can be changed in the future.

However, thanks for your feedback.  I will take it into
consideration.

Adam J. Richter __ __   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ /  San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l   United States of America
fax +1 408 261-6631  "Free Software For The Rest Of Us."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Keith Owens

[Adam J. Richter]
 +static struct pci_device_id atp870u_pci_tbl[] __initdata = {
 +{vendor: 0x1191, device: 0x8002, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},
 +{vendor: 0x1191, device: 0x8010, subvendor: PCI_ANY_ID, subdevice: PCI_ANY_ID},

It would make it easier to read and safer to type if you used a macro
to generate the fields.

#define PCITBL(v,d,sv,sd) \
 { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
   PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }

#define PCITBL_END {0,0,0,0}

static struct pci_device_id foo_pci_tbl[] __initdata = {
  PCITBL(INTEL, INTEL_82437VX, ANY, ANY),
  PCITBL_END
}

Shorter is easier to read.  Using a prefix on the fields makes it much
harder for somebody to accidentally swap device and vendor codes.  If
they swap the parameters and type
  PCITBL(INTEL_82437VX, INTEL, ANY, ANY),
by mistake then they get compile errors, PCI_VENDOR_ID_INTEL_82437VX
and PCI_DEVICE_ID_INTEL are undefined.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: Patch(?): pci_device_id tables for drivers/scsi in 2.4.0-test11

2000-11-22 Thread Keith Owens

On Thu, 23 Nov 2000 02:25:33 -0500, 
Jeff Garzik [EMAIL PROTECTED] wrote:
Keith Owens wrote:
 #define PCITBL(v,d,sv,sd) \
  { PCI_VENDOR_ID_##v, PCI_DEVICE_ID_##d, \
PCI_VENDOR_ID_##sv, PCI_DEVICE_ID_##sd }

* your macro fails for the 'ANY' case, because the proper macro is
PCI_ANY_ID not PCI_{VENDOR,DEVICE}_ID_ANY.

It was just a suggestion.  Actually getting it working was left as an
exercise for the reader ;).

#define PCI_VENDOR_ID_ANY PCI_ANY_ID
#define PCI_DEVICE_ID_ANY PCI_ANY_ID

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/