Re: [PATCH v2 11/12] [POWERPC] Promess Motion-PRO DTS

[Marian Balakowicz <m8@semihalf.com>]

David Gibson wrote:
> On Sun, Nov 04, 2007 at 12:53:17AM +0100, Marian Balakowicz wrote:
>> Add device tree source file for Motion-PRO board.
> [snip]
>> +		motionpro-statusled@660 {	// Motion-PRO status LED
>> +			compatible = "promess,motionpro-statusled";
>> +			cell-index = <6>;
>> +			reg = <660 10>;
>> +			interrupts = <1 f 0>;
>> +			interrupt-parent = <&mpc5200_pic>;
>> +			blink-delay = <64>; // 100 msec
>> +		};
>> +
>> +		motionpro-readyled@670 {	// Motion-PRO ready LED
>> +			compatible = "promess,motionpro-readyled";
>> +			cell-index = <7>;
> 
> These cell-index values for the LEDs look very strange.  How are they
> used?
[snip]
>> +
>> +		mscan@980 {
>> +			compatible = "mpc5200b-mscan","mpc5200-mscan";
>> +			cell-index = <1>;
> 
> As for serial and gpt, is cell-index really suitable here?

Ok, removed those unused "cell-index" properties.

> [snip]
>> +		spi@f00 {
>> +			device_type = "spi";
>> +			compatible = "mpc5200b-spi","mpc5200-spi";
>> +			reg = <f00 20>;
>> +			interrupts = <2 d 0 2 e 0>;
>> +			interrupt-parent = <&mpc5200_pic>;
>> +		};
> [snip]
>> +		// PSC2 in spi master mode 
>> +		spi@2200 {		// PSC2
>> +			device_type = "spi";
>> +			compatible = "mpc5200b-psc-spi","mpc5200-psc-spi";
>> +			cell-index = <1>;
> 
> cell-index present for one spi, but not the other makes be even more
> suspicious about it's applicability here...

These are two different SPIs, both are part of the SoC but first one
is dedicated SPI interface while the second one is PSC port in a SPI
mode. Driver for the second one (mpc52xx_psc_spi.c) is actually using
cell-index to get the PSC port number it's controlling.

> [snip]
>> +	lpb {
>> +		model = "fsl,lpb";
>> +		compatible = "fsl,lpb";
> 
> Is lpb another one of these chipselect/offset configurable bus bridge
> things?  If so, you should use a 2-cell addressing convention for the
> subnodes like fsl localbus and 4xx EBC.

Yes, it is, so I have switched to 2-cell addressing. Please see the v3
respin of the patches.


>> +		// 8-bit custom Anybus Module on LocalPlus Bus CS3
>> +		anybus50020000 {
> 
> Missing '@'.

Fixed.

Thanks,
m.