Re: [RFC/PATCH 4/4] [POWERPC] pci: Disable IO/Mem on a device when resources can't be allocated

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2007-12-18 at 12:56 +0300, Ivan Kokshaysky wrote:
> On Tue, Dec 18, 2007 at 10:01:15AM +1100, Benjamin Herrenschmidt wrote:
> > @@ -1040,7 +1040,10 @@ static inline void __devinit alloc_resou
> >  		r->flags |= IORESOURCE_UNSET;
> >  		r->end -= r->start;
> >  		r->start = 0;
> 
> Perhaps we should use IORESOURCE_UNSET universally... It's a lot better
> than clearing r->start which is in fact architecture dependent thing
> and in the end just destroys information for no purpose.

I'm not totally sure. I was actually tempted to switch powerpc to get
rid of it lately :-)

The problem is that a resource is never "unset" on PCI... the BAR always
contains something... the question is whether that something is going to
be problematic or not, or rather, whether that something can be
"allocated" (ie. fitted into the resource tree in a free spot or not).

And we have a way of knowing that already which is ... resource->parent
being NULL or not. That also happens to be what
pci_assign_unassigned_resources() uses and I think the various arch
pcibios_enable_device() should use (in fact I have a fix to make PowerPC
use that) instead of testing res->start or even IORESOURCE_UNSET. 

Anything that isn't in the resource tree is potentially stale and thus
mustn't be enabled (ie. IO/MEM decoding must not be enabled on a device
that has a BAR whose res->parent is NULL).

The reason why we introduced use of IORESOURCE_UNSET on powerpc goes
back from when we had our own resource assignment code there that was
somewhat a half-assed version of what pci_assign_unassigned_resources().
We decided to introduce IORESOURCE_UNSET rather than testing res->start
because it could cope with a value of 0 being valid, and on machines
with no legacy ISA stuff at all, 0 is a valid BAR value for some things,
and we weren't smart enough to reassign those things in all
circumstances.

Nowadays, that doesn't seem necessary anymore, and in fact, I think we
can still support those valid 0 BARs -and- not have IORESOURCE_UNSET by
standardizing on the idea that res->parent is the only indicator of a
resource validity... unless I'm missing something which is quite
possible :-)

Cheers,
Ben.