Subject: Re: [PATCH 1/3] add default device trees for MPC837x MDS board

From: Scott Wood <[email-masked]>
Date: Tue, 8 Jan 2008 11:38:45 -0600

On Tue, Jan 08, 2008 at 08:58:17AM -0700, Grant Likely wrote:
> On 1/7/08, Li Yang <[email-masked]> wrote:
> > +       soc@e0000000 {
> > +               #address-cells = <1>;
> > +               #size-cells = <1>;
> > +               device_type = "soc";
> 
> I recommend dropping device_type and adding 'compatible = "fsl,mpc8377-immr";'

I recommend fixing the code that looks for this device_type before
recommending that people drop it. :-)

> > +               wdt@200 {
> > +                       compatible = "mpc83xx_wdt";
> 
> "fsl,mpc8377_wdt", "fsl,mpc83xx_wdt" as per generic names recommended practice.

Speaking of generic names, can we change the node name to "watchdog"?

> > +               /* phy type (ULPI, UTMI, UTMI_WIDE, SERIAL) */
> > +               usb@23000 {
> > +                       compatible = "fsl-usb2-dr";
> > +                       reg = <23000 1000>;
> > +                       #address-cells = <1>;
> > +                       #size-cells = <0>;
> > +                       interrupt-parent = < &ipic >;
> > +                       interrupts = <26 8>;
> > +                       phy_type = "utmi_wide";
> 
> fsl,phy_type please.

Again, code will break.  Can we stop ambushing people submitting board
support with complaints against existing, non-board-specific code/device
trees?  Fix that first, then complain if new code reintroduces the crud.

Also, if we're going to change the property name, we should also change the
underscore to a dash.

> > +               enet0: ethernet@24000 {
> > +                       cell-index = <0>;
> > +                       device_type = "network";
> > +                       model = "eTSEC";
> 
> Drop model property

Fix the code that checks for it.

-Scott