Messages in this thread | | | Date | Mon, 11 Dec 2006 17:27:37 +0300 | From | Ivan Kokshaysky <> | Subject | Re: pci_assign_resource() inconsistency |
| |
On Mon, Dec 11, 2006 at 02:47:55PM +1100, Benjamin Herrenschmidt wrote: > So at first, an unassigned resource has the IORESOURCE_UNSET flag set > (or is supposed to). pci_assign_resource() itself will clear that flag > if it succeeds. > > However, pretty much nothing else checks that flag, so it's mostly > useful.
I doubt of the generic usefulness of that flag, as it would be mere equivalent of resource->parent == NULL. I think the IORESOURCE_UNSET flag came from PnP and PCI subsystem has never used it.
> Now, we have drivers/pci/setup-bus.c doing: > > if (pci_assign_resource(list->dev, idx)) { > res->start = 0; > res->end = 0; > res->flags = 0; > } > > So it basically destroys the resource content utterly when > pci_assign_resource() fails...
Yes, it's ugly. But, IIRC, this was the only way to prevent some drivers and arch code from using an unassigned resource...
> There are questions raised here: > > - Shouldn't we instead fix things so that instead, we properly > test for IORESOURCE_UNSET in pci_request_* & friends and just have > pci_assign_resource() continue as it's doing now, that is not clear that > flag if the assignment fails ? > > - setup-bus.c is a bit violent: As soon as it hits a p2p bridge, it > will bluntly re-assign everybody, not trying to check wether a resource > was already correctly assigned by the firmware or not. However, it never
No - it checks the resource->parent and doesn't touch the resources which are already assigned.
> sets IORESOURCE_UNSET. Thus if we do the above, we should probably have > it always set that bit before calling pci_assign_resource()... > > Now the question is, what should I do in pci_32.c ... right now, we > unconditionally clear IORESOURCE_UNSET, which isn't very correct, then > call pci_assign_resource(). > > Should I do like the setup-bus.c and just completely wipe the resource > if pci_assign_resource() fail ? Or should I just stop clearing > IORESOURCE_UNSET (and thus rely on pci_assign_resource() to clear it > only if it succeeds, which seems to work) in which case I see no point > in making that function much check since there is nothing useful to do > when it fails and it does printk already.
Well, as IORESOURCE_UNSET usage seems to be an exclusive PPC32 thing, there is no much difference. ;-) Though I agree that ignoring the return value of pci_assign_resource() is entirely valid in most cases.
Ivan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |