Re: [PATCH 2/3] arm/dt: add very basic dts file for babbage board

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Mon, Feb 28, 2011 at 10:32:01PM +0800, Shawn Guo wrote:
> Hi Grant,
> 
> On Mon, Feb 21, 2011 at 10:10:24AM -0700, Grant Likely wrote:
> > On Mon, Feb 21, 2011 at 2:46 AM, Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> [...]
> > >> +       clocks {
> 
> Do we already have a clock binding specification, which is followed
> by this example?  Or the properties given here still can be discussed
> and open to change?
> 
> Also to support dynamically creating and registering 'struct clk' per
> dt clock nodes, I need to add a few properties to map stuff like
> parent 'struct clk', con_id of 'struct clk_lookup'.  Is there any
> specification to follow, or I can draft it per the needs of my work?

The clock specifications are still in draft.  I'm not willing to lock
down on a binding until we've got some real implementations to back
them up.  If you need additional properties, go ahead an propose them.

Keep in mind when defining/extending bindings, that the dt is intended
to represent hardware layout, not Linux kernel internal implementation
details.  Try to avoid describing things in a linux-specific way.


> 
> > >> +               #address-cells = <1>;
> > >> +               #size-cells = <0>;
> > >> +
> > >
> > > Can we use hex value here to keep the consistency throughout the file?
> > 
> > For #address-cells and #size-cells use decimal integers.
> > 
> > >
> > >> +               uart_clk0: uart@0 {
> > 
> > @0 should only be specified if the node has a 'reg = <0>' property.
> > In this case it doesn't so either 'reg' should be added, or '@0'
> > should be removed.
> > 
> > >> +                       compatible = "clock";
> > >> +                       clock-outputs = "imx-uart.0";
> > >> +               };
> > >> +
> > >> +               uart_clk1: uart@1{
> > >
> > > Can we put a space before "{" to keep the consistency throughout the file?
> > 
> > Yes, please.
> > 
> > >
> > >> +                       compatible = "clock";
> > >> +                       clock-outputs = "imx-uart.1";
> > >> +               };
> > >> +
> > >> +               uart_clk2: uart@2{
> > >
> > > ditto
> > >
> > >> +                       compatible = "clock";
> > >> +                       clock-outputs = "imx-uart.2";
> > >> +               };
> 
> -- 
> Regards,
> Shawn
>