[PATCH] Fixup warning for acenic

[PATCH] Fixup warning for acenic

[Martin J. Bligh]

OK, it's war on warnings hour. Get this from acenic,

drivers/net/acenic.c:135: warning: `acenic_pci_tbl' defined but not used

And indeed it doesn't *seem* to be used (though I'm less than confident
about that) ... can we just rip it out? Or should this be wrapped in
#ifdef MODULE or something (I'm compiling it in)?

M.

diff -urpN -X /home/fletch/.diff.exclude virgin/drivers/net/acenic.c acenic_fix/drivers/net/acenic.c
--- virgin/drivers/net/acenic.c	Wed Mar  5 07:37:02 2003
+++ acenic_fix/drivers/net/acenic.c	Wed Mar 19 23:44:28 2003
@@ -131,34 +131,6 @@
 #define PCI_DEVICE_ID_SGI_ACENIC	0x0009
 #endif
 
-#if LINUX_VERSION_CODE >= 0x20400
-static struct pci_device_id acenic_pci_tbl[] __initdata = {
-	{ PCI_VENDOR_ID_ALTEON, PCI_DEVICE_ID_ALTEON_ACENIC_FIBRE,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_ALTEON, PCI_DEVICE_ID_ALTEON_ACENIC_COPPER,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_3COM, PCI_DEVICE_ID_3COM_3C985,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_NETGEAR, PCI_DEVICE_ID_NETGEAR_GA620,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_NETGEAR, PCI_DEVICE_ID_NETGEAR_GA620T,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	/*
-	 * Farallon used the DEC vendor ID on their cards incorrectly,
-	 * then later Alteon's ID.
-	 */
-	{ PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_FARALLON_PN9000SX,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_ALTEON, PCI_DEVICE_ID_FARALLON_PN9100T,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ PCI_VENDOR_ID_SGI, PCI_DEVICE_ID_SGI_ACENIC,
-	  PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_NETWORK_ETHERNET << 8, 0xffff00, },
-	{ }
-};
-MODULE_DEVICE_TABLE(pci, acenic_pci_tbl);
-#endif
-
-
 #ifndef MODULE_LICENSE
 #define MODULE_LICENSE(a)
 #endif

Re: [PATCH] Fixup warning for acenic

[Jes Sorensen]

>>>>> "Martin" == Martin J Bligh <mbligh@aracnet.com> writes:

Martin> OK, it's war on warnings hour. Get this from acenic,
Martin> drivers/net/acenic.c:135: warning: `acenic_pci_tbl' defined
Martin> but not used

Martin> And indeed it doesn't *seem* to be used (though I'm less than
Martin> confident about that) ... can we just rip it out? Or should
Martin> this be wrapped in #ifdef MODULE or something (I'm compiling
Martin> it in)?

Hi Martin,

The table isn't used at the moment, however at some point it should
be. I have been reluctant to change the driver over to the new hotplug
scheme since in the past there has been a significant number of AceNIC
users running on older kernels which do not have the hotplug
infrastructure (2.4.9 etc). On the other hand I don't remember hearing
from a single person who wanted to use AceNIC in a hotplug
environment.

Anyway most people seems to be at 2.4.18 or later these days so maybe
it is time to retire the old code. I will try to find time to take a
look at it (or if someone beats me to it and sends me a patch ... ;-)

Cheers,
Jes

PS: There is an email address listed in the driver, please CC patches
to it.

Re: [PATCH] Fixup warning for acenic

[Christoph Hellwig]

On Thu, Mar 20, 2003 at 10:47:49AM -0500, Jes Sorensen wrote:
> be. I have been reluctant to change the driver over to the new hotplug
> scheme since in the past there has been a significant number of AceNIC
> users running on older kernels which do not have the hotplug
> infrastructure (2.4.9 etc). On the other hand I don't remember hearing
> from a single person who wanted to use AceNIC in a hotplug
> environment.

2.4.9 of course has the newstyle pci interface! And actual hotplug
PCI support also is in all today singnificant 2.4.9 forks (RH..).

There's even some shim to emulate the pci_driver style interface on
2.2.

Anyway, this table has another use, it's used by userland ools like
installers for selecting the right driver for a given pci device.  So
even if it seems unused from kernelspace it has a use.

Re: [PATCH] Fixup warning for acenic

[Martin J. Bligh]

> 2.4.9 of course has the newstyle pci interface! And actual hotplug
> PCI support also is in all today singnificant 2.4.9 forks (RH..).
> 
> There's even some shim to emulate the pci_driver style interface on
> 2.2.
> 
> Anyway, this table has another use, it's used by userland ools like
> installers for selecting the right driver for a given pci device.  So
> even if it seems unused from kernelspace it has a use.

Are they kmem diving? Or parsing source code? 

Thanks,

M.

Re: [PATCH] Fixup warning for acenic

[Greg KH]

On Thu, Mar 20, 2003 at 08:11:58AM -0800, Martin J. Bligh wrote:
> > 2.4.9 of course has the newstyle pci interface! And actual hotplug
> > PCI support also is in all today singnificant 2.4.9 forks (RH..).
> > 
> > There's even some shim to emulate the pci_driver style interface on
> > 2.2.
> > 
> > Anyway, this table has another use, it's used by userland ools like
> > installers for selecting the right driver for a given pci device.  So
> > even if it seems unused from kernelspace it has a use.
> 
> Are they kmem diving? Or parsing source code? 

