On 10/25/2013 10:57 AM, Tero Kristo wrote: > This is a multipurpose clock node, which contains support for multiple > sub-clocks. Uses basic composite clock type to implement the actual > functionality, and TI specific gate, mux and divider clocks. > > Signed-off-by: Tero Kristo <t-kristo@xxxxxx> > --- > .../devicetree/bindings/clock/ti/composite.txt | 54 +++++ > drivers/clk/ti/Makefile | 2 +- > drivers/clk/ti/composite.c | 222 ++++++++++++++++++++ > include/linux/clk/ti.h | 8 + > 4 files changed, 285 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/clock/ti/composite.txt > create mode 100644 drivers/clk/ti/composite.c > > diff --git a/Documentation/devicetree/bindings/clock/ti/composite.txt b/Documentation/devicetree/bindings/clock/ti/composite.txt > new file mode 100644 > index 0000000..5f43c47 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/ti/composite.txt > @@ -0,0 +1,54 @@ > +Binding for TI composite clock. > + > +Binding status: Unstable - ABI compatibility may be broken in the future > + > +This binding uses the common clock binding[1]. It assumes a > +register-mapped composite clock with multiple different sub-types; > + > +a multiplexer clock with multiple input clock signals or parents, one > +of which can be selected as output, this behaves exactly as [2] > + > +an adjustable clock rate divider, this behaves exactly as [3] > + > +a gating function which can be used to enable and disable the output > +clock, this behaves exactly as [4] > + > +The binding must provide a list of the component clocks that shall be > +merged to this clock. The component clocks shall be of one of the > +"ti,*composite*-clock" types. > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > +[2] Documentation/devicetree/bindings/clock/ti/mux.txt > +[3] Documentation/devicetree/bindings/clock/ti/divider.txt > +[4] Documentation/devicetree/bindings/clock/ti/gate.txt > + > +Required properties: > +- compatible : shall be: "ti,composite-clock" > +- clocks : link phandles of component clocks > +- #clock-cells : from common clock binding; shall be set to 0. > + > +Examples: > + > +usb_l4_gate_ick: usb_l4_gate_ick { > + #clock-cells = <0>; > + compatible = "ti,composite-interface-clock"; > + clocks = <&l4_ick>; > + ti,bit-shift = <5>; > + reg = <0x0a10>; > +}; > + > +usb_l4_div_ick: usb_l4_div_ick { > + #clock-cells = <0>; > + compatible = "ti,composite-divider-clock"; > + clocks = <&l4_ick>; > + ti,bit-shift = <4>; > + ti,max-div = <1>; > + reg = <0x0a40>; > + ti,index-starts-at-one; > +}; > + > +usb_l4_ick: usb_l4_ick { > + #clock-cells = <0>; > + compatible = "ti,composite-clock"; > + clocks = <&usb_l4_gate_ick>, <&usb_l4_div_ick>; > +}; > diff --git a/drivers/clk/ti/Makefile b/drivers/clk/ti/Makefile > index 533efb4..a4a7595 100644 > --- a/drivers/clk/ti/Makefile > +++ b/drivers/clk/ti/Makefile > @@ -1,3 +1,3 @@ > ifneq ($(CONFIG_OF),) > -obj-y += clk.o dpll.o autoidle.o > +obj-y += clk.o dpll.o autoidle.o composite.o > endif > diff --git a/drivers/clk/ti/composite.c b/drivers/clk/ti/composite.c > new file mode 100644 > index 0000000..9ce7e54 > --- /dev/null > +++ b/drivers/clk/ti/composite.c > @@ -0,0 +1,222 @@ > +/* > + * TI composite 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/io.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/clk/ti.h> > +#include <linux/list.h> > + > +#define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw) > + > +static unsigned long ti_composite_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + return clk_divider_ops.recalc_rate(hw, parent_rate); > +} > + > +static long ti_composite_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *prate) > +{ > + return -EINVAL; > +} > + > +static int ti_composite_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + return -EINVAL; > +} > + > +static const struct clk_ops ti_composite_divider_ops = { > + .recalc_rate = &ti_composite_recalc_rate, > + .round_rate = &ti_composite_round_rate, > + .set_rate = &ti_composite_set_rate, > +}; > + > +static const struct clk_ops ti_composite_gate_ops = { > + .enable = &omap2_dflt_clk_enable, > + .disable = &omap2_dflt_clk_disable, > + .is_enabled = &omap2_dflt_clk_is_enabled, > +}; > + > +struct component_clk { > + int num_parents; > + const char **parent_names; > + struct device_node *node; > + int type; > + struct clk_hw *hw; > + struct list_head link; > +}; > + > +static const char * __initdata component_clk_types[] = { [CLK_COMPONENT_TYPE_MAX]? > + "mux", "gate", "divider", > +}; > + > +static LIST_HEAD(component_clks); > + > +static struct component_clk *_lookup_component(struct device_node *node, int i) > +{ > + int rc; > + struct of_phandle_args clkspec; > + struct component_clk *comp; > + > + rc = of_parse_phandle_with_args(node, "clocks", "#clock-cells", i, > + &clkspec); > + if (rc) > + return NULL; > + > + list_for_each_entry(comp, &component_clks, link) { > + if (comp->node == clkspec.np) > + return comp; > + } > + return NULL; > +} > + > +static inline struct clk_hw *_get_hw(struct component_clk *clk) > +{ > + if (clk) > + return clk->hw; > + > + return NULL; > +} > + > +static int __init of_ti_composite_clk_setup(struct device_node *node, > + struct regmap *regmap) > +{ > + int num_clks; > + int i; > + struct clk *clk; > + struct component_clk *comp; > + struct component_clk *clks[3] = { NULL }; sizeof(component_clk_types) or CLK_COMPONENT_TYPE_MAX instead of 3? > + const char *name = node->name; > + int num_parents = 0; > + const char **parent_names = NULL; > + int ret = 0; > + > + /* Number of component clocks to be put inside this clock */ > + num_clks = of_clk_get_parent_count(node); > + > + if (num_clks < 1) { > + pr_err("%s: ti composite clk %s must have component(s)\n", > + __func__, name); using pr_fmt will reduce the pr_err code. > + return -EINVAL; > + } > + > + /* Check for presence of each component clock */ > + for (i = 0; i < num_clks; i++) { > + comp = _lookup_component(node, i); > + if (!comp) > + return -EAGAIN; > + if (clks[comp->type] != NULL) { > + pr_err("%s: duplicate component types for %s (%s)!\n", > + __func__, name, component_clk_types[comp->type]); > + return -EINVAL; would you not want to do a cleanup of the list here? > + } > + clks[comp->type] = comp; > + } > + > + /* All components exists, proceed with registration */ > + for (i = 0; i < 3; i++) { CLK_COMPONENT_TYPE_MAX instead > + if (!clks[i]) > + continue; > + if (clks[i]->num_parents) { > + num_parents = clks[i]->num_parents; > + parent_names = clks[i]->parent_names; > + } > + } > + > + /* Enforce mux parents if exists */ > + comp = clks[CLK_COMPONENT_TYPE_MUX]; > + if (comp) { > + num_parents = comp->num_parents; > + parent_names = comp->parent_names; > + } you could move the for loop in the else case and reduce the loops to CLK_COMPONENT_TYPE_MUX+1 to CLK_COMPONENT_TYPE_MAX. > + > + if (!num_parents) { > + pr_err("%s: no parents found for %s!\n", __func__, > + name); > + return -EINVAL; would you not want to do a cleanup of the list here? > + } > + > + clk = clk_register_composite(NULL, name, parent_names, num_parents, > + _get_hw(clks[CLK_COMPONENT_TYPE_MUX]), > + &clk_mux_ops, > + _get_hw(clks[CLK_COMPONENT_TYPE_DIVIDER]), > + &ti_composite_divider_ops, > + _get_hw(clks[CLK_COMPONENT_TYPE_GATE]), > + &ti_composite_gate_ops, 0); > + > + if (!IS_ERR(clk)) { > + of_clk_add_provider(node, of_clk_src_simple_get, clk); > + goto cleanup; > + } > + > + ret = PTR_ERR(clk); > +cleanup: > + /* Free component clock list entries */ > + for (i = 0; i < 3; i++) { > + if (!clks[i]) > + continue; > + list_del(&clks[i]->link); > + kfree(clks[i]); > + } could you not just do a kfree(clks[i]) and set the component_clks to NULL? Further, if there are only 3 clocks that can ever be present and we do not allow for duplicates types, could we not do those checks in ti_clk_add_component instead of having a harder recovery in setup of composite clk? > + > + return ret; > +} > +CLK_OF_DECLARE(ti_composite_clock, "ti,composite-clock", > + of_ti_composite_clk_setup); > + > +int __init ti_clk_add_component(struct device_node *node, struct clk_hw *hw, > + int type) > +{ > + int num_parents; > + const char **parent_names; > + struct component_clk *clk; > + int i; > + > + num_parents = of_clk_get_parent_count(node); > + > + if (num_parents < 1) { > + pr_err("%s: component-clock %s must have parent(s)\n", __func__, > + node->name); > + return -EINVAL; > + } > + > + parent_names = kzalloc((sizeof(char *) * num_parents), GFP_KERNEL); > + if (!parent_names) > + return -ENOMEM; > + > + for (i = 0; i < num_parents; i++) > + parent_names[i] = of_clk_get_parent_name(node, i); > + > + clk = kzalloc(sizeof(*clk), GFP_KERNEL); > + if (!clk) { > + kfree(parent_names); > + return -ENOMEM; > + } > + > + clk->num_parents = num_parents; > + clk->parent_names = parent_names; > + clk->hw = hw; > + clk->node = node; > + clk->type = type; > + list_add(&clk->link, &component_clks); > + > + return 0; > +} > diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h > index e45005c..96c9725 100644 > --- a/include/linux/clk/ti.h > +++ b/include/linux/clk/ti.h > @@ -137,6 +137,13 @@ struct clk_hw_omap { > /* DPLL Type and DCO Selection Flags */ > #define DPLL_J_TYPE 0x1 > > +/* Composite clock component types */ > +enum { > + CLK_COMPONENT_TYPE_MUX = 0, > + CLK_COMPONENT_TYPE_GATE, > + CLK_COMPONENT_TYPE_DIVIDER adding a CLK_COMPONENT_TYPE_MAX will allow a better implementation > +}; > + > /** > * struct ti_dt_clk - OMAP DT clock alias declarations > * @lk: clock lookup definition > @@ -179,6 +186,7 @@ int omap3_dpll4_set_rate(struct clk_hw *clk, unsigned long rate, > void ti_dt_clocks_register(struct ti_dt_clk *oclks); > void ti_dt_clk_init_provider(struct device_node *np, struct regmap *regmap); > int of_ti_autoidle_setup(struct device_node *node, struct regmap *regmap); > +int ti_clk_add_component(struct device_node *node, struct clk_hw *hw, int type); > > #ifdef CONFIG_OF > void of_ti_clk_allow_autoidle_all(void); > -- Regards, Nishanth Menon -- 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