struct pci_device_id declaration cleanups

struct pci_device_id declaration cleanups

[Jonas Bonn]

This set of patches applies the macro DECLARE_PCI_DEVICE_TABLE to the 
declarations of struct pci_device_id in several network drivers.  The drivers
build cleanly with these patches (without any new warnings) on both x86 and 
PowerPC.

There are ~90 such cleanups in the /drivers/net tree alone (and ~350 in the
kernel overall); these patches are just a subset to avoid flooding the mailing 
list before getting feedback.

These 9 patches can also be pulled from the "net" branch of:

git://www.southpole.se/~jonas/git/linux.git

The remainder of the cleanups will be pushed out to my public tree once I 
know the reaction to this subset.

The diffstat for this subset is:

 drivers/net/cassini.c                |    2 +-
 drivers/net/e1000/e1000_main.c       |    2 +-
 drivers/net/e1000e/netdev.c          |    2 +-
 drivers/net/netxen/netxen_nic_main.c |    2 +-
 drivers/net/wan/farsync.c            |    2 +-
 drivers/net/wan/pc300_drv.c          |    2 +-
 drivers/net/wan/pc300too.c           |    2 +-
 drivers/net/wan/pci200syn.c          |    2 +-
 drivers/net/wan/wanxl.c              |    2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

/Jonas

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/cassini.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index 14299f8..1451533 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -233,7 +233,7 @@ static u16 link_modes[] __devinitdata = {
 	CAS_BMCR_SPEED1000|BMCR_FULLDPLX /* 5 : 1000bt full duplex */
 };
 
