Re: [PATCH] prtlvt board support (MPC5121e) added

2008-06-10 Thread Grant Likely
Thanks for the patch, comments below.

BTW, When sending to the mailing list, please don't use bcc: or hide
the recipients.

Cheers,
g.

On Tue, Jun 10, 2008 at 10:29 AM, David Jander <[EMAIL PROTECTED]> wrote:
> Signed-off-by: David Jander <[EMAIL PROTECTED]>
> ---
>  arch/powerpc/boot/dts/prtlvt.dts |  280 
> ++
>  arch/powerpc/platforms/512x/Kconfig  |9 +
>  arch/powerpc/platforms/512x/Makefile |1 +
>  arch/powerpc/platforms/512x/prtlvt.c |  105 +
>  4 files changed, 395 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/boot/dts/prtlvt.dts
>  create mode 100644 arch/powerpc/platforms/512x/prtlvt.c
>
> diff --git a/arch/powerpc/boot/dts/prtlvt.dts 
> b/arch/powerpc/boot/dts/prtlvt.dts
> new file mode 100644
> index 000..238dc89
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/prtlvt.dts
> @@ -0,0 +1,280 @@
> +/*
> + * MPC5121E MDS Device Tree Source
> + *
> + * Copyright 2007 Freescale Semiconductor Inc.
> + *
> + * 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.
> + */
> +
> + /* compile with: ./dtc -p 10240 -R 20 -I dts -o prtlvt.dtb -O dtb -b 0 
> dts/prtlvt.dts */
> +
> +/dts-v1/;
> +
> +/ {
> +   model = "prtlvt";
> +   compatible = "fsl,prtlvt";

Not true.  This board is not manufactured by Freescale, so you should
not use the 'fsl,' prefix.  Use your companies name or stock ticker
symbol instead.

> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +
> +   cpus {
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   PowerPC,[EMAIL PROTECTED] {
> +   device_type = "cpu";
> +   reg = <0>;
> +   d-cache-line-size = <0x20>; // 32 bytes
> +   i-cache-line-size = <0x20>; // 32 bytes
> +   d-cache-size = <0x8000>;// L1, 32K
> +   i-cache-size = <0x8000>;// L1, 32K
> +   timebase-frequency = <5000>;// 50 MHz (csb/4)
> +   bus-frequency = <2>;// 200 MHz csb bus
> +   clock-frequency = <4>;  // 400 MHz ppc core
> +   };
> +   };
> +
> +   memory {
> +   device_type = "memory";
> +   reg = <0x 0x1000>;  // 256MB at 0
> +   };
> +
> +   [EMAIL PROTECTED] {
> +   compatible = "fsl,prtlvt-localbus";

This doesn't look right.  The local bus is not board specific; it is
SoC specific.  So this should be:

compatible = "fsl,mpc5121-localbus", "simple-bus";

> +   #address-cells = <2>;
> +   #size-cells = <1>;
> +   reg = <0x8020 0x40>;
> +
> +   ranges = <0x0 0x0 0xfe00 0x0200>;
> +
> +   [EMAIL PROTECTED],0 {
> +   compatible = "cfi-flash";
> +   reg = <0 0x0 0x400>;
> +   bank-width = <4>;
> +   device-width = <1>;
> +   };
> +   };
> +
> +   [EMAIL PROTECTED] {
> +   compatible = "fsl,mpc5121-immr";
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   #interrupt-cells = <2>;
> +   ranges = <0x0 0x8000 0x40>;
> +   reg = <0x8000 0x40>;
> +   bus-frequency = <6600>; // 66 MHz ips bus
> +
> +
> +   // IPIC
> +   // interrupts cell = 
> +   // sense values match linux IORESOURCE_IRQ_* defines:
> +   // sense == 8: Level, low assertion
> +   // sense == 2: Edge, high-to-low change
> +   //
> +   ipic: [EMAIL PROTECTED] {
> +   compatible = "fsl,mpc5121-ipic", "fsl,ipic";
> +   interrupt-controller;
> +   #address-cells = <0>;
> +   #interrupt-cells = <2>;
> +   reg = <0xc00 0x100>;
> +   };
> +
> +   // 512x PSCs are not 52xx PSCs compatible
> +   // PSC0 serial port A aka ttyPSC0
> +   [EMAIL PROTECTED] {
> +   device_type = "serial";
> +   compatible = "fsl,mpc5121-psc-uart";
> +   // Logical port assignment needed until driver
> +   // learns to use aliases
> +   port-number = <0>;
> +   cell-index = <0>;
> +   reg = <0x11000 0x100>;
> +   interrupts = <0x28 0x8>; // actually the fifo irq
> +   interrupt-parent = < &ipic >;
> +   };
> +
> +   // PSC1 seri

Re: [PATCH] prtlvt board support (MPC5121e) added

2008-06-11 Thread Grant Likely
On Wed, Jun 11, 2008 at 3:41 AM, David Jander <[EMAIL PROTECTED]> wrote:
> On Tuesday 10 June 2008 22:37:18 you wrote:
>> Thanks for the patch, comments below.
>> > --- /dev/null
>> > +++ b/arch/powerpc/platforms/512x/prtlvt.c
>>
>> Looks like you've just duplicated mpc5121_ads.c.  You should just add
>> your boards name to the mpc5121_ads_probe function.  Bonus points if
>> you rename the .c file to something more generic.  You only need a new
>> board file if your board needs something special.
>
> Well, it looks like mpc5121_ads.c is pretty generic. The MPC5121 is almost
> a "motherboard-in-a-chip" type of SoC, so there isn't much board-specific
> stuff anywhere. IO-control register setup should be done in the boot-loader.
> What should we do about this then? Rename the whole mpc5121_ads stuff to
> something more generic? What about calling it "mpc5121_generic.c"?

mpc5121_generic.c works for me.

> Btw, you posted this to linucppc-dev mailing list, but I think that was a
> mistake, since this is clearly an embedded processor, so I removed the CC.

Nope, not a mistake.  Discussions on embedded stuff is welcome on the
linuxppc-dev mailing list.  There has been some talk of removing the
linuxppc-embedded list entirely.  I've added the list back to the to:
list.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev