> > This has done to all of the PCI drivers. See
> > http://cesdis.gsfc.nasa.gov/linux/drivers/kern-2.3/index.html
> > ftp://cesdis.gsfc.nasa.gov/pub/linux/drivers/kern-2.3/*
>
> These use the new pci scan code when Linus picked Jeff Garzik's stuff instead
That pci scan code has significant limitations.
It doesn't handle the ACPI-D3 problem when warm-booting from Windows.
This generates a bunch of bug reports, and they all go to me.
It doesn't handle hot-swap PCI and CardBus. I know other people have
"claimed" hot-swap PCI as their project, but it's been working for years
with CardBus cards. The new structure allows a single driver binary to
support both normal and hot-swap cards, instead compiling a special
CardBus-only version of the driver.
And it doesn't help the cross-version compatibility, which no one except
users seems to care about.
> > The check_mem_region() functionality doesn't exist in older kernel version.
> > The work-around is to use check_region()/register_region() for both memory
> > and I/O spaces.
>
> This is potentially wrong for non x86 and wrong for 2.3.x though even if it
> is a sane hack for 2.2.x.
It's "potentially wrong" in theory for x86 as well, but it's always OK in
practice. I/O space and memory spaces are disjoint.
> > The problem also points out that the PCI driver probes are incorrectly
> > called a bunch of times at boot, and the structure has a foolish flaw that
> > limits "eth??" naming to only 16 cards.
>
> The eth?? thing is a bug. The multiple probe is a symptom of a bug in
> the driver. Suppose two drivers are on the machine for the same PCI card
> (eg if someone has decided to use 3coms card driver for the 3c90x and
> loads yours to get his 3c595 handled too).
There is still a buglet in the Space.c probe structure. It's calling the
PCI probe routines multiple times, when it should only try once. (I though
that this has been fixed.)
> Should be static or you get a symbol clash with 2 of the drivers compiled
> in.
> np = (void *)(((long)kmalloc(sizeof(*np), GFP_KERNEL) + 31) & ~31);
> memset(np, 0, sizeof(*np));
>
> The malloc isnt NULL checked, and the block isnt freed on unload.
This was a common buglet in many of the drivers -- it occured when I
converted them to always kmalloc() their dev->priv structure rather than
count on the init_etherdev(dev, sizeof(struct net_private)) function.
This results in a tiny one-time memory leak when a driver module is
removed. (Note: the much larger data buffers, typically 20-40KB total, are
checked for allocation success and released each time the interface is shut
down.)
Most drivers, and pci-skeleton.c, have been updates to avoid this. Only the
via-rhine.c and starfire.c drives have not been updated.
> So yes you are right about the vmalloc being my error. sorry. The others are
> long standing things I've reported before.
>
> Also notionally init_etherdev() could return NULL and should be checked. THat
> is something that needs propogating on since its more of a rule change than
> a bug.
OK, I'll make that change to pci-skeleton.c.
BTW, the old skeleton.c should be renamed isa-skeleton.c, then deleted.
Donald Becker
Scyld Computing Corporation, and
USRA-CESDIS, becker@cesdis.gsfc.nasa.gov
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/