Re: [PATCH 1/3 v3] Add StorCenter DTS first draft.

[Jon Loeliger <jdl@freescale.com>]

Grant Likely wrote:
> On 1/22/08, Jon Loeliger <jdl@jdl.com> wrote:
>> Based on the Kurobox DTS files.
>>
>> Signed-off-by: Andy Wilcox <andy@protium.com>
>> Signed-off-by: Jon Loeliger <jdl@jdl.com>
> 
> Comments below
> 
>> +
>> +/ {
>> +       model = "StorCenter";
>> +       compatible = "storcenter";
> 
> Be specific!  "iomega,storcenter".  Even better if you put in the model number.

As I mentioned vefore, there is no further model number.
That _is_ the model name.

>> +
>> +       soc@fc000000 {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               device_type = "soc";
> 
> device_type should be dropped (but I know that requires changes to the
> existing mpc82xx support code).

And when the code is fixed, we can fix the DTS too... :-)

>> +               compatible = "fsl,mpc8241", "mpc10x";
> 
> fsl,mpc8241-immr would be better; this node describes the internally
> memory mapped registers; not the entire soc.

Uh, whatever? :-)  'Cuz how many other DTS files say that?

>> +
>> +               mpic: interrupt-controller@40000 {
>> +                       #interrupt-cells = <2>;
>> +                       #address-cells = <0>;
> 
> Is #address-cells needed?  There are no child nodes.

I thought so.  Could be wrong.

>> +       chosen {
>> +               linux,stdout-path = "/soc/serial@4500";
> 
> /soc@fc000000/ perhaps?

Not really necessary to specify the unit number.

jdl