On Mon, 2013-08-26 at 21:49 -0500, Tang Yuantian-B29983 wrote:

quoted

quoted

quoted

quoted

+		};
+		pll1: pll1@820 {
+			#clock-cells = <1>;
+			reg = <0x820>;
+			compatible = "fsl,core-pll-clock";
+			clocks = <&clockgen>;
+			clock-output-names = "pll1", "pll1-div2", "pll1-

div4";

quoted

quoted

quoted

+		};

Please leave a blank line between properties and nodes, and between

nodes.

quoted

quoted

OK, will add.

quoted

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.

quoted

quoted

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.

quoted

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.
 

quoted

quoted

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