[PATCH v6 09/11] clk: mmp: parse clock from dts
Tomasz Figa
tomasz.figa at gmail.com
Sat Aug 10 07:06:01 EDT 2013
Hi Mark, Haojian,
On Friday 09 of August 2013 17:04:01 Mark Rutland wrote:
> On Fri, Jul 26, 2013 at 11:05:31AM +0100, Haojian Zhuang wrote:
> > Parse clock information from DTS file for mach-mmp.
> >
> > Changelog:
> > v6:
> > 1. Remove marvell string from properties in clock driver.
> > 2. Append document for clock binding device tree.
> > 3. Use apbc to replace apbcp. Since the main difference is register
> > base.
> >
> > v5:
> > 1. Replace clk_register_mux() by clk_register_mux_table().
> >
> > Signed-off-by: Haojian Zhuang <haojian.zhuang at gmail.com>
> > ---
> >
> > .../devicetree/bindings/clock/mmp-clock.txt | 51 ++++
> > arch/arm/mach-mmp/mmp-dt.c | 2 +
> > drivers/clk/mmp/Makefile | 2 +-
> > drivers/clk/mmp/clk-mmp.c | 300
> > +++++++++++++++++++++ 4 files changed, 354 insertions(+), 1
> > deletion(-)
> > create mode 100644
> > Documentation/devicetree/bindings/clock/mmp-clock.txt create mode
> > 100644 drivers/clk/mmp/clk-mmp.c
> >
> > diff --git a/Documentation/devicetree/bindings/clock/mmp-clock.txt
> > b/Documentation/devicetree/bindings/clock/mmp-clock.txt new file mode
> > 100644
> > index 0000000..5fe52a2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mmp-clock.txt
> > @@ -0,0 +1,51 @@
> > +Device Tree Clock bindings for arch-mmp
> > +
> > +This binding uses the common clock binding[1].
> > +
> > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
>
> Are these used somewhere already?
Hmm? Are you asking about the generic clock bindings or I didn't get your
question right? They are supposed to be used (and they are used) whenever
clocks are involved.
> > +Required properties for fixed rate clocks:
> > + - compatible : should be "marvell,mmp-fixed-clkrate".
> > + - clocks : should be the input parent clock phandle for the clock.
> > This + should be the reference clock.
> > + - clock-output-names : should be reference name.
> > + - #clock-cells : from common clock binding; should be set to 0.
> > + - clock-frequency : frequency of the fixed rate clock
>
> Why is "fixed-clock" not good enough?
>
> This doesn't seem to describe anything more, and there's nothing
> additional in the driver...
+1
> > +
> > +Required properties for fixed factor clocks:
> > + - compatible : should be "marvell,mmp-fixed-clkfactor".
> > + - clocks : should be the input parent clock phandle for the clock.
> > This + should be the reference clock.
> > + - clock-output-names : should be reference name.
> > + - #clock-cells : from common clock binding; should be set to 0.
>
> The code seems to use an additional property not described here
> (mmp-fixed-factor). What does that represent?
>
> Why not "fixed-factor-clock"?
+1
> > +
> > +Required properties for mux clocks:
> > + - compatible : should be "marvell,mmp-clkmux".
> > + - clocks : should be the input parent clock phandle for the clock.
> > This + should be the reference clock.
> > + - clock-output-names : should be reference name.
> > + - #clock-cells : from common clock binding; should be set to 0.
> > + - mmp-clk-reg : mux register offset & mask bits.
>
> Offset from where? There's no reg, no container's described and there's
> no example.
+1
> > + - mmp-clk-sel : array of mux select bits
> > +
> > +Required properties for divider clocks:
> > + - compatible : should be "marvell,mmp-apmu-clkdiv".
> > + - clocks : should be the input parent clock phandle for the clock.
> > This + should be the reference clock.
> > + - clock-output-names : should be reference name.
> > + - #clock-cells : from common clock binding; should be set to 0.
> > + - mmp-clk-reg : divider register offset & mask bits.
>
> Similar comments to those above. This is also a fixed-factor-clock, no?
This looks like a clock with configurable divisor for me, but again, why
not use the standard divider clock here?
Best regards,
Tomasz
> > +
> > +Required properties for gate clocks:
> > + - compatible : should be "marvell,mmp-apbc-clk".
> > + - clocks : should be the input parent clock phandle for the clock.
> > This + should be the reference clock.
> > + - clock-output-names : should be reference name.
> > + - #clock-cells : from common clock binding; should be set to 0.
> > + - mmp-clk-reg : gate register offset & mask bits.
>
> Offset from where?
>
> > +
> > +Optional properties for gate clocks:
> > + - mmp-clk-delay : delay between enabling function clock and apb
> > clock
> Units?
>
> > + - mmp-apbc-power-ctrl : enable apbc power control bit
>
> Is this a boolean or the offset of the bit offset?
>
> > +
>
> > +For example:
> Missing example?
>
> [...]
>
> > +static const struct of_device_id mmp_of_match[] = {
> > + { .compatible = "marvell,mmp-mpmu", .data = (void *)MMP_MPMU,
> > }, + { .compatible = "marvell,mmp-apmu", .data = (void
> > *)MMP_APMU, }, + { .compatible = "marvell,mmp-apbc", .data =
> > (void *)MMP_APBC, }, + { .compatible = "marvell,mmp-apbcp",
> > .data = (void *)MMP_APBCP, }, + { .compatible = "mrvl,apb-bus",
> > .data = (void *)MMP_APB, }, +};
>
> Not documented?
>
> Thanks,
> Mark.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
More information about the linux-arm-kernel
mailing list