On Tuesday 30 September 2008 10:29:44 am Linus Torvalds wrote:
>
> On Tue, 30 Sep 2008, Rene Herman wrote:
> >
> > > Of course, it may be that the PnP code runs too early, and we have only
> > > parsed the PCI resources, not inserted them into the resource tree yet. If
> > > so, none of this will work, of course.
> >
> > It doesn't. With the test negated it triggers for all PCI resources (and
> > ofcourse my soundcard driver fails again).
>
> Oh, ok. Looking at it, it does seem that we actually _insert_ the PCI
> resources too late. We do it in pcibios_allocate_resources(), and there we
> even take care to look at whether it was enabled or disabled (we
> prioritize enabled resources, so that a disabled one will never be
> requested before an enabled one and if they clash, it's always the
> disabled one that loses the resource).
>
> But pcibios_allocate_resources() is called from pcibios_resource_survey(),
> which is called from pcibios_init(), which in turn is caled from
> pci_subsys_init() that is a "subsys_initcall()".
>
> In contrast, the PnP fixup thing is called from pnp_fixup_device, called
> from __pnp_add_device(), called from pnp_add_device() (and
> pnp_add_card(), but that should be later), and those in turn from
> pnpacpi_add_device and pnpacpi_init().
>
> And pnpacpi_init is _also_ a subsys_initcall, but arch/x86/pci/built-in.o
> gets linked in _after_ drivers/built-in.o. That, in turn, is because it's
> marked as a "driver" in the x86 Makefile, and the main Makefile actually
> ends up forcing "drivers-y" to have drivers/ first.
>
> Just for fun, does this patch make a difference and allow you to just
> take the "is it registered" thing into account?
>
> It's a scary change right now, and I wouldn't commit it as is (I think
> that for 2.6.27 the thing to do is to just do the minimal "zero means
> disabled" thing), but having some random driver level initialize before
> the core architecture-specific PCI code does smell. So something like this
> sounds conceptually right anyway.
Sorry for the delay in responding -- I'm officially on vacation
yesterday and today.
I agree that for 2.6.27 the "res->start == 0 means disabled" check
in the PNP quirk seems safest.
I don't like it long-term, because (a) I'd like that knowledge to at
least be in PCI, not PNP, and (b) res->start is the CPU address, not
necessarily the PCI bus address, and the BAR value (== PCI bus address)
is what we're trying to check. Even if we looked at the BAR value
instead of res->start, I think zero is a valid PCI bus address on
machines where the root bridge applies an address offset.
So something like the patch below (same as what Rene originally
proposed, I think)?
diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index 0bdf9b8..b3319e4 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -254,6 +254,10 @@ static void quirk_system_pci_resources(struct pnp_dev *dev)
pci_start = pci_resource_start(pdev, i);
pci_end = pci_resource_end(pdev, i);
+
+ if (!pci_start)
+ continue;
+
for (j = 0;
(res = pnp_get_resource(dev, type, j)); j++) {
if (res->start == 0 && res->end == 0)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
