Maybe I will whip up a pci_simple helper function interface, while the
wheels turn about a more complete solution like pci-scan. For example
in a simple driver there is no need for a pci_id_info struct _and_ a
drv_id_info struct.
This simple interface would also be useful for the somewhat-common case
of a driver which must do a standard PCI probe (duplicating code), and
then something hairy and device-specific to the PCI registers. The VIA
IDE timings driver I am hacking on is an example of this.
Donald Becker wrote:
>
> On Sat, 11 Sep 1999, Jeff Garzik wrote:
>
> > Donald Becker wrote:
> > > There is very little that is network-specific about the code.
> > > It's certainly not "network centric".
> >
> > That was shorthand for
> > only-applicable-to-your-net-drivers-without-changes. Sorry. :) Or in
> > other words, using that code unchanged will cause problems in my
> > drivers. :)
>
> The pci-scan code doesn't need changes.
How does a driver define an arbitrary probe order under your scheme?
Forcing a driver to depend on the order of the values returned from a
pci_find_class() iteration is not always desireable.
> > > Does allow backwards- and forward-compatible drivers?
> > Nothing prevents the interface from being backported to older kernels
> > verbatim.
>
> Sure, you can "backport" the interface just by applying all the kernel
> patches to bring the kernel up to the latest version. That's not what I
> mean.
Keyword: verbatim. A driver using my PCI helper patch should be able
to recompile under 2.2.x or 2.0.x without changes. Only the helper
implementation would change. Although, my structure definitely needs a
'name' member, as older pci_dev structs did not have such a member.
> > The central problem is that the pci-netif.c code is not a drop-in
> > replacement for existing PCI probe code.
>
> Nothing will be a drop-in replacement. Most of the current scan code is
> ad hoc. Where it's similar, it's because the code semi-cribbed from some
> other driver, usually not a clean one.
My point was that pci-scan.c might have to change in order to be useful
to a complex driver using its API. In that respect pci-scan.c would not
be a drop-in replacement for some of my current PCI probing code which
gets duplicated across drivers.
> > However, if changing your code to be more generic means lots of private
> > development and testing, it will be ages before anyone but you can stop
> > duplicating code in their drivers.
>
> Yes, it has taken ages -- it's a derivative of the code I've been using and
> refining for years.
> The current version has
> documentation
> support for every kernel version with PCI functions (later 1.1.*, 2.[123].*)
> an annotated example driver
> over a dozen real driver ports
> a working interface to CardBus
>
> The comment about "changing .. to be more generic" is spurious. The only
> thing network-specific is the current file name (pci-netif.[ch] when it
> should be renamed pci-scan.[ch]), and a special case for if-name
> vs. major/minor numbers in cb_shim.c due to some ugliness in the CardBus
> interface. I'm the author or co-author on all but two of the CardBus
> client drivers, so I'm pretty that aspect is pretty well covered.
Make pci-scan useful (to me) means:
o don't printk if the PCI device I/O or mem region decoding is not
enabled
o allowing arbitrary probe order
o using void pointers to pass data to callbacks, allowing a driver to
avoid using globals
o supporting multiple base addresses
o using the new members in pci_dev instead of re-reading register values
o resource code hooks for MMIO regions, not just I/O addresses
o limiting the number of devices probed/registered to N
And small nit while I was looking at pci-scan: You need to use
IORESOURCE_IO not PCI_BASE_ADDRESS_SPACE_IO when testing the flags
member of 'struct resource' if I'm not mistaken.
> > I'm very interested in comments on the patch sent to you. I probably
> > need a drv_unregister in order to support CardBus. And I like your
> > handling of pciaddr/ioaddr, but that needs to be extended to support
> > multiple regions, and different schemes of using ioremap().
>
> NNnnnnoooooo....
>
> You have missed the point -- it's not supposed to be an overdesigned
> abstraction layer that handles every theorized combination. It's supposed
> to be a clean utility for real-life hardware, the vast majority of which
> needs only a single mapping and an IRQ. If it's more complex than that, it's
> likely to be so much more complex that it would require an elaborate
> description language.
Your code seems more complex than necessary for many drivers, which
often only do a simple PCI id check.
Your code seems inadequate for cases needing any of the features listed
above. For this case, generic code can be written to do the job, while
not needlessly duplicating code all over the kernel.
Making your code more generic decreases overall kernel code size, IMHO,
by reducing needless duplication of complex case handling.
> > And a comment from pci-netif.c:
> > > /* The PCI code in 2.2 is harder to use, and the extra complexity serves
> > > no real purpose. The resource code in 2.3 is far worse. It is a complex
> > > abstraction layer with negative benefit. */
> >
> > It is incredibly useful for video cards, and I'm thinking for my
> > framegrabber driver too. Easy sub-allocation of PCI resources is made
> > possible by the new scheme.
>
> It's over-engineered. It's an abstraction layer where there should be
> none. It abstracts out the essential differences, thus everyone must learn
> both the abstraction and the underlying details.
>
> A device driver writer needs to know addresses, the exact semantics of bus
> operations, and where the bits are going. Having everything be an abstract
> resource just makes life difficult.
The resource allocation code makes my multi-channel framegrabber driver
much simpler. A driver which needs implement malloc()-style
functionality for its on-board memory can now rely on generic code to
make its task much easier. Finding suitable space for a new channel
buffer becomes very easy. Video drivers can similarly benefit from the
new resource code, as their framebuffer memory has multitudes of uses
too.
> These chunks of code we are talking about are just utilities. They should
> make device drivers smaller and more approachable. In turn device drivers
> shouldn't be complete languages for describing every possible feature of a
> device. They should be the minimal code for setting up hardware to move
> bits around in a standard way for the OS. And to tie this into a favorite
> flame, the application user scanning the web for porn or bidding for beanie
> babies on eBay doesn't care that your OS allows building elaborate
> per-process viewpoints of the file system.
If it makes my code smaller and more simple, I care.
Regards,
Jeff
-- Custom driver development | Never worry about theory as long Open source programming | as the machinery does what it's | supposed to do. -- R. A. Heinlein- 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/