Freescale MPC5554 device tree (was: cross-compiling Linux for PowerPC e200 core?)

[Grant Likely]

2010/3/11 Németh Márton <nm127 at freemail.hu>:
> Hi,
>
> thank you for the comments, I reworked the Freescale MPC5554 device tree
> accordingly. I'm listening for comments on this draft.
>
> Regards,
>
>        Márton Németh
>
> ---
> From: Márton Németh <nm127 at freemail.hu>
>
> Add device tree for Freescale MPC5554.
>
> Signed-off-by: Márton Németh <nm127 at freemail.hu>
> ---
> diff -uprN linux-2.6.33.orig/arch/powerpc/boot/dts/mpc5554.dts linux/arch/powerpc/boot/dts/mpc5554.dts
> --- linux-2.6.33.orig/arch/powerpc/boot/dts/mpc5554.dts 1970-01-01 01:00:00.000000000 +0100
> +++ linux/arch/powerpc/boot/dts/mpc5554.dts     2010-03-12 07:22:37.000000000 +0100
> @@ -0,0 +1,189 @@
> +/*
> + * Freescale MPC5554 Device Tree Source
> + *
> + * Based on MPC5553/5554 Microcontroller Reference Manual, Rev. 4.0, 04/2007
> + * http://www.freescale.com/files/32bit/doc/ref_manual/MPC5553_MPC5554_RM.pdf
> + *
> + * Copyright 2010 Márton Németh
> + * Márton Németh <nm127 at freemail.hu>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +       model = "MPC5554";
> +       compatible = "fsl,MPC5554EVB";          // Freescale MPC5554 Evaluation Board
> +       #address-cells = <1>;
> +       #size-cells = <1>;

also need: interrupt-parent = <&intc>;

I describe why later...

> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               cpu at 0 {
> +                       device_type = "cpu";
> +                       compatible = "PowerPC,5554";

I'd rather see the same convention used here as for all the other
compatible values in this file.  ie:

compatible = "fsl,mpc5554-e200z6", "fsl,powerpc-e200z6";

Dave, what do you think?

> +                       reg = <0>;
> +                       d-cache-line-size = <32>;
> +                       i-cache-line-size = <32>;
> +                       d-cache-size = <0x8000>;        // L1, 32KiB
> +                       i-cache-size = <0x8000>;        // L1, 32KiB
> +                       timebase-frequency = <0>;       // from bootloader
> +                       bus-frequency = <0>;            // from bootloader
> +                       clock-frequency = <0>;          // from bootloader
> +               };
> +       };
> +
> +       memory at 40000000 {
> +               device_type = "memory";
> +               reg = <0x40000000 0x10000>;     // 32KiB internal SRAM
> +       };
> +
> +       xbar at fff04000 {         // System Bus Crossbar Switch (XBAR)
> +               compatible = "fsl,mpc5554-xbar";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               // The full memory range is covered by XBAR
> +               ranges = <>;

An empty ranges property looks like this:

ranges;

> +               bridge at fff00000 {
> +                       compatible = "fsl,mpc5554-pbridge-b";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0 0xe0000000 0x20000000>;
> +                       reg = <0xfff00000 0x4000>;
> +
> +                       ecsm at fff40000 {         // Error Correction Status Module (ECSM)
> +                               compatible = "fsl,mpc5554-ecsm";
> +                               reg = <0xfff40000 0x4000>;
> +                       };
> +
> +                       edma at fff44000 {         // Enhanced DMA Controller (eDMA)
> +                               compatible = "fsl,mpc5554-edma";
> +                               reg = <0xfff44000 0x4000>;
> +                       };
> +
> +                       intc at fff48000 {         // Interrupt Controller (INTC)
> +                               compatible = "fsl,mpc5554-intc";
> +                               reg = <0xfff48000 0x4000>;
> +                       };

Need a label on this node so that the rest of the tree can find it,
and it needs the interrupt-controller and #interrupt-cells properties:

                       intc: intc at fff48000 {         // Interrupt
Controller (INTC)
                               compatible = "fsl,mpc5554-intc";
                               interrupt-controller;
                               #interrupt-cells = <2>;
                               reg = <0xfff48000 0x4000>;
                       };

I've set #interrupt-cells to 2 which is fairly typical on fsl parts,
but that may or may not make sense.  You need to decide how each
device is going to specify it's interrupt line.  Often the first cell
is the hardware interrupt number, and the second cell encodes the
sense (high, low, edge).  Conversely, PCI interrupts only use
#interrupt-cells = <1> because all PCI irqs use the active low sense.

Then, each device in the tree should have an 'interrupts = < [hwirq#]
[sense]>;' property.

Otherwise, starting to look pretty good.

g.