Quoting Tero Kristo (2013-08-29 06:15:53)
> The OMAP clock driver now supports DPLL clock type. This patch also
> adds support for DT DPLL nodes.
>
> Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
Tero,
Overall this patch is really great. Some minor comments below.
> diff --git a/Documentation/devicetree/bindings/clock/ti/dpll.txt b/Documentation/devicetree/bindings/clock/ti/dpll.txt
> new file mode 100644
> index 0000000..83c21c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti/dpll.txt
> @@ -0,0 +1,68 @@
> +Binding for Texas Instruments DPLL clock.
> +
> +This binding uses the common clock binding[1]. It assumes a
> +register-mapped DPLL with usually two selectable input clocks
> +(reference clock and bypass clock), with digital phase locked
> +loop logic for multiplying the input clock to a desired output
> +clock. This clock also typically supports different operation
> +modes (locked, low power stop etc.) This binding has several
> +sub-types, which effectively result in slightly different setup
> +for the actual DPLL clock.
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : shall be one of:
> + "ti,omap4-dpll-x2-clock",
> + "ti,omap3-dpll-clock",
> + "ti,omap3-dpll-core-clock",
> + "ti,omap3-dpll-per-clock",
> + "ti,omap3-dpll-per-j-type-clock",
> + "ti,omap4-dpll-clock",
> + "ti,omap4-dpll-core-clock",
> + "ti,omap4-dpll-m4xen-clock",
> + "ti,omap4-dpll-j-type-clock",
> + "ti,omap4-dpll-no-gate-clock",
> + "ti,omap4-dpll-no-gate-j-type-clock",
> +
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- clocks : link phandles of parent clocks, first entry lists reference clock
> + and second entry bypass clock
> +- reg : address and length of the register set for controlling the DPLL.
> + It contains the information of registers in the same order as described by
> + reg-names.
> +- reg-names : array of the register names for controlling the device, sorted
> + in the same order as the reg property.
> + "control" - contains the control register base address
> + "idlest" - contains the idle status register base address
> + "autoidle" - contains the autoidle register base address
> + "mult-div1" - contains the multiplier / divider register base address
> +
> +Optional properties:
> +- ti,modes : available modes for the DPLL, bitmask of:
> + 0x02 - DPLL supports low power stop mode
> + 0x20 - DPLL can be put to low power bypass mode
> + 0x80 - DPLL can be put to lock mode (running)
Any particular reason to use a bitmask here versus flags/DT properties?
Something like
Optional properties:
low-power-stop : DPLL supports low power stop mode, gating output
low-power-bypass : DPLL output matches rate of parent bypass clock
lock : DPLL locks in programmed rate
Is there ever a time when a DPLL does not support the locked state? We
can probably remove that from binding entirely.
> + Other values currently unsupported.
> +- ti,clkdm-name : clockdomain name for the DPLL
What does this mean? I thought that the clockdomain data would not go
into the clock binding?
> diff --git a/drivers/clk/ti/dpll.c b/drivers/clk/ti/dpll.c
> new file mode 100644
> index 0000000..3f4236d
> --- /dev/null
> +++ b/drivers/clk/ti/dpll.c
> @@ -0,0 +1,476 @@
> +/*
> + * OMAP DPLL clock support
> + *
> + * Copyright (C) 2013 Texas Instruments, Inc.
> + *
> + * Tero Kristo <t-kristo@xxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/clk/ti.h>
> +
> +static const struct clk_ops dpll_m4xen_ck_ops = {
> + .enable = &omap3_noncore_dpll_enable,
> + .disable = &omap3_noncore_dpll_disable,
> + .recalc_rate = &omap4_dpll_regm4xen_recalc,
> + .round_rate = &omap4_dpll_regm4xen_round_rate,
> + .set_rate = &omap3_noncore_dpll_set_rate,
> + .get_parent = &omap2_init_dpll_parent,
> +};
> +
> +static const struct clk_ops dpll_core_ck_ops = {
> + .recalc_rate = &omap3_dpll_recalc,
> + .get_parent = &omap2_init_dpll_parent,
> +};
> +
> +static const struct clk_ops omap3_dpll_core_ck_ops = {
> + .init = &omap2_init_clk_clkdm,
> + .get_parent = &omap2_init_dpll_parent,
> + .recalc_rate = &omap3_dpll_recalc,
> + .round_rate = &omap2_dpll_round_rate,
> +};
> +
> +static const struct clk_ops dpll_ck_ops = {
> + .enable = &omap3_noncore_dpll_enable,
> + .disable = &omap3_noncore_dpll_disable,
> + .recalc_rate = &omap3_dpll_recalc,
> + .round_rate = &omap2_dpll_round_rate,
> + .set_rate = &omap3_noncore_dpll_set_rate,
> + .get_parent = &omap2_init_dpll_parent,
> + .init = &omap2_init_clk_clkdm,
> +};
> +
> +static const struct clk_ops dpll_no_gate_ck_ops = {
> + .recalc_rate = &omap3_dpll_recalc,
> + .get_parent = &omap2_init_dpll_parent,
> + .round_rate = &omap2_dpll_round_rate,
> + .set_rate = &omap3_noncore_dpll_set_rate,
> +};
> +
> +static const struct clk_ops omap3_dpll_ck_ops = {
> + .init = &omap2_init_clk_clkdm,
> + .enable = &omap3_noncore_dpll_enable,
> + .disable = &omap3_noncore_dpll_disable,
> + .get_parent = &omap2_init_dpll_parent,
> + .recalc_rate = &omap3_dpll_recalc,
> + .set_rate = &omap3_noncore_dpll_set_rate,
> + .round_rate = &omap2_dpll_round_rate,
> +};
> +
> +static const struct clk_ops omap3_dpll_per_ck_ops = {
> + .init = &omap2_init_clk_clkdm,
> + .enable = &omap3_noncore_dpll_enable,
> + .disable = &omap3_noncore_dpll_disable,
> + .get_parent = &omap2_init_dpll_parent,
> + .recalc_rate = &omap3_dpll_recalc,
> + .set_rate = &omap3_dpll4_set_rate,
> + .round_rate = &omap2_dpll_round_rate,
> +};
> +
> +static const struct clk_ops dpll_x2_ck_ops = {
> + .recalc_rate = &omap3_clkoutx2_recalc,
> +};
No .set_parent ops for any of the DPLLs? How do you handle bypassing
them? Are you using another mux clock that is the parent of the DPLL, or
just open-coding the bypass stuff?
Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
