Hi Sylwester, Thank you for the patch. On Thursday 27 March 2014 13:16:19 Sylwester Nawrocki wrote: > This function adds a helper function to configure clock parents and rates > as specified in clock-parents, clock-rates DT properties for a consumer > device and a call to it before driver is bound to a device. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > --- > Changes since v2: > - edited in clock-bindings.txt, added note about 'assigned-clocks' > subnode which may be used to specify "global" clocks configuration > at a clock provider node, > - moved of_clk_device_setup() function declaration from clk-provider.h > to clk-conf.h so required function stubs are available when > CONFIG_COMMON_CLK is not enabled, > > Changes since v1: > - the helper function to parse and set assigned clock parents and > rates made public so it is available to clock providers to call > directly; > - dropped the platform bus notification and call of_clk_device_setup() > is is now called from the driver core, rather than from the > notification callback; > - s/of_clk_get_list_entry/of_clk_get_by_property. > --- > .../devicetree/bindings/clock/clock-bindings.txt | 26 ++++++ > drivers/base/dd.c | 7 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-conf.c | 87 +++++++++++++++++ > drivers/clk/clk.c | 10 ++- > include/linux/clk/clk-conf.h | 19 +++++ > 6 files changed, 149 insertions(+), 1 deletion(-) > create mode 100644 drivers/clk/clk-conf.c > create mode 100644 include/linux/clk/clk-conf.h > > diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt > b/Documentation/devicetree/bindings/clock/clock-bindings.txt index > 7c52c29..b452f80 100644 > --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt > +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt > @@ -115,3 +115,29 @@ clock signal, and a UART. > ("pll" and "pll-switched"). > * The UART has its baud clock connected the external oscillator and its > register clock connected to the PLL clock (the "pll-switched" signal) > + > +==Assigned clock parents and rates== > + > +Some platforms require static initial configuration of parts of the clocks > +controller. Such a configuration can be specified in a clock consumer node > +through clock-parents and clock-rates DT properties. The former should > contain > +a list of parent clocks in form of phandle and clock specifier pairs, the > +latter the list of assigned clock frequency values (one cell each). > + > + uart@a000 { > + compatible = "fsl,imx-uart"; > + reg = <0xa000 0x1000>; > + ... > + clocks = <&clkcon 0>, <&clkcon 3>; > + clock-names = "baud", "mux"; > + > + clock-parents = <0>, <&pll 1>; > + clock-rates = <460800>; > + }; > + > +In this example the pll is set as parent of "mux" clock and frequency of > "baud" > +clock is specified as 460800 Hz. I'm curious, what should happen when two devices have conflicting requirements ? If a different device required the <&clkcon 3> parent to be set to <&pll 2> for instance, who should win ? Shouldn't a warning be printed ? > +For clocks which are not directly connected to any consumer device > similarly > +clocks, clock-parents and/or clock-rates properties should be specified in > +assigned-clocks subnode of a clock controller DT node. It might be that I'm not familiar enough with the clock framework, but this sounds unclear to me. I'm not sure what you mean exactly. > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 0605176..4c633e7 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -25,6 +25,7 @@ > #include <linux/async.h> > #include <linux/pm_runtime.h> > #include <linux/pinctrl/devinfo.h> > +#include <linux/clk/clk-conf.h> > > #include "base.h" > #include "power/power.h" > @@ -278,6 +279,12 @@ static int really_probe(struct device *dev, struct > device_driver *drv) if (ret) > goto probe_failed; > > + if (dev->of_node) { > + ret = of_clk_device_init(dev->of_node); > + if (ret) > + goto probe_failed; > + } > + > if (driver_sysfs_add(dev)) { > printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n", > __func__, dev_name(dev)); > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index a367a98..c720e4b 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o > obj-$(CONFIG_COMMON_CLK) += clk-gate.o > obj-$(CONFIG_COMMON_CLK) += clk-mux.o > obj-$(CONFIG_COMMON_CLK) += clk-composite.o > +obj-$(CONFIG_COMMON_CLK) += clk-conf.o > > # hardware specific clock types > # please keep this section sorted lexicographically by file/directory path > name diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c > new file mode 100644 > index 0000000..a2e992e > --- /dev/null > +++ b/drivers/clk/clk-conf.c > @@ -0,0 +1,87 @@ > +/* > + * Copyright (C) 2014 Samsung Electronics Co., Ltd. > + * Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > + * > + * 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. > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/device.h> > +#include <linux/of.h> > +#include <linux/printk.h> > + > +/** > + * of_clk_device_init() - parse and set clk configuration assigned to a > device + * @node: device node to apply the configuration for > + * > + * This function parses 'clock-parents' and 'clock-rates' properties and > sets + * any specified clock parents and rates. > + */ > +int of_clk_device_init(struct device_node *node) > +{ > + struct property *prop; > + const __be32 *cur; > + int rc, index, num_parents; > + struct clk *clk, *pclk; > + u32 rate; > + > + num_parents = of_count_phandle_with_args(node, "clock-parents", > + "#clock-cells"); > + if (num_parents == -EINVAL) > + pr_err("clk: Invalid value of clock-parents property at %s\n", > + node->full_name); > + > + for (index = 0; index < num_parents; index++) { > + pclk = of_clk_get_by_property(node, "clock-parents", index); > + if (IS_ERR(pclk)) { > + /* skip empty (null) phandles */ > + if (PTR_ERR(pclk) == -ENOENT) > + continue; > + > + pr_warn("clk: couldn't get parent clock %d for %s\n", > + index, node->full_name); > + return PTR_ERR(pclk); > + } > + > + clk = of_clk_get(node, index); > + if (IS_ERR(clk)) { > + pr_warn("clk: couldn't get clock %d for %s\n", > + index, node->full_name); > + return PTR_ERR(clk); > + } > + > + rc = clk_set_parent(clk, pclk); > + if (rc < 0) > + pr_err("clk: failed to reparent %s to %s: %d\n", > + __clk_get_name(clk), __clk_get_name(pclk), rc); > + else > + pr_debug("clk: set %s as parent of %s\n", > + __clk_get_name(pclk), __clk_get_name(clk)); > + } > + > + index = 0; > + of_property_for_each_u32(node, "clock-rates", prop, cur, rate) { > + if (rate) { > + clk = of_clk_get(node, index); > + if (IS_ERR(clk)) { > + pr_warn("clk: couldn't get clock %d for %s\n", > + index, node->full_name); > + return PTR_ERR(clk); > + } > + > + rc = clk_set_rate(clk, rate); > + if (rc < 0) > + pr_err("clk: couldn't set %s clock rate: %d\n", > + __clk_get_name(clk), rc); > + else > + pr_debug("clk: set rate of clock %s to %lu\n", > + __clk_get_name(clk), clk_get_rate(clk)); > + } > + index++; > + } > + > + return 0; > +} > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 29dc1e7..240d776 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -10,6 +10,7 @@ > */ > > #include <linux/clk-private.h> > +#include <linux/clk/clk-conf.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/spinlock.h> > @@ -2604,7 +2605,15 @@ void __init of_clk_init(const struct of_device_id > *matches) list_for_each_entry_safe(clk_provider, next, > &clk_provider_list, node) { > if (force || parent_ready(clk_provider->np)) { > + > clk_provider->clk_init_cb(clk_provider->np); > + > + /* Set any assigned clock parents and rates */ > + np = of_get_child_by_name(clk_provider->np, > + "assigned-clocks"); > + if (np) > + of_clk_device_init(np); > + > list_del(&clk_provider->node); > kfree(clk_provider); > is_init_done = true; > @@ -2619,7 +2628,6 @@ void __init of_clk_init(const struct of_device_id > *matches) */ > if (!is_init_done) > force = true; > - > } > } > #endif > diff --git a/include/linux/clk/clk-conf.h b/include/linux/clk/clk-conf.h > new file mode 100644 > index 0000000..f06b1c2 > --- /dev/null > +++ b/include/linux/clk/clk-conf.h > @@ -0,0 +1,19 @@ > +/* > + * Copyright (C) 2014 Samsung Electronics Co., Ltd. > + * Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > + * > + * 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. > + */ > + > +struct device_node; > + > +#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) > +int of_clk_device_init(struct device_node *node); > +#else > +int of_clk_device_init(struct device_node *node) > +{ > + return 0; > +} > +#endif -- Regards, Laurent Pinchart -- 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