lkml.org 
[lkml]   [1999]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] pci probing
Donald Becker wrote:
>
> On Sat, 11 Sep 1999, Jeff Garzik wrote:
>
> > As this discussion continues, the more I think that PCI drivers need a
> > SIMPLE probe interface, more like my original patch, which just scans
> > for a list of PCI ids, and calls a callback for each successful id
> > match.
>
> 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)'.


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

> > The PCI code already takes care of
> > the latency problem that Alan mentioned, in response to my original
> > patch.
>
> Not on older kernel versions.

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)


> > 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.
>
> There is a fundamental conflict in specifying device order and supporting
> dynamically inserted devices.

Probe order and device registration order may matter for any number of
reasons.


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

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.

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/

\
 
 \ /
  Last update: 2005-03-22 13:53    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site