-static struct pci_device_id cas_pci_tbl[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(cas_pci_tbl) = {
 	{ PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_CASSINI,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
 	{ PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_SATURN,
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/netxen/netxen_nic_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 9737eae..7eaff19 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -73,7 +73,7 @@ static irqreturn_t netxen_intr(int irq, void *data);
 int physical_port[] = {0, 1, 2, 3};
 
 /*  PCI Device ID Table  */
-static struct pci_device_id netxen_pci_tbl[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(netxen_pci_tbl) = {
 	{PCI_DEVICE(0x4040, 0x0001)},
 	{PCI_DEVICE(0x4040, 0x0002)},
 	{PCI_DEVICE(0x4040, 0x0003)},
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/wan/pc300too.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/pc300too.c b/drivers/net/wan/pc300too.c
index bf1b015..deacb94 100644
--- a/drivers/net/wan/pc300too.c
+++ b/drivers/net/wan/pc300too.c
@@ -501,7 +501,7 @@ static int __devinit pc300_pci_init_one(struct pci_dev *pdev,
 
 
 
-static struct pci_device_id pc300_pci_tbl[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(pc300_pci_tbl) = {
 	{ PCI_VENDOR_ID_CYCLADES, PCI_DEVICE_ID_PC300_RX_1, PCI_ANY_ID,
 	  PCI_ANY_ID, 0, 0, 0 },
 	{ PCI_VENDOR_ID_CYCLADES, PCI_DEVICE_ID_PC300_RX_2, PCI_ANY_ID,
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/wan/pc300_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/pc300_drv.c b/drivers/net/wan/pc300_drv.c
index 57914fb..df62fef 100644
--- a/drivers/net/wan/pc300_drv.c
+++ b/drivers/net/wan/pc300_drv.c
@@ -252,7 +252,7 @@ static char rcsid[] =
 #undef	PC300_DEBUG_RX
 #undef	PC300_DEBUG_OTHER
 
-static struct pci_device_id cpc_pci_dev_id[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(cpc_pci_dev_id) = {
 	/* PC300/RSV or PC300/X21, 2 chan */
 	{0x120e, 0x300, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0x300},
 	/* PC300/RSV or PC300/X21, 1 chan */
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/wan/wanxl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/wanxl.c b/drivers/net/wan/wanxl.c
index d4aab8a..9e2f084 100644
--- a/drivers/net/wan/wanxl.c
+++ b/drivers/net/wan/wanxl.c
@@ -811,7 +811,7 @@ static int __devinit wanxl_pci_init_one(struct pci_dev *pdev,
 	return 0;
 }
 
-static struct pci_device_id wanxl_pci_tbl[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(wanxl_pci_tbl) = {
 	{ PCI_VENDOR_ID_SBE, PCI_DEVICE_ID_SBE_WANXL100, PCI_ANY_ID,
 	  PCI_ANY_ID, 0, 0, 0 },
 	{ PCI_VENDOR_ID_SBE, PCI_DEVICE_ID_SBE_WANXL200, PCI_ANY_ID,
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/wan/pci200syn.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/pci200syn.c b/drivers/net/wan/pci200syn.c
index b595b64..5e2eccd 100644
--- a/drivers/net/wan/pci200syn.c
+++ b/drivers/net/wan/pci200syn.c
@@ -445,7 +445,7 @@ static int __devinit pci200_pci_init_one(struct pci_dev *pdev,
 
 
 
-static struct pci_device_id pci200_pci_tbl[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(pci200_pci_tbl) = {
 	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_PLX,
 	  PCI_DEVICE_ID_PLX_9050, 0, 0, 0 },
 	{ PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050, PCI_VENDOR_ID_PLX,
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and moves it to __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/wan/farsync.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c
index cf27bf4..db68596 100644
--- a/drivers/net/wan/farsync.c
+++ b/drivers/net/wan/farsync.c
@@ -530,7 +530,7 @@ static int fst_debug_mask = { FST_DEBUG };
 /*
  *      PCI ID lookup table
  */
-static struct pci_device_id fst_pci_dev_id[] __devinitdata = {
+static DECLARE_PCI_DEVICE_TABLE(fst_pci_dev_id) = {
 	{PCI_VENDOR_ID_FARSITE, PCI_DEVICE_ID_FARSITE_T2P, PCI_ANY_ID, 
 	 PCI_ANY_ID, 0, 0, FST_TYPE_T2P},
 
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and adds section attribute __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/e1000/e1000_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 0991648..542e0fa 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -53,7 +53,7 @@ static const char e1000_copyright[] = "Copyright (c) 1999-2006 Intel Corporation
   #define PCIE(x) x,
 #endif
 
-static struct pci_device_id e1000_pci_tbl[] = {
+static DECLARE_PCI_DEVICE_TABLE(e1000_pci_tbl) = {
 	INTEL_E1000_ETHERNET_DEVICE(0x1000),
 	INTEL_E1000_ETHERNET_DEVICE(0x1001),
 	INTEL_E1000_ETHERNET_DEVICE(0x1004),
-- 
1.5.4.3

[PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Makes struct pci_device_id array const and adds section attribute __devinitconst

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 drivers/net/e1000e/netdev.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index fc5c63f..acfc157 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4059,7 +4059,7 @@ static struct pci_error_handlers e1000_err_handler = {
 	.resume = e1000_io_resume,
 };
 
-static struct pci_device_id e1000_pci_tbl[] = {
+static DECLARE_PCI_DEVICE_TABLE(e1000_pci_tbl) = {
 	{ PCI_VDEVICE(INTEL, E1000_DEV_ID_82571EB_COPPER), board_82571 },
 	{ PCI_VDEVICE(INTEL, E1000_DEV_ID_82571EB_FIBER), board_82571 },
 	{ PCI_VDEVICE(INTEL, E1000_DEV_ID_82571EB_QUAD_COPPER), board_82571 },
-- 
1.5.4.3

Re: struct pci_device_id declaration cleanups

[Christoph Hellwig]

On Thu, Mar 06, 2008 at 11:55:38AM +0100, Jonas Bonn wrote:
> 
> This set of patches applies the macro DECLARE_PCI_DEVICE_TABLE to the 
> declarations of struct pci_device_id in several network drivers.  The drivers
> build cleanly with these patches (without any new warnings) on both x86 and 
> PowerPC.

Why would we want this macro?  It looks slightly ugly at least..

Re: struct pci_device_id declaration cleanups

[Jonas Bonn]

Christoph Hellwig wrote:
> On Thu, Mar 06, 2008 at 11:55:38AM +0100, Jonas Bonn wrote:
>> This set of patches applies the macro DECLARE_PCI_DEVICE_TABLE to the 
>> declarations of struct pci_device_id in several network drivers.  The drivers
>> build cleanly with these patches (without any new warnings) on both x86 and 
>> PowerPC.
> 
> Why would we want this macro?  It looks slightly ugly at least..
> 
> 

Basically because the alternative would be along the lines of:

static const struct pci_device_id fst_pci_dev_id[] __devinitconst = {

...and this was deemed to be even uglier when I first brought this up a 
while back.

/Jonas

Re: [PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Andrew Morton]

On Thu,  6 Mar 2008 11:55:39 +0100
Jonas Bonn <jonas@southpole.se> wrote:

> Makes struct pci_device_id array const and moves it to __devinitconst
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> ---
>  drivers/net/cassini.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
> index 14299f8..1451533 100644
> --- a/drivers/net/cassini.c
> +++ b/drivers/net/cassini.c
> @@ -233,7 +233,7 @@ static u16 link_modes[] __devinitdata = {
>  	CAS_BMCR_SPEED1000|BMCR_FULLDPLX /* 5 : 1000bt full duplex */
>  };
>  
> -static struct pci_device_id cas_pci_tbl[] __devinitdata = {
> +static DECLARE_PCI_DEVICE_TABLE(cas_pci_tbl) = {
>  	{ PCI_VENDOR_ID_SUN, PCI_DEVICE_ID_SUN_CASSINI,
>  	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0UL },
>  	{ PCI_VENDOR_ID_NS, PCI_DEVICE_ID_NS_SATURN,

Bah.

a) DECLARE_PCI_DEVICE_TABLE is misnamed.  It is used to *define* tables,
   not to declare them.  It should be called DEFINE_PCI_DEVICE_TABLE.

b) It's lame, anyway.  We could implement any number of such helper
   thingies, but we choose not to.


So I wouldn't go adding code which uses this thing until it has a correct
name, and until we've decided that we actually want to live with it.


Here's the easy part:

 Documentation/pci.txt |    4 ++--
 include/linux/pci.h   |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff -puN Documentation/pci.txt~rename-declare_pci_device_table-to-define_pci_device_table Documentation/pci.txt
--- a/Documentation/pci.txt~rename-declare_pci_device_table-to-define_pci_device_table
+++ a/Documentation/pci.txt
@@ -123,7 +123,7 @@ initialization with a pointer to a struc
 
 
 The ID table is an array of struct pci_device_id entries ending with an
-all-zero entry; use of the macro DECLARE_PCI_DEVICE_TABLE is the preferred
+all-zero entry; use of the macro DEFINE_PCI_DEVICE_TABLE is the preferred
 method of declaring the table.  Each entry consists of:
 
 	vendor,device	Vendor and device ID to match (or PCI_ANY_ID)
@@ -193,7 +193,7 @@ Tips on when/where to use the above attr
 	o Do not mark the struct pci_driver.
 
 	o The ID table array should be marked __devinitconst; this is done
-	  automatically if the table is declared with DECLARE_PCI_DEVICE_TABLE().
+	  automatically if the table is declared with DEFINE_PCI_DEVICE_TABLE().
 
 	o The probe() and remove() functions should be marked __devinit
 	  and __devexit respectively.  All initialization functions
diff -puN include/linux/pci.h~rename-declare_pci_device_table-to-define_pci_device_table include/linux/pci.h
--- a/include/linux/pci.h~rename-declare_pci_device_table-to-define_pci_device_table
+++ a/include/linux/pci.h
@@ -389,13 +389,13 @@ struct pci_driver {
 #define	to_pci_driver(drv) container_of(drv, struct pci_driver, driver)
 
 /**
- * DECLARE_PCI_DEVICE_TABLE - macro used to describe a pci device table
+ * DEFINE_PCI_DEVICE_TABLE - macro used to describe a pci device table
  * @_table: device table name
  *
  * This macro is used to create a struct pci_device_id array (a device table)
  * in a generic manner.
  */
-#define DECLARE_PCI_DEVICE_TABLE(_table) \
+#define DEFINE_PCI_DEVICE_TABLE(_table) \
 	const struct pci_device_id _table[] __devinitconst
 
 /**
_

Re: [PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Thu, 6 Mar 2008 15:41:50 -0800

> So I wouldn't go adding code which uses this thing until it has a correct
> name, and until we've decided that we actually want to live with it.

I personally think it's noise.

Re: [PATCH] net: use DECLARE_PCI_DEVICE_TABLE

[Jonas Bonn]

Andrew Morton wrote:
> Bah.
> 
> a) DECLARE_PCI_DEVICE_TABLE is misnamed.  It is used to *define* tables,
>    not to declare them.  It should be called DEFINE_PCI_DEVICE_TABLE.
> 

Fair enough...

> b) It's lame, anyway.  We could implement any number of such helper
>    thingies, but we choose not to.
> 

When I originally proposed these cleanups, I posted opened-coded 
versions.  That was deemed to be ugly and using a macro was suggested. 
So now I've reworked this with a macro, and now the macro is being 
deemed to be ugly, too!

The point of this exercise wasn't really to do a macro at all; the point 
was to get these structures marked 'const'  and into __devinitconst 
where they belong.  Whether the open-coded approach or the macro is the 
way to go, is all the same to me; any guidance on how to proceed would 
be appreciated.