[PATCH 1/2] ppc64: Block config accesses during BIST #3

[Brian King]

Thanks for taking a look at this. I'm currently looking at handling this
in the generic pci layer instead. I recently submitted a patch to 
linux-kernel to just this.

Thanks,

-Brian

Milton Miller wrote:
> Hi Brian.
> 
> Sorry it took so long to look at this, but I was totally burried 2 weeks
> ago and am just catching up with the fun stuff.
> 
> A few comments, mostly 1/2.
> 
> 0) line numbers are off after Anton's clean up pci controller allocation :)
> 1) I don't see any reason for adding exports pcibios_*config* routines
> 2) pci_ops should go in pci-bridge, its private to the arch.  Maybe call
>    this block_ops ?
> 3) in pci_bus_to_host don't worry about searching for the hose, Ben and
>    I agree the pci code will always have bus->sysdata when calling
>    config routines.  (pci_bus sysdata is copied form parent, and to pci_dev)
> 4) define a HAVE_PCI_CONFIG_BLOCK or something for the drivers to key on
> 5) Other places we return 1's we are careful to return the right number
>    of them (ie FF, FFFF, or FFFFFFFF depending on size).  (ok _PCI_NOP
>    doesn't even set the return).
> 
> And last but not least, how it is broken:
> 
> 6) There is a hose per phb not per bus.   You are not including the bus
>    number in your block table.  With a EADS bridge, you are blocking the
>    other slots on the same phb.
> 
> The real problem with number 6 is that (as mentioned in 5) the bus->sysdata
> is not always correct.   We currently use bus->sysdata 2 or 3 places in
> arch code, and then later explicitly copy the sysdata down from the phb
> (which I think is redundant with 2.6 pci code).   Actually we never use
> bus-sysdata except for host bridges (bus->self == NULL) but it is still
> copied to the pci devs on hotplug etc so it has to be compatable with 
> dev->sysdata aka a OF device node.  
> 
> The cleanest is to add this to struct pci_bus but we could also use a word
> in struct device_node (call to_OF_node on bus->self, sysdata if NULL) if
> needed.   We could also search a list of (bus, mask) pairs or something, but
> we don't use many of the 256 possible buses on each PHB.
> 
> milton
> 


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center