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

From: Segher Boessenkool <[email-masked]>
Date: Sat, 25 Aug 2007 11:49:58 +0200

> +		pci {
> +    			reg = <1 eec00000 40 1 ef400000 40>;  /* phb cfg, phb reg */

First component of reg is the unit address, so: pci@1eec00000 .

"phb cfg" is how you access PCI configuration space?  It wouldn't
hurt to document that, either in a little binding or just here in
the code.

> +			bus-range = <0 0>;

Can't you have subordinate PCI busses?  Or are there no slots :-)

> +			/*
> +			 * 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>;

Comment doesn't match code for the memory space.  What does "set
up indirectly" mean here?  Oh wait, you want to say that the host
addresses 1_8000_0000..1_8fff_ffff are translated to PCI addresses
8000_0000..8fff_ffff.

What about PCI DMA, is that identity mapped?

> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			#address-cells = <3>;

The reverse order of these is more conventional.  Not that it
actually matters ;-)

> +			compatible = "ibm, 440epx";

Stray space.  And you need to say it is the PCI host, so something
like "ibm,440epx-pci".

Segher