Subject: Re: [PATCH] powerpc/fsl-booke: Add T1040D4RDB/T1042D4RDB board support

From: Scott Wood <[email-masked]>
Date: Mon, 13 Jul 2015 15:39:19 -0500

On Sun, 2015-07-12 at 00:47 -0500, Priyanka Jain wrote:
> +/ {
> +     model = "fsl,T1040D4RDB";
> +     compatible = "fsl,T1040D4RDB";
> +     #address-cells = <2>;
> +     #size-cells = <2>;
> +     interrupt-parent = <&mpic>;
> +
> +     ifc:  localbus@ffe124000{
> +             cpld@3,0 {
> +                     compatible = "fsl,t1040d4rdb-cpld",
> +                                     "fsl,deepsleep-cpld";
> +             };
> +     };
> +};

What is "fsl,deepsleep-cpld"?  Besides the lack of a binding, it seems too 
vague.  Not all deep sleep capable CPLDs are necessarily going to be 
identical.

> +     ifc:  localbus@ffe124000{
> +             reg = <0xf 0xfe124000 0 0x2000>;
> +             ranges = <0 0 0xf 0xe8000000 0x08000000
> +                       2 0 0xf 0xff800000 0x00010000
> +                       3 0 0xf 0xffdf0000 0x00008000>;
> +
> +             nor@0,0 {
> +                     #address-cells = <1>;
> +                     #size-cells = <1>;
> +                     compatible = "cfi-flash";
> +                     reg = <0x0 0x0 0x8000000>;
> +                     bank-width = <2>;
> +                     device-width = <1>;
> +             };
> +
> +             nand@2,0 {
> +                     #address-cells = <1>;
> +                     #size-cells = <1>;
> +                     compatible = "fsl,ifc-nand";
> +                     reg = <0x2 0x0 0x10000>;
> +             };
> +
> +             cpld@3,0 {
> +                     reg = <3 0 0x300>;
> +             };

I'd rather keep reg with the compatible.

> +                      adt7461@4c{
> +                             compatible = "adi,adt7461";
> +                             reg = <0x4c>;
> +                     };

Node names should be generic.

> +              i2c@118100{
> +                      pca9546@77{
> +                             compatible = "nxp,pca9546";
> +                             reg = <0x77>;
> +                             #address-cells = <1>;
> +                             #size-cells = <0>;
> +                     };

Likewise.

-Scott