Grant Likely wrote:
On Thu, Aug 07, 2008 at 11:36:25AM -0600, John Rigby wrote:
Modify mpc83xx_add_bridge to get config space register base address from the
device
tree instead of immr + hardcoded offset.
83xx pci nodes have these changes:
register properties now contain two address length tuples:
First is the pci bridge register base, this has always been there.
Second is the config base, this is new.
The primary pci bus should have the "primary" property.
These are documented in Documentation/powerpc/dts-bindings/fsl/83xx-512x-pci.txt
Looks mostly good to me. I only have one comment on the device tree
binding...
diff --git a/Documentation/powerpc/dts-bindings/fsl/83xx-512x-pci.txt
b/Documentation/powerpc/dts-bindings/fsl/83xx-512x-pci.txt
new file mode 100644
index 0000000..51214a0
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/fsl/83xx-512x-pci.txt
@@ -0,0 +1,43 @@
+* Freescale 83xx and 512x PCI bridges
+
+Freescale 83xx and 512x SOCs include the same pci bridge core.
+
+83xx/512x specific notes:
+- reg: should contain two address length tuples
+ The first is for the internal pci bridge registers
+ The second is for the pci config space access registers
+- primary:
+ This property should be present for the primary pci bridge
Can you use something like 'fsl,primary-pci-bridge' instead? 'primary'
is a little too generic for my taste. Also, the purpose of identifying
one of the PCI bridges as primary should be documented (This is me
pushing against encoding Linux internal implementation details into the
device tree, I suspect that 'primary' doesn't belong in the device tree
at all).
Ok, I got the primary idea from sam440ep.dts, I'm willing to do
something different.
I have thought about adding an is_primary argument to mpc83xx_add_bridge
like fsl_add_bridge has and make the callers figure out which is primary.
The simple case is the platform that have only one bus:
for_each_compatible_node(np, "pci", "fsl,mpc8540-pci")
fsl_add_bridge(np, 1);
Callers with multiple bridges do something like this:
for_each_compatible_node(np, "pci", "fsl,mpc8641-pcie") {
struct resource rsrc;
of_address_to_resource(np, 0, &rsrc);
if ((rsrc.start & 0xfffff) == 0x8000)
fsl_add_bridge(np, 1);
else
fsl_add_bridge(np, 0);
}
So now we are using hardcoded offsets again.
As I type this I'm coming to like the primary flag more and more. I'm
willing to change it to 'fsl,primary-pci-bridge' though I don't really
agree with your argument against the generic name. It is in the context
of an 'fsl,pci-whatever' node so it does not have to be specific. And
maybe generic is good, a universal generic solution would be to require
all primary pci nodes to have the property then
pci_process_bridge_OF_ranges could get it out of the device node instead
of having it passed in.
I would like to hear what the 83xx folks think about this.
John
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
