> > > > + };
> > > > + 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.
>
> > I should have had a binding document.
> > About the compatible, you should pointed it out earlier in SDK review.
>
> Sorry, it doesn't work that way. I don't know why I didn't notice this
> stuff there -- the SDK review was probably rushed, with someone shouting
> "urgent". The SDK does not dictate what goes upstream. Device tree
> bindings should go upstream first.
>
When I sent the patch v1, there is a binding document with it. But I missed
It in the patch v3, so when patch v3 got merged, the binding document didn't get merged.
I will make the binding go upstream first next time.
> > 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.
In fact, this clock driver is the first one that use common clk on PPC arch.
I will get the ack from you guys next time. I hope it doesn't make me wait too long.
>
> 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.
>
> > 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?
Regards,
Yuantian
>
> -Scott
>
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
