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 <[EMAIL PROTECTED]> wrote:
> On Sun, Oct 07, 2007 at 01:25:33PM +0200, Marian Balakowicz wrote:
> > +             [EMAIL PROTECTED] {       // 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
>
> > +             [EMAIL PROTECTED] {       // Real time clock
> > +                     compatible = "mpc5200b-rtc\0mpc5200-rtc";
> > +                     device_type = "rtc";
>
> This doesn't actually implement the OF rtc interface...
>
> > +             [EMAIL PROTECTED] {
>
> What is mscan?
>
> > +                     device_type = "mscan";
>
> This is not a standard device type.
>
> > +             [EMAIL PROTECTED] {
> > +                     device_type = "dma-controller";
>
> dma-controller should be the node name, and device_type should be omitted.
>
> > +             [EMAIL PROTECTED] {
> > +                     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. :-)
>
> > +             [EMAIL PROTECTED] {
> > +                     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.
>
> > +             [EMAIL PROTECTED] {
> > +                     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@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Reply via email to