Re: [PATCH 2/3] [POWERPC] Add pci node to sequoia dts

[Stefan Roese <sr@denx.de>]

On Monday 27 August 2007, David Gibson wrote:
> On Sat, Aug 25, 2007 at 01:29:54PM +0400, Vitaly Bordug wrote:
> > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > Signed-off-by: Stefan Roese <sr@denx.de>
> >
> > ---
> >
> >  arch/powerpc/boot/dts/sequoia.dts |   26 ++++++++++++++++++++++++++
> >  1 files changed, 26 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/sequoia.dts
> > b/arch/powerpc/boot/dts/sequoia.dts index ef6f41c..8eb258f 100644
> > --- a/arch/powerpc/boot/dts/sequoia.dts
> > +++ b/arch/powerpc/boot/dts/sequoia.dts
> > @@ -92,6 +92,32 @@
> >  		#size-cells = <1>;
> >  		ranges;
> >  		clock-frequency = <0>; /* Filled in by zImage */
> > +
> > +		pci {
> > +			/* irqs are routed to irq67, dependless of devsel/PIRQx */
> > +			interrupt-map-mask = <0 0 0 0>;
> > +			interrupt-map = <0 0 0 0 &UIC2 3 8>;
> > +
> > +			interrupt-parent = <&UIC2>;
> > +			interrupts = <3 8>;
> > +
> > +			bus-range = <0 0>;
> > +
> > +			/*
> > +			 * mem is at 80000000 set up indirectly
> > +			 * io is at 0001_e800_0000
> > +			 */
> > +			ranges = <02000000 0 80000000 1 80000000 0 10000000
> > +				01000000 0 00000000 1 e8000000 0 00100000>;
> > +
> > +			#interrupt-cells = <1>;
> > +			#size-cells = <2>;
> > +			#address-cells = <3>;
> > +
> > +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */
> > +			compatible = "ibm, 440epx";
> > +			device_type = "pci";
>
> I usually put device_type, compatible and reg at the top of the block,
> to announce what the node actually is before giving all the details.
>
> Also, apart from the stray space in the compatible, I'm guessing that
> the 440EPx bridge is actually more-or-less like the PCI bridges on
> other 4xx chips, so we should have a more general compatible string
> too.

Yes, it is "more-or-less" like any other 4xx PCI core. So it really would make 
sense to define it more generally. Something like:

			compatible = "ibm,pci-440epx", "ibm,pci4xx";

or even:

			compatible = "ibm,pci-440epx", "ibm,pci";

?

> Is the 440EPx a vanilla PCI or a PCI-X bridge?  If the later that
> should be reflected in the name and compatible as well.

It's a vanilla PCI bridge.

Best regards,
Stefan