They are looking at the modules.pcimap files which are generated from
depmod, which pull it from the object files.

greg k-h

Re: [PATCH] Fixup warning for acenic

[Martin J. Bligh]

>> > 2.4.9 of course has the newstyle pci interface! And actual hotplug
>> > PCI support also is in all today singnificant 2.4.9 forks (RH..).
>> > 
>> > There's even some shim to emulate the pci_driver style interface on
>> > 2.2.
>> > 
>> > Anyway, this table has another use, it's used by userland ools like
>> > installers for selecting the right driver for a given pci device.  So
>> > even if it seems unused from kernelspace it has a use.
>> 
>> Are they kmem diving? Or parsing source code? 
> 
> They are looking at the modules.pcimap files which are generated from
> depmod, which pull it from the object files.

Hmmm ... so we're going to get a compiler warning for every hotpluggable
driver? or is this just an error in the way acenic does it? Seems like
the latter, but it's not clear to me how to fix it up properly ...
If it's the former, we need a better solution ;-)

M.

Re: [PATCH] Fixup warning for acenic

[Jeff Garzik]

On Thu, Mar 20, 2003 at 11:13:43AM -0800, Martin J. Bligh wrote:
> Hmmm ... so we're going to get a compiler warning for every hotpluggable
> driver?

Only for the ones that do not actually use the module_device_table,
which is an unusual case.

Adam Richter(sp?) went through and added PCI module_device_tables to
drivers which do not use the PCI API.  From the programmer's standpoint
this looks like dead code -- but it's actually very useful, because it
exports the PCI ids to userspace in
/lib/modules/`uname -r`/modules.pcimap, where installers, hardware
configurators, and other tools pick up the data and do something useful
with it.

	Jeff

Re: [PATCH] Fixup warning for acenic

[Greg KH]

On Thu, Mar 20, 2003 at 11:13:43AM -0800, Martin J. Bligh wrote:
> >> > 2.4.9 of course has the newstyle pci interface! And actual hotplug
> >> > PCI support also is in all today singnificant 2.4.9 forks (RH..).
> >> > 
> >> > There's even some shim to emulate the pci_driver style interface on
> >> > 2.2.
> >> > 
> >> > Anyway, this table has another use, it's used by userland ools like
> >> > installers for selecting the right driver for a given pci device.  So
> >> > even if it seems unused from kernelspace it has a use.
> >> 
> >> Are they kmem diving? Or parsing source code? 
> > 
> > They are looking at the modules.pcimap files which are generated from
> > depmod, which pull it from the object files.
> 
> Hmmm ... so we're going to get a compiler warning for every hotpluggable
> driver? or is this just an error in the way acenic does it?

The fact that acenic doesn't reference the structure, like other pci
drivers do, is the reason why the warning is there.  Once it switches to
use the "new"[1] pci api, the warning will go away.

thanks,

greg k-h

[1] What, isn't it like 2 years old now?

Re: [PATCH] Fixup warning for acenic

[Matthew Wilcox]

Please don't delete this table.  At some point when Jes gets his head
out of the "must support Linux 1.2" space, this table will be used and
then this driver will support hotplugging.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [PATCH] Fixup warning for acenic

[Martin J. Bligh]

> Please don't delete this table.  At some point when Jes gets his head
> out of the "must support Linux 1.2" space, this table will be used and
> then this driver will support hotplugging.

Fair enough ... but can we wrap it in CONFIG_SOMETHING? or #if 0 ?

M.

Re: [PATCH] Fixup warning for acenic

[Matthew Wilcox]

On Thu, Mar 20, 2003 at 07:50:55AM -0800, Martin J. Bligh wrote:
> > Please don't delete this table.  At some point when Jes gets his head
> > out of the "must support Linux 1.2" space, this table will be used and
> > then this driver will support hotplugging.
> 
> Fair enough ... but can we wrap it in CONFIG_SOMETHING? or #if 0 ?

If you must, you could wrap it in MODULE.  I don't see the value in
removing every single warning from the kernel build.  If you're intent on
chasing all these pointless things, try installing gcc 3.3 and compiling
a kernel with that.  It'll pump out more warnings than you can shake
a pointy stick at.  Or turn on -W with gcc 2.96 -- it has much the
same effect.  I made an effort to remove some of the -W warnings from
the header files a while ago so I could compile individual files with
-W as I find it tends to point out some mistakes I often make.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [PATCH] Fixup warning for acenic

[Martin J. Bligh]

>> > Please don't delete this table.  At some point when Jes gets his head
>> > out of the "must support Linux 1.2" space, this table will be used and
>> > then this driver will support hotplugging.
>> 
>> Fair enough ... but can we wrap it in CONFIG_SOMETHING? or #if 0 ?
> 
> If you must, you could wrap it in MODULE.  I don't see the value in
> removing every single warning from the kernel build.  If you're intent on

The main value is that it makes it a damned sight easier to catch *real*
errors and warnings.

> chasing all these pointless things, try installing gcc 3.3 and compiling
> a kernel with that.  It'll pump out more warnings than you can shake
> a pointy stick at.  Or turn on -W with gcc 2.96 -- it has much the
> same effect.  I made an effort to remove some of the -W warnings from
> the header files a while ago so I could compile individual files with
> -W as I find it tends to point out some mistakes I often make.

I refuse to use anything so much massively slower than 2.95, when it
produces worse code ;-) 3.3 might be better (I suppose) but is too 
experimental for my taste.

M.