OK, will update per your suggestions.
Thanks,
Yuantian
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: 2013年9月11日 星期三 5:47
> To: Tang Yuantian-B29983
> Cc: Wood Scott-B07421; galak@xxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx; Mike Turquette
> Subject: Re: [v3] powerpc/mpc85xx: Update the clock device tree nodes
>
> On Mon, 2013-08-26 at 21:49 -0500, Tang Yuantian-B29983 wrote:
> > > > > > + };
> > > > > > + pll1: pll1@820 {
> > > > > > + #clock-cells = <1>;
> > > > > > + reg = <0x820>;
> > > > > > + compatible = "fsl,core-pll-clock";
> > > > > > + clocks = <&clockgen>;
> > > > > > + clock-output-names = "pll1", "pll1-div2", "pll1-
> > > div4";
> > > > > > + };
> > > > >
> > > > > Please leave a blank line between properties and nodes, and
> > > > > between
> > > nodes.
> > > > >
> > > > OK, will add.
> > > >
> > > > > What does reg represent? Where is the binding for this?
> > > > >
> > > > > The compatible is too vague.
> > > > Reg is register offset.
> > >
> > > With no size?
> >
> > No size is needed.
>
> Yes, it is. Register blocks have size -- even if it's just a single
> register.
>
> > > > It is too later to change since the clock driver is merged for
> > > > months although I sent this patch first.
> > >
> > > It should not have gone in without an approved binding. It seems it
> > > went in via Mike Turquette (why is a non-ARM-specific tree using
> > > linux-arm- kernel as its list, BTW?). No ack from Ben, Kumar, or me
> > > is shown in the commit.
> > The Linux common clock framework is not ARM specific. Any other arch
> can use it.
>
> Sure, it just seemed an odd choice of mailing list for something that
> isn't ARM-specific.
>
> > > In any case, you can preserve compatibility with existing trees
> > > without using this compatible in new trees. The driver can check
> > > for both compatibles, with a comment indicating that
> > > "fsl,core-mux-clock" is deprecated and for compatibility only.
> > It is sub-clock node, is it really necessary to think about
> compatibility?
> > I think that's the node clockgen's responsibility.
>
> It describes registers, so yes, you need to consider compatibility. A
> clock provider is not responsible for figuring out how to program devices
> that consume its clocks, nor should it make any assumptions about such
> devices.
>
> > > > Besides, it is not too bad because other arch use the similar name.
> > >
> > > I don't follow. This is a specific Freescale register interface,
> > > not a general concept.
> > >
> > > In any case, which "similar names" are you referring to? A search
> > > in arch/arm/boot/dts for "mux" with "clk" or "clock" turns up
> > > "allwinner,sun4i-apb1-mux-clk" which is much more specific than
> > > "fsl,core-mux-clock".
> > Ok, I will change the compatible string.
> > Do you think "fsl,ppc-core-*" is ok?
>
> No. How about "fsl,qoriq-chassis1-*" (for e500mc/e5500) and fsl,qoriq-
> chassis2-*" (for e6500)?
>
> -Scott
>
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
