On 29-09-08 18:34, Linus Torvalds wrote:
> On Mon, 29 Sep 2008, Bjorn Helgaas wrote:

quoted text
>>

>> +			if (!pci_resource_enabled(pdev, i))

>> +				continue;

>

> I really don't think this is the right approach.

>

> Maybe the PCI device has been turned off, but the *resource* may still be

> valid.

>

> Wouldn't it be much better to just check whether the resource is inserted

> in the resource tree or not?

>

> Quite frankly, it looks like your change will basically cause us to look

> over *every* system PnP resource, and for each of them, it will look at

> *every* PCI device, and for each PCI device it will look at *every* BAR,

> and for each BAR it finds it will read the PCI status register, over and

> over and over again.

>

> Now, I doubt you'll be able to wear out the PCI bus, but doesn't this just

> make you go "umm, that's not pretty, and it doesn't make much sense".

>

> If we've detected the PCI resource as being valid by the PCI layer, why

> not just use that information? And afaik, the easy way to check that is

> just whether it's inserted into the resource tree, which in turn is most

> trivially done by just checking whether the resource has a parent.

>

> IOW, why isn't it just doing

>

> 	struct resource *res = dev->resource[bar];

>

> 	if (!res->parent)

> 		continue;

>

> or something? Or what was wrong with just checking the res->start for

> being zero? Wherever PnP is relevant, resources that start at zero are

> disabled, no? 

I believe the possible issue is that resources that do _not_ (seem to)

start at zero might also be disabled.
Bjorn commented that pci_resource_start() returns a CPU address for I/O

which might not be the actual I/O address on some platforms. I haven't a

clue if that's actually possible "wherever PnP is relevent" as you put

it but that seems to otherwise make sense.
If it does though, it might for all I know also be possible to check

against some ARCH_SPECIFIC_INVALID_IO_ADDRESS instead of plain unadorned

0 (or just recheck the actual BAR again if not stored anywhere).
But that's the issue as I understood it: we might miss them on some

platforms if checking against 0...
Rene.

--

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