Re: [patch] pci probing

Donald Becker (becker@tidalwave.net)
Sun, 12 Sep 1999 16:40:41 -0400 (EDT)


On Sun, 12 Sep 1999, Jeff Garzik wrote:
> Donald Becker wrote:
> > This isn't a useful interface for CardBus/hot-swap-PCI, which means that
> > there will be two interfaces.
>
> Right, (a) a tiny PCI probe w/ callback, something easily dropped into
> existing drivers, and (b) a big meaty routine like pci-scan or my patch
> which takes care of ifdef trees.
>
> 'sizeof(struct pci_simple_probe_entry)' would be a lot smaller than
> 'sizeof(struct pci_id_info) + sizeof(struct drv_id_info)'.

Not by much.
The reason there are two structures in my scheme is because the table
entries may be marked __initdata in a few cases. Although when a driver
supports CardBus/hot-swap-PCI/docking-stations, __initdata rarely applies.

> > > Most PCI drivers I've seen don't need to worry about subsystem
> > > ids, for example, and only driver I've seen needed to worry about
> > > chipset revision _at probe time_.
> > The subsystem IDs check is needed.
> But not for the common case. I actually counted in drivers/*.c files,
> and the number of drivers which do not need to do this outnumber the
> ones that do. (though the number that do subsys id checks is not at all
> insignificant)

There are many cases where I would have used subsystem ID if it has been
easy to do. Instead I used some other indicator, such as EEPROM contents.

> My point was that pci-scan flag PCI_NO_MIN_LATENCY indicates that the
> default is to set a minimum latency -- something which pci_set_master()
> already does. That block of code is not needed. Plus storing
> 'min_pci_latency' as a global in pci-scan.c makes this solution less
> granular. (ie. it might be useful to set a per-device minimum PCI
> latency like I demonstrate in my patch)

This has already been discussed. Most device need only a reasonable
non-zero latency setting, not a specific one. A zero latency setting is the
result of buggy older BIOSes, and drivers have individual fix-ups because
older kernels didn't provide a way to do this.

An example of boards that are exceptions are the 3c590 and 3c595, which must
have their latency register set to 248 to avoid a chip bug. Other devices
supported by the same driver don't have a specific minimum setting. It's
reasonable, and more obviously a bug-work-around, to have the driver do a
rare patch-up such as this.

[[ Note: This work-around has no operational impact. The ancient 3c590
series cannot do a long enough burst to hog the PCI bus. ]]

> > You patch means the driver still needs an #ifdef tree for many aspects of
> > the PCI scan interaction.
>
> Sure. That's why we're having this discussion. Or at least why I'm
> having it. :) To iron out details on a good pci-scan implementation.
> Both patches should use __request_region and ioremap for all flagged
> base addresses, for example. That would clear out a lot of ifdefs.

My approach specifically does *not* do request_region(). That occurs only
when the device is recognized and named. For network drivers that's the
"eth0" name, which isn't known at probe1() time. In some cases a probe1()
routine will reject the device, and the region shouldn't be registered.

> I outlined in my previous mail why your implementation is inadequate for
> some cases, which IMHO are becoming more common at new devices are
> released. For example, multiple PCI base addresses are becoming more
> and more common. Why not handle all the base addresses in a loop,
> instead of just simply handling the first one? Why not also add
> __request_region code also? That's a source of ifdefs. etc.

There are nightmare devices that use all five base address registers. But
they usually full of other nightmare design decisions that prevents any
regular scheme from working. Most non-bridge devices have matching memory
and I/O space mappings, and sometime use the boot ROM register. I see fewer
exceptions to this than when PCI was new.

[[ In fact the only unusual mapping I've seen recently is the new HCF soft
modem. The primary access is through a memory region, and an 8 byte I/O
region is provided to emulate a regular serial port. Accesses to the I/O
space causes a PCI interrupt, and the driver writes the updated 16550-like
register values. This allows a MS-DOS driver to be written, removing our
primary "does it work in MS-DOS" identification for a Winmodem. But,
ignoring all of that, a Linux driver would still use only a single PCI
mapping. ]]

Donald Becker, becker@tidalwave.net
Chief Technology Officer, Scyld Computing Corporation
240-463-0035 (voice) Annapolis MD 21403

-
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/