> > > On Wed, 2013-09-11 at 20:31 -0500, Tang Yuantian-B29983 wrote:
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: 2013年9月12日 星期四 9:10
> > > > > To: Tang Yuantian-B29983
> > > > > Cc: galak@xxxxxxxxxxxxxxxxxxx; linuxppc-dev@xxxxxxxxxxxxxxxx;
> > > > > devicetree@xxxxxxxxxxxxxxx; Li Yang-Leo-R58472
> > > > > Subject: Re: [PATCH v4] powerpc/mpc85xx: Update the clock device
> > > > > tree nodes
> > > > >
> > > > > This description of "reg" is overly specific (assumes how the
> > > > > parent node's ranges are set up), incomplete (there's a size as
> > > > > well as the offset), and does not apply to the clockgen node
> > > > > itself (you probably shouldn't lump them together like this).
> > > > >
> > > > Do you mean I should explain the REG of clockgen and its child
> > > > node
> > > respectively?
> > > >
> > > > > > +- clocks : shall be the input parent clock phandle for the
> clock.
> > > > >
> > > > > Not required on the clockgen node
> > > > >
> > > > Required by child node of clockgen.
> > >
> > > My point is that you're lumping several different types of nodes
> > > together with one binding, when some parts of the binding are not
> > > applicable to the clockgen node.
> > >
> > Not several, just two types of nodes.
> > One is clockgen node, the other is PLL and mux nodes.
>
> clockgen + PLL + mux = 3 = several :-)
>
> > The reason they lumped together is that the clockgen node is not only
> > IP block Node but also a clock provider node
>
> I don't understand why that merits lumping them together.
>
> Just describe them separately.
>
It is not that easy to separate them because clockgen node plays two types
Of roles. Take REG property as example:
As IP block node, REG should be reg = <0xe1000 0x1000>, while as
Clock provider node, it should be reg = <0xabc 0x4> or no reg at all(for fixed clock).
> > At first, I want to add a extra fixed-clock node and move the
> > clock-frequency of clockgen Node to it, but it is against the backward
> > compatibility
>
> Right.
>
> > which I think it is not a big deal, Because nobody hasn't used it yet.
>
> The point is it will require updating U-Boot to use it, versus existing
> U-Boots which already patch up the clock-frequency in the clockgen node.
> And there's nothing semantically wrong with the way it currently is.
>
Yes, nothing wrong about it.
But we will keep adding the clockgen-x.y node all the time in uboot.
If we have one extra node to keep clock-frequency, it would be updated only once.
> > If I add a extra node with the clock-frequency property and don't move
> > the clock-frequency property of clockgen, that would be redundant
> > because both clockgen node and the extra node have the same clock-
> frequency node.
> > So, I choose what I did now.
>
> I'm not complaining about how you structured the nodes, just how you
> documented them.
>
As I said it is hard to document clockgen node if we don't separate its
two roles.
I think the following structure is better.
+ clockgen: global-utilities@e1000 {
+ compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0";
+ reg = <0xe1000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
Sysclk:sysclk {
compatible = "fsl,qoriq-sysclk", "fixed-clock";
clock-output-names = "sysclk";
#clock-cells = <0>;
clock-frequency = <0>;
}
+ pll0: pll0@800 {
+ #clock-cells = <1>;
+ reg = <0x800 0x4>;
+ compatible = "fsl,qoriq-chassis1-core-pll";
+ clocks = <& Sysclk >;
+ clock-output-names = "pll0", "pll0-div2";
+ };
Regards,
Yuantian
> -Scott
>
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
