[CC trimmed a bit]
Jeff Garzik wrote:
> drivers/net/ne.c:
> * use probe_irq_on/off instead of autoirq_xxx (autoirq is going away)
Would be nice to get verification from someone with an older (non-PnP)
card that this doesn't break the irq detection. Is the game plan to
do similar for other drivers before 2.5.x as well?
> * request_region first thing in ne_probe1, before any hardware
> interaction takes place. Eliminates any potential resource races. Also
> eliminates a call to check_region.
Implementation note - For a lot of the drivers it makes sense to keep
around the resource returned by request_region so that we can set the
name after the probe has taken place (ie when we know what the card is.)
I've included a diff for drivers/net/wd.c as an example of what I
thought looks clean. If I know that the death of check_region is also
a pre 2.4.0 goal then I will supply a diff for the other 17 or so 8390
based drivers...
On a related note, there are a handful of drivers that register ioports
(and IRQ) with the name dev->name (e.g. eth0) as opposed to a more
meaningful model name (e.g. "3c503/16"). And some drivers do one for
ioports, and the other for IRQ!! We should aim for consistency on this,
and IMHO supplying the model name of the hardware is the more
informative of the two (and also in keeping with the rest of the
kernel drivers).
Oh, and while I'm asking about opinions on 2.4.0 goals, what about
the replacement of #ifdef MODULE driver init code with module_init(...)?
> As an aside, for 2.5.x, would it be possible to merge all the common
> ne2000 code (block_input/output, etc.) from the various ne2k drivers?
> As it stands there exists ne.c, ne2.c, ne2k-pci.c, and pcnet_cs.c, all
> of which do pretty much the same thing at their core. [and AFAICS, all
> but pcnet_cs might easily call a common ne2k library]
Possible - yes. Worthwhile - not so sure. As it is, 8390.o is the
source of much grief in drivers/net/Makefile from what I read recently ;-)
We will worry about 2.5.x later.
Paul.
--- 2400-t8/linux-g/drivers/net/wd.c~ Wed Jun 28 11:38:32 2000
+++ 2400-t8/linux-g/drivers/net/wd.c Tue Sep 19 03:29:52 2000
@@ -89,19 +89,33 @@
int __init wd_probe(struct net_device *dev)
{
int i;
+ struct resource *r;
int base_addr = dev ? dev->base_addr : 0;
- if (base_addr > 0x1ff) /* Check a single specified location. */
- return wd_probe1(dev, base_addr);
+ if (base_addr > 0x1ff) { /* Check a user specified location. */
+ r = request_region(base_addr, WD_IO_EXTENT, "wd-probe");
+ if ( r == NULL)
+ return -EBUSY;
+ i = wd_probe1(dev, base_addr);
+ if (i != 0)
+ release_resource(r);
+ else
+ r->name = ei_status.name;
+ return i;
+ }
else if (base_addr != 0) /* Don't probe at all. */
return -ENXIO;
for (i = 0; wd_portlist[i]; i++) {
int ioaddr = wd_portlist[i];
- if (check_region(ioaddr, WD_IO_EXTENT))
+ r = request_region(ioaddr, WD_IO_EXTENT, "wd-probe");
+ if (r == NULL)
continue;
- if (wd_probe1(dev, ioaddr) == 0)
+ if (wd_probe1(dev, ioaddr) == 0) {
+ r->name = ei_status.name;
return 0;
+ }
+ release_resource(r);
}
return -ENODEV;
@@ -268,8 +282,6 @@
}
/* OK, were are certain this is going to work. Setup the device. */
- request_region(ioaddr, WD_IO_EXTENT, model_name);
-
ei_status.name = model_name;
ei_status.word16 = word16;
ei_status.tx_start_page = WD_START_PG;
