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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
