[PATCH 07/15] [POWERPC] Promess Motion-PRO DTS

[Grant Likely]

Hey Scott.

Pretty much all your comments below directly apply to the existing
Lite5200 device tree and the Efika firmware.  These are issues that
were created a while ago and I've never gone back to clean up the
mpc5200 device tree bindings.  (Plus we need to have code to maintian
compatibility with the Efika firmware.

g.

On 10/8/07, Scott Wood <scottwood at freescale.com> wrote:
> On Sun, Oct 07, 2007 at 01:25:33PM +0200, Marian Balakowicz wrote:
> > +             gpt at 600 {       // General Purpose Timer
> > +                     compatible = "mpc5200b-gpt\0mpc5200-gpt";
> > +                     device_type = "gpt";
>
> "timer" would be a better node name than "gpt", and the device type should be
> left out entirely.  As others pointed out, compatible should be
> "fsl,mpc5200b-gpt", "fsl,mpc5200-gpt".
>
> > +                     has-wdt;
>
> fsl,has-wdt
>
> > +             rtc at 800 {       // Real time clock
> > +                     compatible = "mpc5200b-rtc\0mpc5200-rtc";
> > +                     device_type = "rtc";
>
> This doesn't actually implement the OF rtc interface...
>
> > +             mscan at 980 {
>
> What is mscan?
>
> > +                     device_type = "mscan";
>
> This is not a standard device type.
>
> > +             bestcomm at 1200 {
> > +                     device_type = "dma-controller";
>
> dma-controller should be the node name, and device_type should be omitted.
>
> > +             ethernet at 3000 {
> > +                     device_type = "network";
> > +                     compatible = "mpc5200b-fec\0mpc5200-fec";
> > +                     reg = <3000 800>;
> > +                     mac-address = [ 02 03 04 05 06 07 ]; // Bad!
>
> Should be local-mac-address.
> And yes, hardcoding a mac address is bad.  Don't do it. :-)
>
> > +             i2c at 3d40 {
> > +                     device_type = "i2c";
> > +                     compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
> > +                     cell-index = <1>;
>
> What is cell-index?  The fsl-i2c driver doesn't use it AFAICT.
>
> > +             sram at 8000 {
> > +                     device_type = "sram";
>
> No device type.
>
> > +     cpld {
> > +             device_type = "cpld";
> > +             compatible = "cpld";
> > +             reg = <50010000 ffff>;
> > +     };
>
> This device is compatible with every CPLD that has ever existed?  Wow! :-)
>
> -Scott
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195