* [RFC v2 2/9] arm/versatile*: Consolidate clk_ops and setvco implementations.
[not found] ` <1323727329-4989-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
@ 2011-12-12 22:02 ` Grant Likely
2011-12-12 22:02 ` [RFC v2 3/9] of: Add of_property_match_string() to find index into a string list Grant Likely
` (3 subsequent siblings)
4 siblings, 0 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-12 22:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Russell King, Rob Herring, Sascha Hauer, Mike Turquette
All of the icst setvco routines for ARM development boards are pretty close
to identical. Consolidate them all to one implementation.
Note: This might be broken on Integrator. To simplify the common function,
it always does a read/modify/write on the register. Integrator only does
a simple write without preserving some of the bits in the old value.
Russell, if this will break Integrator, then I can do it slightly differently
to avoid the problem.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Mike Turquette <mturquette-l0cyMroinI0@public.gmane.org>
---
arch/arm/mach-integrator/impd1.c | 30 ++-----------------------
arch/arm/mach-integrator/integrator_cp.c | 21 +----------------
arch/arm/mach-realview/core.c | 22 +-----------------
arch/arm/mach-versatile/core.c | 22 +-----------------
arch/arm/plat-versatile/clock.c | 21 ++++++++++++++++++
arch/arm/plat-versatile/include/plat/clock.h | 4 ++-
6 files changed, 33 insertions(+), 87 deletions(-)
diff --git a/arch/arm/mach-integrator/impd1.c b/arch/arm/mach-integrator/impd1.c
index 8cbb75a..a42f6bd 100644
--- a/arch/arm/mach-integrator/impd1.c
+++ b/arch/arm/mach-integrator/impd1.c
@@ -52,31 +52,6 @@ static const struct icst_params impd1_vco_params = {
.idx2s = icst525_idx2s,
};
-static void impd1_setvco(struct clk *clk, struct icst_vco vco)
-{
- struct impd1_module *impd1 = clk->data;
- u32 val = vco.v | (vco.r << 9) | (vco.s << 16);
-
- writel(0xa05f, impd1->base + IMPD1_LOCK);
- writel(val, clk->vcoreg);
- writel(0, impd1->base + IMPD1_LOCK);
-
-#ifdef DEBUG
- vco.v = val & 0x1ff;
- vco.r = (val >> 9) & 0x7f;
- vco.s = (val >> 16) & 7;
-
- pr_debug("IM-PD1: VCO%d clock is %ld Hz\n",
- vconr, icst525_hz(&impd1_vco_params, vco));
-#endif
-}
-
-static const struct clk_ops impd1_clk_ops = {
- .round = icst_clk_round,
- .set = icst_clk_set,
- .setvco = impd1_setvco,
-};
-
void impd1_tweak_control(struct device *dev, u32 mask, u32 val)
{
struct impd1_module *impd1 = dev_get_drvdata(dev);
@@ -377,13 +352,14 @@ static int impd1_probe(struct lm_device *dev)
(unsigned long)dev->resource.start);
for (i = 0; i < ARRAY_SIZE(impd1->vcos); i++) {
- impd1->vcos[i].ops = &impd1_clk_ops,
+ impd1->vcos[i].ops = &icst_clk_default_ops,
impd1->vcos[i].owner = THIS_MODULE,
impd1->vcos[i].params = &impd1_vco_params,
- impd1->vcos[i].data = impd1;
}
impd1->vcos[0].vcoreg = impd1->base + IMPD1_OSC1;
+ impd1->vcos[0].lockreg = impd1->base + IMPD1_LOCK;
impd1->vcos[1].vcoreg = impd1->base + IMPD1_OSC2;
+ impd1->vcos[1].lockreg = impd1->base + IMPD1_LOCK;
impd1->clks[0] = clkdev_alloc(&impd1->vcos[0], NULL, "lm%x:01000",
dev->id);
diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index 5de49c3..3342926 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -205,28 +205,11 @@ static const struct icst_params cp_auxvco_params = {
.idx2s = icst525_idx2s,
};
-static void cp_auxvco_set(struct clk *clk, struct icst_vco vco)
-{
- u32 val;
-
- val = readl(clk->vcoreg) & ~0x7ffff;
- val |= vco.v | (vco.r << 9) | (vco.s << 16);
-
- writel(0xa05f, CM_LOCK);
- writel(val, clk->vcoreg);
- writel(0, CM_LOCK);
-}
-
-static const struct clk_ops cp_auxclk_ops = {
- .round = icst_clk_round,
- .set = icst_clk_set,
- .setvco = cp_auxvco_set,
-};
-
static struct clk cp_auxclk = {
- .ops = &cp_auxclk_ops,
+ .ops = &icst_clk_default_ops,
.params = &cp_auxvco_params,
.vcoreg = CM_AUXOSC,
+ .lockreg= CM_LOCK,
};
static struct clk sp804_clk = {
diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index d5ed5d4..41b2f91 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -242,27 +242,8 @@ static const struct icst_params realview_oscvco_params = {
.idx2s = icst307_idx2s,
};
-static void realview_oscvco_set(struct clk *clk, struct icst_vco vco)
-{
- void __iomem *sys_lock = __io_address(REALVIEW_SYS_BASE) + REALVIEW_SYS_LOCK_OFFSET;
- u32 val;
-
- val = readl(clk->vcoreg) & ~0x7ffff;
- val |= vco.v | (vco.r << 9) | (vco.s << 16);
-
- writel(0xa05f, sys_lock);
- writel(val, clk->vcoreg);
- writel(0, sys_lock);
-}
-
-static const struct clk_ops oscvco_clk_ops = {
- .round = icst_clk_round,
- .set = icst_clk_set,
- .setvco = realview_oscvco_set,
-};
-
static struct clk oscvco_clk = {
- .ops = &oscvco_clk_ops,
+ .ops = &icst_clk_default_ops,
.params = &realview_oscvco_params,
};
@@ -333,6 +314,7 @@ void __init realview_init_early(void)
oscvco_clk.vcoreg = sys + REALVIEW_SYS_OSC0_OFFSET;
else
oscvco_clk.vcoreg = sys + REALVIEW_SYS_OSC4_OFFSET;
+ oscvco_clk.lockreg = sys + REALVIEW_SYS_LOCK_OFFSET;
clkdev_add_table(lookups, ARRAY_SIZE(lookups));
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index e340a54..47f4531 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -337,27 +337,8 @@ static const struct icst_params versatile_oscvco_params = {
.idx2s = icst307_idx2s,
};
-static void versatile_oscvco_set(struct clk *clk, struct icst_vco vco)
-{
- void __iomem *sys_lock = __io_address(VERSATILE_SYS_BASE) + VERSATILE_SYS_LOCK_OFFSET;
- u32 val;
-
- val = readl(clk->vcoreg) & ~0x7ffff;
- val |= vco.v | (vco.r << 9) | (vco.s << 16);
-
- writel(0xa05f, sys_lock);
- writel(val, clk->vcoreg);
- writel(0, sys_lock);
-}
-
-static const struct clk_ops osc4_clk_ops = {
- .round = icst_clk_round,
- .set = icst_clk_set,
- .setvco = versatile_oscvco_set,
-};
-
static struct clk osc4_clk = {
- .ops = &osc4_clk_ops,
+ .ops = &icst_clk_default_ops,
.params = &versatile_oscvco_params,
};
@@ -751,6 +732,7 @@ void __init versatile_init_early(void)
void __iomem *sys = __io_address(VERSATILE_SYS_BASE);
osc4_clk.vcoreg = sys + VERSATILE_SYS_OSCCLCD_OFFSET;
+ osc4_clk.lockreg = sys + VERSATILE_SYS_LOCK_OFFSET;
clkdev_add_table(lookups, ARRAY_SIZE(lookups));
versatile_sched_clock_init(sys + VERSATILE_SYS_24MHz_OFFSET, 24000000);
diff --git a/arch/arm/plat-versatile/clock.c b/arch/arm/plat-versatile/clock.c
index 5c8b656..98a9dd8 100644
--- a/arch/arm/plat-versatile/clock.c
+++ b/arch/arm/plat-versatile/clock.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/clk.h>
+#include <linux/io.h>
#include <linux/mutex.h>
#include <asm/hardware/icst.h>
@@ -53,6 +54,19 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
}
EXPORT_SYMBOL(clk_set_rate);
+void icst_clk_setvco(struct clk *clk, struct icst_vco vco)
+{
+ u32 val;
+
+ val = readl(clk->vcoreg) & ~0x7ffff;
+ val |= vco.v | (vco.r << 9) | (vco.s << 16);
+
+ writel(0xa05f, clk->lockreg);
+ writel(val, clk->vcoreg);
+ writel(0, clk->lockreg);
+}
+EXPORT_SYMBOL(icst_clk_setvco);
+
long icst_clk_round(struct clk *clk, unsigned long rate)
{
struct icst_vco vco;
@@ -72,3 +86,10 @@ int icst_clk_set(struct clk *clk, unsigned long rate)
return 0;
}
EXPORT_SYMBOL(icst_clk_set);
+
+const struct clk_ops icst_clk_default_ops = {
+ .round = icst_clk_round,
+ .set = icst_clk_set,
+ .setvco = icst_clk_setvco,
+};
+EXPORT_SYMBOL(icst_clk_default_ops);
diff --git a/arch/arm/plat-versatile/include/plat/clock.h b/arch/arm/plat-versatile/include/plat/clock.h
index 2117701..5749f79 100644
--- a/arch/arm/plat-versatile/include/plat/clock.h
+++ b/arch/arm/plat-versatile/include/plat/clock.h
@@ -10,9 +10,9 @@ struct clk {
const struct clk_ops *ops;
const struct icst_params *params;
void __iomem *vcoreg;
+ void __iomem *lockreg;
#ifdef CONFIG_ARCH_INTEGRATOR
struct module *owner;
- void *data;
#endif
};
@@ -22,8 +22,10 @@ struct clk_ops {
void (*setvco)(struct clk *, struct icst_vco);
};
+void icst_clk_setvco(struct clk *clk, struct icst_vco vco);
int icst_clk_set(struct clk *, unsigned long);
long icst_clk_round(struct clk *, unsigned long);
+extern const struct clk_ops icst_clk_default_ops;
#ifdef CONFIG_ARCH_INTEGRATOR
static inline int __clk_get(struct clk *clk)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v2 3/9] of: Add of_property_match_string() to find index into a string list
[not found] ` <1323727329-4989-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-12-12 22:02 ` [RFC v2 2/9] arm/versatile*: Consolidate clk_ops and setvco implementations Grant Likely
@ 2011-12-12 22:02 ` Grant Likely
2011-12-12 22:02 ` [RFC v2 4/9] of: add clock providers Grant Likely
` (2 subsequent siblings)
4 siblings, 0 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-12 22:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Rob Herring, Sascha Hauer, Mike Turquette
Add a helper function for finding the index of a string in a string
list property. This helper is useful for bindings that use a separate
*-name property for attaching names to tuples in another property such
as 'reg' or 'gpios'.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
---
arch/arm/boot/dts/testcases/tests-phandle.dtsi | 2 +
drivers/of/base.c | 36 ++++++++++++++++++++++++
drivers/of/selftest.c | 29 +++++++++++++++++++
include/linux/of.h | 3 ++
4 files changed, 70 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/testcases/tests-phandle.dtsi b/arch/arm/boot/dts/testcases/tests-phandle.dtsi
index ec0c4e6..0007d3c 100644
--- a/arch/arm/boot/dts/testcases/tests-phandle.dtsi
+++ b/arch/arm/boot/dts/testcases/tests-phandle.dtsi
@@ -31,6 +31,8 @@
phandle-list-bad-phandle = <12345678 0 0>;
phandle-list-bad-args = <&provider2 1 0>,
<&provider3 0>;
+ empty-property;
+ unterminated-string = [40 41 42 43];
};
};
};
diff --git a/drivers/of/base.c b/drivers/of/base.c
index c6db9ab..ff3c1fb 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -761,6 +761,42 @@ int of_property_read_string_index(struct device_node *np, const char *propname,
}
EXPORT_SYMBOL_GPL(of_property_read_string_index);
+/**
+ * of_property_match_string() - Find string in a list and return index
+ * @np: pointer to node containing string list property
+ * @propname: string list property name
+ * @string: pointer to string to search for in string list
+ *
+ * This function searches a string list property and returns the index
+ * of a specific string value.
+ */
+int of_property_match_string(struct device_node *np, const char *propname,
+ const char *string)
+{
+ struct property *prop = of_find_property(np, propname, NULL);
+ size_t l;
+ int i;
+ const char *p, *end;
+
+ if (!prop)
+ return -EINVAL;
+ if (!prop->value)
+ return -ENODATA;
+
+ p = prop->value;
+ end = p + prop->length;
+
+ for (i = 0; p < end; i++, p += l) {
+ l = strlen(p) + 1;
+ if (p + l > end)
+ return -EILSEQ;
+ pr_debug("comparing %s with %s\n", string, p);
+ if (strcmp(string, p) == 0)
+ return i; /* Found it; return index */
+ }
+ return -ENODATA;
+}
+EXPORT_SYMBOL_GPL(of_property_match_string);
/**
* of_property_count_strings - Find and return the number of strings from a
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index 9d2b480..f24ffd7 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -120,6 +120,34 @@ static void __init of_selftest_parse_phandle_with_args(void)
pr_info("end - %s\n", passed_all ? "PASS" : "FAIL");
}
+static void __init of_selftest_property_match_string(void)
+{
+ struct device_node *np;
+ int rc;
+
+ pr_info("start\n");
+ np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a");
+ if (!np) {
+ pr_err("No testcase data in device tree\n");
+ return;
+ }
+
+ rc = of_property_match_string(np, "phandle-list-names", "first");
+ selftest(rc == 0, "first expected:0 got:%i\n", rc);
+ rc = of_property_match_string(np, "phandle-list-names", "second");
+ selftest(rc == 1, "second expected:0 got:%i\n", rc);
+ rc = of_property_match_string(np, "phandle-list-names", "third");
+ selftest(rc == 2, "third expected:0 got:%i\n", rc);
+ rc = of_property_match_string(np, "phandle-list-names", "fourth");
+ selftest(rc == -ENODATA, "unmatched string; rc=%i", rc);
+ rc = of_property_match_string(np, "missing-property", "blah");
+ selftest(rc == -EINVAL, "missing property; rc=%i", rc);
+ rc = of_property_match_string(np, "empty-property", "blah");
+ selftest(rc == -ENODATA, "empty property; rc=%i", rc);
+ rc = of_property_match_string(np, "unterminated-string", "blah");
+ selftest(rc == -EILSEQ, "unterminated string; rc=%i", rc);
+}
+
static int __init of_selftest(void)
{
struct device_node *np;
@@ -133,6 +161,7 @@ static int __init of_selftest(void)
pr_info("start of selftest - you will see error messages\n");
of_selftest_parse_phandle_with_args();
+ of_selftest_property_match_string();
pr_info("end of selftest - %s\n", selftest_passed ? "PASS" : "FAIL");
return 0;
}
diff --git a/include/linux/of.h b/include/linux/of.h
index ea44fd7..f02794e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -218,6 +218,9 @@ extern int of_property_read_string(struct device_node *np,
extern int of_property_read_string_index(struct device_node *np,
const char *propname,
int index, const char **output);
+extern int of_property_match_string(struct device_node *np,
+ const char *propname,
+ const char *string);
extern int of_property_count_strings(struct device_node *np,
const char *propname);
extern int of_device_is_compatible(const struct device_node *device,
--
1.7.5.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* [RFC v2 4/9] of: add clock providers
[not found] ` <1323727329-4989-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-12-12 22:02 ` [RFC v2 2/9] arm/versatile*: Consolidate clk_ops and setvco implementations Grant Likely
2011-12-12 22:02 ` [RFC v2 3/9] of: Add of_property_match_string() to find index into a string list Grant Likely
@ 2011-12-12 22:02 ` Grant Likely
[not found] ` <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2012-01-17 20:44 ` Stephen Warren
2011-12-12 22:02 ` [RFC v2 6/9] arm/dt: add devicetree support to sp804 timer support Grant Likely
2011-12-12 22:02 ` [RFC v2 9/9] arm/highbank: Use clock binding common support code Grant Likely
4 siblings, 2 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-12 22:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Rob Herring, Sascha Hauer, Mike Turquette
Based on work by Ben Herrenschmidt and Jeremy Kerr, this patch adds an
of_clk_get function to allow platforms to retrieve clock data from the
device tree.
Platform register a provider through of_clk_add_provider, which will be
called when a device references the provider's OF node for a clock
reference.
v2: - fixed errant ';' causing compile error
- Editorial fixes from Shawn Guo
- merged in adding lookup to clkdev
- changed property names to match established convention. After
working with the binding a bit it really made more sense to follow the
lead of 'reg', 'gpios' and 'interrupts' by making the input simply
'clocks' & 'clock-names' instead of 'clock-input-*', and to only use
clock-output* for the producer nodes. (Sorry Shawn, this will mean
you need to change some code, but it should be trivial)
- Add ability to inherit clocks from parent nodes by using an empty
'clock-ranges' property. Useful for busses. I could use some feedback
on the new property name, 'clock-ranges' doesn't feel right to me.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Reviewed-by: Shawn Guo <shawn.guo-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Mike Turquette <mturquette-l0cyMroinI0@public.gmane.org>
---
.../devicetree/bindings/clock/clock-bindings.txt | 114 ++++++++++++++
.../devicetree/bindings/clock/fixed-clock.txt | 21 +++
drivers/clk/clkdev.c | 9 +
drivers/of/Kconfig | 6 +
drivers/of/Makefile | 1 +
drivers/of/clock.c | 165 ++++++++++++++++++++
include/linux/of_clk.h | 37 +++++
7 files changed, 353 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt
create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
create mode 100644 drivers/of/clock.c
create mode 100644 include/linux/of_clk.h
diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
new file mode 100644
index 0000000..e40c436
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -0,0 +1,114 @@
+This binding is a work-in-progress, and are based on some experimental
+work by benh[1].
+
+Sources of clock signal can be represented by any node in the device
+tree. Those nodes are designated as clock providers. Clock consumer
+nodes use a phandle and clock specifier pair to connect clock provider
+outputs to clock inputs. Similar to the gpio specifiers, a clock
+specifier is an array of one more more cells identifying the clock
+output on a device. The length of a clock specifier is defined by the
+value of a #clock-cells property in the clock provider node.
+
+[1] http://patchwork.ozlabs.org/patch/31551/
+
+==Clock providers==
+
+Required properties:
+#clock-cells: Number of cells in a clock specifier; typically will be
+ set to 1
+
+Optional properties:
+clock-output-names: Recommended to be a list of strings of clock output signal
+ names indexed by the first cell in the clock specifier.
+ However, the meaning of clock-output-name is domain
+ specific to the clock provider, and is only provided to
+ encourage using the same meaning for the majority of clock
+ providers. This format may not work for clock providers
+ using a complex clock specifier format. In those cases it
+ is recommended to omit this property and create a binding
+ specific names property.
+
+ Clock consumer nodes must never directly reference
+ the provider's clock-output-name property.
+
+For example:
+
+ oscillator {
+ #clock-cells = <1>;
+ clock-output-names = "ckil", "ckih";
+ };
+
+- this node defines a device with two clock outputs, the first named
+ "ckil" and the second named "ckih". Consumer nodes always reference
+ clocks by index. The names should reflect the clock output signal
+ names for the device.
+
+==Clock consumers==
+
+Required properties:
+clocks: List of phandle and clock specifier pairs, one pair
+ for each clock input to the device.
+clock-names: List of clock input name strings sorted in the same
+ order as the clocks property. Consumers drivers
+ will use clock-names to match clock input names
+ with clocks specifiers.
+
+Optional properties:
+clock-ranges: Empty property indicating that child nodes can inherit named
+ clocks from this node. Useful for bus nodes to provide a
+ clock to their children.
+
+For example:
+
+ device {
+ clocks = <&osc 1>, <&ref 0>;
+ clock-names = "baud", "register";
+ };
+
+
+This represents a device with two clock inputs, named "baud" and "register".
+The baud clock is connected to output 1 of the &osc device, and the register
+clock is connected to output 0 of the &ref.
+
+==Example==
+
+ /* external oscillator */
+ osc: oscillator {
+ compatible = "fixed-clock";
+ #clock-cells = <1>;
+ clock-frequency = <32678>;
+ clock-output-names = "osc";
+ };
+
+ /* phase-locked-loop device, generates a higher frequency clock
+ * from the external oscillator reference */
+ pll: pll@4c000 {
+ compatible = "vendor,some-pll-interface"
+ #clock-cells = <1>;
+ clocks = <&osc 0>;
+ clock-names = "ref";
+ reg = <0x4c000 0x1000>;
+ clock-output-names = "pll", "pll-switched";
+ };
+
+ /* UART, using the low frequency oscillator for the baud clock,
+ * and the high frequency switched PLL output for register
+ * clocking */
+ uart@a000 {
+ compatible = "fsl,imx-uart";
+ reg = <0xa000 0x1000>;
+ interrupts = <33>;
+ clocks = <&osc 0>, <&pll 1>;
+ clock-names = "baud", "register";
+ };
+
+This DT fragment defines three devices: an external oscillator to provide a
+low-frequency reference clock, a PLL device to generate a higher frequency
+clock signal, and a UART.
+
+* The oscillator is fixed-frequency, and provides one clock output, named "osc".
+* The PLL is both a clock provider and a clock consumer. It uses the clock
+ signal generated by the external oscillator, and provides two output signals
+ ("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)
diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt b/Documentation/devicetree/bindings/clock/fixed-clock.txt
new file mode 100644
index 0000000..9a75342
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/fixed-clock.txt
@@ -0,0 +1,21 @@
+Binding for simple fixed-rate clock sources.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/fixed-clock.txt
+
+Required properties:
+- compatible : shall be "fixed-clock".
+- #clock-cells : from common clock binding; shall be set to 0.
+- clock-frequency : frequency of clock in Hz. May be multiple cells.
+
+Optional properties:
+- gpios : From common gpio binding; gpio connection to clock enable pin.
+- clock-output-names : From common clock binding
+
+Example:
+ clock {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1000000000>;
+ };
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6db161f..43628d0 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -19,6 +19,8 @@
#include <linux/mutex.h>
#include <linux/clk.h>
#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_clk.h>
static LIST_HEAD(clocks);
static DEFINE_MUTEX(clocks_mutex);
@@ -78,6 +80,13 @@ EXPORT_SYMBOL(clk_get_sys);
struct clk *clk_get(struct device *dev, const char *con_id)
{
const char *dev_id = dev ? dev_name(dev) : NULL;
+ struct clk *clk;
+
+ if (dev) {
+ clk = of_clk_get_by_name(dev->of_node, con_id);
+ if (clk && __clk_get(clk))
+ return clk;
+ }
return clk_get_sys(dev_id, con_id);
}
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 268163d..b49ab9c 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -47,6 +47,12 @@ config OF_IRQ
def_bool y
depends on !SPARC
+config OF_CLOCK
+ def_bool y
+ depends on HAVE_CLK
+ help
+ OpenFirmware clock accessors
+
config OF_DEVICE
def_bool y
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index a73f5a5..e7ad1e9 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_OF_ADDRESS) += address.o
obj-$(CONFIG_OF_IRQ) += irq.o
obj-$(CONFIG_OF_DEVICE) += device.o platform.o
obj-$(CONFIG_OF_GPIO) += gpio.o
+obj-$(CONFIG_OF_CLOCK) += clock.o
obj-$(CONFIG_OF_I2C) += of_i2c.o
obj-$(CONFIG_OF_NET) += of_net.o
obj-$(CONFIG_OF_SPI) += of_spi.o
diff --git a/drivers/of/clock.c b/drivers/of/clock.c
new file mode 100644
index 0000000..6519e96
--- /dev/null
+++ b/drivers/of/clock.c
@@ -0,0 +1,165 @@
+/*
+ * Clock infrastructure for device tree platforms
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_clk.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+
+/**
+ * struct of_clk_provider - Clock provider registration structure
+ * @link: Entry in global list of clock providers
+ * @node: Pointer to device tree node of clock provider
+ * @get: Get clock callback. Returns NULL or a struct clk for the
+ * given clock specifier
+ * @data: context pointer to be passed into @get callback
+ */
+struct of_clk_provider {
+ struct list_head link;
+
+ struct device_node *node;
+ struct clk *(*get)(struct of_phandle_args *clkspec, void *data);
+ void *data;
+};
+
+static LIST_HEAD(of_clk_providers);
+static DEFINE_MUTEX(of_clk_lock);
+
+/**
+ * of_clk_add_provider() - Register a clock provider for a node
+ * @np: Device node pointer associated with clock provider
+ * @clk_src_get: callback for decoding clock
+ * @data: context pointer for @clk_src_get callback.
+ */
+int of_clk_add_provider(struct device_node *np,
+ struct clk *(*clk_src_get)(struct of_phandle_args *clkspec,
+ void *data),
+ void *data)
+{
+ struct of_clk_provider *cp;
+
+ cp = kzalloc(sizeof(struct of_clk_provider), GFP_KERNEL);
+ if (!cp)
+ return -ENOMEM;
+
+ cp->node = of_node_get(np);
+ cp->data = data;
+ cp->get = clk_src_get;
+
+ mutex_lock(&of_clk_lock);
+ list_add(&cp->link, &of_clk_providers);
+ mutex_unlock(&of_clk_lock);
+ pr_debug("Added clock from %s\n", np->full_name);
+
+ return 0;
+}
+
+/**
+ * of_clk_del_provider() - Remove a previously registered clock provider
+ * @np: Device node pointer associated with clock provider
+ */
+void of_clk_del_provider(struct device_node *np)
+{
+ struct of_clk_provider *cp;
+
+ mutex_lock(&of_clk_lock);
+ list_for_each_entry(cp, &of_clk_providers, link) {
+ if (cp->node == np) {
+ list_del(&cp->link);
+ of_node_put(cp->node);
+ kfree(cp);
+ break;
+ }
+ }
+ mutex_unlock(&of_clk_lock);
+}
+
+static struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec)
+{
+ struct of_clk_provider *provider;
+ struct clk *clk = NULL;
+
+ /* Check if we have such a provider in our array */
+ mutex_lock(&of_clk_lock);
+ list_for_each_entry(provider, &of_clk_providers, link) {
+ if (provider->node == clkspec->np)
+ clk = provider->get(clkspec, provider->data);
+ if (clk)
+ break;
+ }
+ mutex_unlock(&of_clk_lock);
+
+ return clk;
+}
+
+struct clk *of_clk_get(struct device_node *np, int index)
+{
+ struct of_phandle_args clkspec;
+ struct clk *clk;
+ int rc;
+
+ if (index < 0)
+ return NULL;
+
+ rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
+ &clkspec);
+ if (rc)
+ return NULL;
+
+ clk = __of_clk_get_from_provider(&clkspec);
+ of_node_put(clkspec.np);
+ return clk;
+}
+
+/**
+ * of_clk_get_by_name() - Parse and lookup a clock referenced by a device node
+ * @np: pointer to clock consumer node
+ * @name: name of consumer's clock input, or NULL for the first clock reference
+ *
+ * This function parses the clocks and clock-names properties,
+ * and uses them to look up the struct clk from the registered list of clock
+ * providers.
+ */
+struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
+{
+ struct clk *clk = NULL;
+
+ /* Walk up the tree of devices looking for a clock that matches */
+ while (np) {
+ int index = 0;
+
+ /*
+ * For named clocks, first look up the name in the
+ * "clock-names" property. If it cannot be found, then
+ * index will be an error code, and of_clk_get() will fail.
+ */
+ if (name)
+ index = of_property_match_string(np, "clock-names", name);
+ clk = of_clk_get(np, index);
+ if (clk)
+ break;
+ else if (name && index >= 0) {
+ pr_err("ERROR: could not get clock %s:%s(%i)\n",
+ np->full_name, name ? name : "", index);
+ return NULL;
+ }
+
+ /*
+ * No matching clock found on this node. If the parent node
+ * has a "clock-ranges" property, then we can try one of its
+ * clocks.
+ */
+ np = np->parent;
+ if (np && !of_get_property(np, "clock-ranges", NULL))
+ break;
+ }
+
+ return clk;
+}
diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h
new file mode 100644
index 0000000..dcbd27b
--- /dev/null
+++ b/include/linux/of_clk.h
@@ -0,0 +1,37 @@
+/*
+ * Clock infrastructure for device tree platforms
+ */
+#ifndef __OF_CLK_H
+#define __OF_CLK_H
+
+struct device;
+struct clk;
+
+#ifdef CONFIG_OF_CLOCK
+
+struct device_node;
+
+int of_clk_add_provider(struct device_node *np,
+ struct clk *(*clk_src_get)(struct of_phandle_args *args,
+ void *data),
+ void *data);
+
+void of_clk_del_provider(struct device_node *np);
+
+struct clk *of_clk_get(struct device_node *np, int index);
+struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
+
+#else
+static struct clk *of_clk_get(struct device_node *np, int index)
+{
+ return NULL;
+}
+
+static struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
+{
+ return NULL;
+}
+#endif
+
+#endif /* __OF_CLK_H */
+
--
1.7.5.4
^ permalink raw reply related [flat|nested] 39+ messages in thread[parent not found: <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]
* Re: [RFC v2 4/9] of: add clock providers
[not found] ` <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
@ 2011-12-12 23:29 ` Jamie Iles
2011-12-13 17:54 ` Grant Likely
2012-01-10 21:33 ` Jamie Iles
2012-01-13 13:50 ` Shawn Guo
2 siblings, 1 reply; 39+ messages in thread
From: Jamie Iles @ 2011-12-12 23:29 UTC (permalink / raw)
To: Grant Likely
Cc: Sascha Hauer, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mike Turquette
Hi Grant,
I'm still going through these and trying to digest them but a couple of
quick questions/comments.
Jamie
On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> new file mode 100644
> index 0000000..e40c436
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -0,0 +1,114 @@
> +This binding is a work-in-progress, and are based on some experimental
> +work by benh[1].
> +
> +Sources of clock signal can be represented by any node in the device
> +tree. Those nodes are designated as clock providers. Clock consumer
> +nodes use a phandle and clock specifier pair to connect clock provider
> +outputs to clock inputs. Similar to the gpio specifiers, a clock
> +specifier is an array of one more more cells identifying the clock
> +output on a device. The length of a clock specifier is defined by the
> +value of a #clock-cells property in the clock provider node.
> +
> +[1] http://patchwork.ozlabs.org/patch/31551/
> +
> +==Clock providers==
> +
> +Required properties:
> +#clock-cells: Number of cells in a clock specifier; typically will be
> + set to 1
I'm not sure I fully understand what the extra cells actually mean for
clocks. I think the first integer is the clock output to use but some
of the versatile and highbank ones only have a phandle or is it more
implementation defined? The clock-output-names description hints at
recommended, so I find this a little confusing, but that could just be
me!
> +Optional properties:
> +clock-output-names: Recommended to be a list of strings of clock output signal
> + names indexed by the first cell in the clock specifier.
> + However, the meaning of clock-output-name is domain
> + specific to the clock provider, and is only provided to
> + encourage using the same meaning for the majority of clock
> + providers. This format may not work for clock providers
> + using a complex clock specifier format. In those cases it
> + is recommended to omit this property and create a binding
> + specific names property.
> +
> + Clock consumer nodes must never directly reference
> + the provider's clock-output-name property.
> +
> +For example:
> +
> + oscillator {
> + #clock-cells = <1>;
> + clock-output-names = "ckil", "ckih";
> + };
> +
> +- this node defines a device with two clock outputs, the first named
> + "ckil" and the second named "ckih". Consumer nodes always reference
> + clocks by index. The names should reflect the clock output signal
> + names for the device.
> +
> +==Clock consumers==
> +
> +Required properties:
> +clocks: List of phandle and clock specifier pairs, one pair
> + for each clock input to the device.
Some of the highbank and versatile devicetree nodes have clocks
properties that aren't a pair e.g. versatile timer has
"clocks = <&tim_clk>;".
> +clock-names: List of clock input name strings sorted in the same
> + order as the clocks property. Consumers drivers
> + will use clock-names to match clock input names
> + with clocks specifiers.
The versatile and highbank patches appears to omit this required
property in several nodes. So is this really optional?
Jamie
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2011-12-12 23:29 ` Jamie Iles
@ 2011-12-13 17:54 ` Grant Likely
2011-12-13 18:01 ` Rob Herring
2011-12-15 13:51 ` Shawn Guo
0 siblings, 2 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-13 17:54 UTC (permalink / raw)
To: Jamie Iles
Cc: Sascha Hauer, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mike Turquette
On Mon, Dec 12, 2011 at 4:29 PM, Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org> wrote:
> Hi Grant,
>
> I'm still going through these and trying to digest them but a couple of
> quick questions/comments.
>
> Jamie
>
> On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>> new file mode 100644
>> index 0000000..e40c436
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>> @@ -0,0 +1,114 @@
>> +This binding is a work-in-progress, and are based on some experimental
>> +work by benh[1].
>> +
>> +Sources of clock signal can be represented by any node in the device
>> +tree. Those nodes are designated as clock providers. Clock consumer
>> +nodes use a phandle and clock specifier pair to connect clock provider
>> +outputs to clock inputs. Similar to the gpio specifiers, a clock
>> +specifier is an array of one more more cells identifying the clock
>> +output on a device. The length of a clock specifier is defined by the
>> +value of a #clock-cells property in the clock provider node.
>> +
>> +[1] http://patchwork.ozlabs.org/patch/31551/
>> +
>> +==Clock providers==
>> +
>> +Required properties:
>> +#clock-cells: Number of cells in a clock specifier; typically will be
>> + set to 1
>
> I'm not sure I fully understand what the extra cells actually mean for
> clocks. I think the first integer is the clock output to use but some
> of the versatile and highbank ones only have a phandle or is it more
> implementation defined? The clock-output-names description hints at
> recommended, so I find this a little confusing, but that could just be
> me!
I'm following convention here that has been established with
interrupts, gpios, and others. Sometimes more information is needed
that just the clock number. Using #clock-cells gives a clock provider
the option of having additional fields for clock flags or other data.
This is very much implementation defined. Simple clock providers that
only have a single clock output can easily use #clock-cells = <0>.
Providers with multiple outputs will need to use 1 or more cells.
>> +Optional properties:
>> +clock-output-names: Recommended to be a list of strings of clock output signal
>> + names indexed by the first cell in the clock specifier.
>> + However, the meaning of clock-output-name is domain
>> + specific to the clock provider, and is only provided to
>> + encourage using the same meaning for the majority of clock
>> + providers. This format may not work for clock providers
>> + using a complex clock specifier format. In those cases it
>> + is recommended to omit this property and create a binding
>> + specific names property.
>> +
>> + Clock consumer nodes must never directly reference
>> + the provider's clock-output-name property.
>> +
>> +For example:
>> +
>> + oscillator {
>> + #clock-cells = <1>;
>> + clock-output-names = "ckil", "ckih";
>> + };
>> +
>> +- this node defines a device with two clock outputs, the first named
>> + "ckil" and the second named "ckih". Consumer nodes always reference
>> + clocks by index. The names should reflect the clock output signal
>> + names for the device.
>> +
>> +==Clock consumers==
>> +
>> +Required properties:
>> +clocks: List of phandle and clock specifier pairs, one pair
>> + for each clock input to the device.
>
> Some of the highbank and versatile devicetree nodes have clocks
> properties that aren't a pair e.g. versatile timer has
> "clocks = <&tim_clk>;".
It's still a pair.... it's just that the specifier portion has a zero
length. :-) I do agree that the documentation needs work though.
>
>> +clock-names: List of clock input name strings sorted in the same
>> + order as the clocks property. Consumers drivers
>> + will use clock-names to match clock input names
>> + with clocks specifiers.
>
> The versatile and highbank patches appears to omit this required
> property in several nodes. So is this really optional?
You're right, it's not required. I'll move it to optional.
g.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2011-12-13 17:54 ` Grant Likely
@ 2011-12-13 18:01 ` Rob Herring
2011-12-13 18:03 ` Grant Likely
2011-12-15 13:51 ` Shawn Guo
1 sibling, 1 reply; 39+ messages in thread
From: Rob Herring @ 2011-12-13 18:01 UTC (permalink / raw)
To: Grant Likely
Cc: Jamie Iles, linux-kernel, devicetree-discuss, Sascha Hauer,
Mike Turquette
Grant,
On 12/13/2011 11:54 AM, Grant Likely wrote:
> On Mon, Dec 12, 2011 at 4:29 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>> Hi Grant,
>>
>> I'm still going through these and trying to digest them but a couple of
>> quick questions/comments.
>>
>> Jamie
>>
>> On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>>> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>> new file mode 100644
>>> index 0000000..e40c436
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>> @@ -0,0 +1,114 @@
>>> +This binding is a work-in-progress, and are based on some experimental
>>> +work by benh[1].
>>> +
>>> +Sources of clock signal can be represented by any node in the device
>>> +tree. Those nodes are designated as clock providers. Clock consumer
>>> +nodes use a phandle and clock specifier pair to connect clock provider
>>> +outputs to clock inputs. Similar to the gpio specifiers, a clock
>>> +specifier is an array of one more more cells identifying the clock
>>> +output on a device. The length of a clock specifier is defined by the
>>> +value of a #clock-cells property in the clock provider node.
>>> +
>>> +[1] http://patchwork.ozlabs.org/patch/31551/
>>> +
>>> +==Clock providers==
>>> +
>>> +Required properties:
>>> +#clock-cells: Number of cells in a clock specifier; typically will be
>>> + set to 1
>>
>> I'm not sure I fully understand what the extra cells actually mean for
>> clocks. I think the first integer is the clock output to use but some
>> of the versatile and highbank ones only have a phandle or is it more
>> implementation defined? The clock-output-names description hints at
>> recommended, so I find this a little confusing, but that could just be
>> me!
>
> I'm following convention here that has been established with
> interrupts, gpios, and others. Sometimes more information is needed
> that just the clock number. Using #clock-cells gives a clock provider
> the option of having additional fields for clock flags or other data.
> This is very much implementation defined. Simple clock providers that
> only have a single clock output can easily use #clock-cells = <0>.
> Providers with multiple outputs will need to use 1 or more cells.
>
Aren't you off by 1 here? The minimum cells is 1 to hold the phandle of
the source/parent. Multiple outputs will need a cell size of 2 (typically).
Rob
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 4/9] of: add clock providers
2011-12-13 18:01 ` Rob Herring
@ 2011-12-13 18:03 ` Grant Likely
0 siblings, 0 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-13 18:03 UTC (permalink / raw)
To: Rob Herring
Cc: Jamie Iles, linux-kernel, devicetree-discuss, Sascha Hauer,
Mike Turquette
On Tue, Dec 13, 2011 at 11:01 AM, Rob Herring <robherring2@gmail.com> wrote:
> Grant,
>
> On 12/13/2011 11:54 AM, Grant Likely wrote:
>> On Mon, Dec 12, 2011 at 4:29 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>>> Hi Grant,
>>>
>>> I'm still going through these and trying to digest them but a couple of
>>> quick questions/comments.
>>>
>>> Jamie
>>>
>>> On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>>>> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> new file mode 100644
>>>> index 0000000..e40c436
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> @@ -0,0 +1,114 @@
>>>> +This binding is a work-in-progress, and are based on some experimental
>>>> +work by benh[1].
>>>> +
>>>> +Sources of clock signal can be represented by any node in the device
>>>> +tree. Those nodes are designated as clock providers. Clock consumer
>>>> +nodes use a phandle and clock specifier pair to connect clock provider
>>>> +outputs to clock inputs. Similar to the gpio specifiers, a clock
>>>> +specifier is an array of one more more cells identifying the clock
>>>> +output on a device. The length of a clock specifier is defined by the
>>>> +value of a #clock-cells property in the clock provider node.
>>>> +
>>>> +[1] http://patchwork.ozlabs.org/patch/31551/
>>>> +
>>>> +==Clock providers==
>>>> +
>>>> +Required properties:
>>>> +#clock-cells: Number of cells in a clock specifier; typically will be
>>>> + set to 1
>>>
>>> I'm not sure I fully understand what the extra cells actually mean for
>>> clocks. I think the first integer is the clock output to use but some
>>> of the versatile and highbank ones only have a phandle or is it more
>>> implementation defined? The clock-output-names description hints at
>>> recommended, so I find this a little confusing, but that could just be
>>> me!
>>
>> I'm following convention here that has been established with
>> interrupts, gpios, and others. Sometimes more information is needed
>> that just the clock number. Using #clock-cells gives a clock provider
>> the option of having additional fields for clock flags or other data.
>> This is very much implementation defined. Simple clock providers that
>> only have a single clock output can easily use #clock-cells = <0>.
>> Providers with multiple outputs will need to use 1 or more cells.
>>
>
> Aren't you off by 1 here? The minimum cells is 1 to hold the phandle of
> the source/parent. Multiple outputs will need a cell size of 2 (typically).
No. The #clock-cells does not include the phandle. Never did.
g.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 4/9] of: add clock providers
2011-12-13 17:54 ` Grant Likely
2011-12-13 18:01 ` Rob Herring
@ 2011-12-15 13:51 ` Shawn Guo
[not found] ` <20111215135129.GA2831-+NayF8gZjK2ctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
1 sibling, 1 reply; 39+ messages in thread
From: Shawn Guo @ 2011-12-15 13:51 UTC (permalink / raw)
To: Grant Likely
Cc: Jamie Iles, Sascha Hauer, devicetree-discuss, linux-kernel,
Rob Herring, Mike Turquette
On Tue, Dec 13, 2011 at 10:54:58AM -0700, Grant Likely wrote:
> On Mon, Dec 12, 2011 at 4:29 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> > Hi Grant,
> >
> > I'm still going through these and trying to digest them but a couple of
> > quick questions/comments.
> >
> > Jamie
> >
> > On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
> >> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> new file mode 100644
> >> index 0000000..e40c436
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> @@ -0,0 +1,114 @@
> >> +This binding is a work-in-progress, and are based on some experimental
> >> +work by benh[1].
> >> +
> >> +Sources of clock signal can be represented by any node in the device
> >> +tree. Those nodes are designated as clock providers. Clock consumer
> >> +nodes use a phandle and clock specifier pair to connect clock provider
> >> +outputs to clock inputs. Similar to the gpio specifiers, a clock
> >> +specifier is an array of one more more cells identifying the clock
> >> +output on a device. The length of a clock specifier is defined by the
> >> +value of a #clock-cells property in the clock provider node.
> >> +
> >> +[1] http://patchwork.ozlabs.org/patch/31551/
> >> +
> >> +==Clock providers==
> >> +
> >> +Required properties:
> >> +#clock-cells: Number of cells in a clock specifier; typically will be
> >> + set to 1
> >
> > I'm not sure I fully understand what the extra cells actually mean for
> > clocks. I think the first integer is the clock output to use but some
> > of the versatile and highbank ones only have a phandle or is it more
> > implementation defined? The clock-output-names description hints at
> > recommended, so I find this a little confusing, but that could just be
> > me!
>
> I'm following convention here that has been established with
> interrupts, gpios, and others. Sometimes more information is needed
> that just the clock number. Using #clock-cells gives a clock provider
> the option of having additional fields for clock flags or other data.
> This is very much implementation defined. Simple clock providers that
> only have a single clock output can easily use #clock-cells = <0>.
> Providers with multiple outputs will need to use 1 or more cells.
>
It totally destroys my understanding on #clock-cells :)
I thought it's introduced to reduce the clock nodes in dts. That said,
#clock-cells stands for the number of clks we describe in the node.
When #clock-cells > 1 for a node, the node becomes a clk blob which
actually contains multiple clks. I migrated the imx6 clock to your
first post with this approach [1], using 70 nodes to describe 110
clocks (~35% nodes reduced).
Now, you are saying it's not designed for this purpose. I'm pretty
confused, because the only reasonable use of #clock-cells to me is
just that way, and I fail to see why we need #clock-cells if we do
not use it that way.
http://comments.gmane.org/gmane.linux.drivers.devicetree/9631
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 4/9] of: add clock providers
[not found] ` <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-12-12 23:29 ` Jamie Iles
@ 2012-01-10 21:33 ` Jamie Iles
2012-01-12 4:46 ` Grant Likely
2012-01-13 13:50 ` Shawn Guo
2 siblings, 1 reply; 39+ messages in thread
From: Jamie Iles @ 2012-01-10 21:33 UTC (permalink / raw)
To: Grant Likely
Cc: Sascha Hauer, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mike Turquette
Hi Grant,
On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
> Based on work by Ben Herrenschmidt and Jeremy Kerr, this patch adds an
> of_clk_get function to allow platforms to retrieve clock data from the
> device tree.
>
> Platform register a provider through of_clk_add_provider, which will be
> called when a device references the provider's OF node for a clock
> reference.
[...]
> diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt b/Documentation/devicetree/bindings/clock/fixed-clock.txt
> new file mode 100644
> index 0000000..9a75342
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/fixed-clock.txt
> @@ -0,0 +1,21 @@
> +Binding for simple fixed-rate clock sources.
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/fixed-clock.txt
This appears to reference itself?
> +
> +Required properties:
> +- compatible : shall be "fixed-clock".
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- clock-frequency : frequency of clock in Hz. May be multiple cells.
> +
> +Optional properties:
> +- gpios : From common gpio binding; gpio connection to clock enable pin.
This seems a little odd to me to have in the common binding, but I'm not
that familiar with too many platforms.
> +- clock-output-names : From common clock binding
> +
> +Example:
> + clock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <1000000000>;
> + };
I wonder if this should have an optional clock consumer with a standard
name for parenting? For example, on picoxcell there is a fixed 200MHz
APB clock that is really a PLL from a 20MHz reference input and I'd like
to represent that in the clock tree.
I'm about to start trying to get this and Mike's common struct clk
patches working on picoxcell, and the one thing I'm not understanding at
the moment is how to handle the tree itself. Currently I was planning
on iterating over all clocks and finding a named input clock "ref" or
"input" perhaps. This would be fine for picoxcell, but I guess more
complicated chips may need something else.
Jamie
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-10 21:33 ` Jamie Iles
@ 2012-01-12 4:46 ` Grant Likely
2012-01-12 10:07 ` Jamie Iles
[not found] ` <CACxGe6vqb8AZcZb5WMvfmLsUZH7=SEtBrJCSfsFmDYpKX42MzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 2 replies; 39+ messages in thread
From: Grant Likely @ 2012-01-12 4:46 UTC (permalink / raw)
To: Jamie Iles
Cc: Sascha Hauer, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mike Turquette
On Tue, Jan 10, 2012 at 2:33 PM, Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.org> wrote:
> Hi Grant,
>
> On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>> Based on work by Ben Herrenschmidt and Jeremy Kerr, this patch adds an
>> of_clk_get function to allow platforms to retrieve clock data from the
>> device tree.
>>
>> Platform register a provider through of_clk_add_provider, which will be
>> called when a device references the provider's OF node for a clock
>> reference.
> [...]
>> diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt b/Documentation/devicetree/bindings/clock/fixed-clock.txt
>> new file mode 100644
>> index 0000000..9a75342
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/fixed-clock.txt
>> @@ -0,0 +1,21 @@
>> +Binding for simple fixed-rate clock sources.
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +[1] Documentation/devicetree/bindings/clock/fixed-clock.txt
>
> This appears to reference itself?
Heh, oops.
>
>> +
>> +Required properties:
>> +- compatible : shall be "fixed-clock".
>> +- #clock-cells : from common clock binding; shall be set to 0.
>> +- clock-frequency : frequency of clock in Hz. May be multiple cells.
>> +
>> +Optional properties:
>> +- gpios : From common gpio binding; gpio connection to clock enable pin.
>
> This seems a little odd to me to have in the common binding, but I'm not
> that familiar with too many platforms.
>
>> +- clock-output-names : From common clock binding
>> +
>> +Example:
>> + clock {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <1000000000>;
>> + };
>
> I wonder if this should have an optional clock consumer with a standard
> name for parenting? For example, on picoxcell there is a fixed 200MHz
> APB clock that is really a PLL from a 20MHz reference input and I'd like
> to represent that in the clock tree.
If it depends on a parent clock, then it really isn't a fixed clock,
is it (from the perspective of the OS). The point of this binding is
a trivial way to describe clocks that don't change. If that
assumption doesn't hold true, then this binding isn't suitable for
that clock. As you point out, even the gpio enable feature is pushing
things a bit.
That said, I'm open to extending this binding if something can be
defined that will have a lot of use cases.
> I'm about to start trying to get this and Mike's common struct clk
> patches working on picoxcell, and the one thing I'm not understanding at
> the moment is how to handle the tree itself. Currently I was planning
> on iterating over all clocks and finding a named input clock "ref" or
> "input" perhaps. This would be fine for picoxcell, but I guess more
> complicated chips may need something else.
It might be useful to have something like of_irq_init() for setting up
initial clocks, but the solution feels inelegant to me. I suspect
that there will be end up being intertwined init order dependencies
between clocks and irqs and other early setup stuff that could be
handled better. Again, I need to think about this some more. There
might need to be something like an of_early_probe() call that accepts
a match table of compatible values and setup functions with some logic
or data to resolve dependencies. The trick will be to not end up with
something complex. I'll need to think about this more...
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-12 4:46 ` Grant Likely
@ 2012-01-12 10:07 ` Jamie Iles
2012-01-12 18:44 ` Turquette, Mike
[not found] ` <CACxGe6vqb8AZcZb5WMvfmLsUZH7=SEtBrJCSfsFmDYpKX42MzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 1 reply; 39+ messages in thread
From: Jamie Iles @ 2012-01-12 10:07 UTC (permalink / raw)
To: Grant Likely
Cc: Jamie Iles, linux-kernel, devicetree-discuss, Rob Herring,
Sascha Hauer, Mike Turquette
On Wed, Jan 11, 2012 at 09:46:58PM -0700, Grant Likely wrote:
> On Tue, Jan 10, 2012 at 2:33 PM, Jamie Iles <jamie@jamieiles.com> wrote:
> > On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
> >> +- clock-output-names : From common clock binding
> >> +
> >> +Example:
> >> + clock {
> >> + compatible = "fixed-clock";
> >> + #clock-cells = <0>;
> >> + clock-frequency = <1000000000>;
> >> + };
> >
> > I wonder if this should have an optional clock consumer with a standard
> > name for parenting? For example, on picoxcell there is a fixed 200MHz
> > APB clock that is really a PLL from a 20MHz reference input and I'd like
> > to represent that in the clock tree.
>
> If it depends on a parent clock, then it really isn't a fixed clock,
> is it (from the perspective of the OS). The point of this binding is
> a trivial way to describe clocks that don't change. If that
> assumption doesn't hold true, then this binding isn't suitable for
> that clock. As you point out, even the gpio enable feature is pushing
> things a bit.
>
> That said, I'm open to extending this binding if something can be
> defined that will have a lot of use cases.
Well for picoxcell where the 200MHz APB clock is just a PLL generated
from a 20MHz clock then it kind of is a fixed clock as far as Linux is
concerned isn't it? The rate can't be changed and and the clock can't
be disabled so it's just a dumb clock with a parent.
I guess I could just omit the parent as far as Linux is concerned, but
it would be nice to represent the real clock tree if possible.
> > I'm about to start trying to get this and Mike's common struct clk
> > patches working on picoxcell, and the one thing I'm not understanding at
> > the moment is how to handle the tree itself. Currently I was planning
> > on iterating over all clocks and finding a named input clock "ref" or
> > "input" perhaps. This would be fine for picoxcell, but I guess more
> > complicated chips may need something else.
>
> It might be useful to have something like of_irq_init() for setting up
> initial clocks, but the solution feels inelegant to me. I suspect
> that there will be end up being intertwined init order dependencies
> between clocks and irqs and other early setup stuff that could be
> handled better. Again, I need to think about this some more. There
> might need to be something like an of_early_probe() call that accepts
> a match table of compatible values and setup functions with some logic
> or data to resolve dependencies. The trick will be to not end up with
> something complex. I'll need to think about this more...
Yes, probably not an easy problem to solve, especially for the platforms
where the parent can change at runtime.
I wonder if an of_clk_init() could take a platform callback, so that
of_clk_init() goes of and creates a struct clk for each clk in the DT,
then for each registered clock calls a platform specific callback which
returns the parent (if any). It feels like a fairly platform specific
problem to me.
Jamie
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-12 10:07 ` Jamie Iles
@ 2012-01-12 18:44 ` Turquette, Mike
2012-01-12 19:16 ` Grant Likely
0 siblings, 1 reply; 39+ messages in thread
From: Turquette, Mike @ 2012-01-12 18:44 UTC (permalink / raw)
To: Jamie Iles
Cc: Grant Likely, linux-kernel, devicetree-discuss, Rob Herring,
Sascha Hauer
On Thu, Jan 12, 2012 at 2:07 AM, Jamie Iles <jamie@jamieiles.com> wrote:
> On Wed, Jan 11, 2012 at 09:46:58PM -0700, Grant Likely wrote:
>> On Tue, Jan 10, 2012 at 2:33 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>> > On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>> > I'm about to start trying to get this and Mike's common struct clk
>> > patches working on picoxcell, and the one thing I'm not understanding at
>> > the moment is how to handle the tree itself. Currently I was planning
>> > on iterating over all clocks and finding a named input clock "ref" or
>> > "input" perhaps. This would be fine for picoxcell, but I guess more
>> > complicated chips may need something else.
>>
>> It might be useful to have something like of_irq_init() for setting up
>> initial clocks, but the solution feels inelegant to me. I suspect
>> that there will be end up being intertwined init order dependencies
>> between clocks and irqs and other early setup stuff that could be
>> handled better. Again, I need to think about this some more. There
>> might need to be something like an of_early_probe() call that accepts
>> a match table of compatible values and setup functions with some logic
>> or data to resolve dependencies. The trick will be to not end up with
>> something complex. I'll need to think about this more...
>
> Yes, probably not an easy problem to solve, especially for the platforms
> where the parent can change at runtime.
>
> I wonder if an of_clk_init() could take a platform callback, so that
> of_clk_init() goes of and creates a struct clk for each clk in the DT,
> then for each registered clock calls a platform specific callback which
> returns the parent (if any). It feels like a fairly platform specific
> problem to me.
Based on Thomas' feedback I'm removing the requirement for clocks to
be registered in-order with clk_init(). Any clock that cannot resolve
it's parent within clk_init() (via the .get_parent callback, or
otherwise having .parent statically initialized) will be put into an
orphaned clocks list, which will be walked every time a new clock is
registered. Hurray for n^2 solutions.
Does the above help with the of_clk_init problems?
One final data point: I certainly plan on allowing for statically
allocated clocks to live alongside DT clocks. In fact the clock trees
on OMAP are so large that there is some discussion about having some
of the clocks statically allocated and some in DT, but I don't know
what that split looks like right now. I don't enjoy the idea of
packing 200+ of any entity into a .dts blob.
Regards,
Mike
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 4/9] of: add clock providers
2012-01-12 18:44 ` Turquette, Mike
@ 2012-01-12 19:16 ` Grant Likely
0 siblings, 0 replies; 39+ messages in thread
From: Grant Likely @ 2012-01-12 19:16 UTC (permalink / raw)
To: Turquette, Mike
Cc: Jamie Iles, linux-kernel, devicetree-discuss, Rob Herring,
Sascha Hauer
On Thu, Jan 12, 2012 at 11:44 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Thu, Jan 12, 2012 at 2:07 AM, Jamie Iles <jamie@jamieiles.com> wrote:
>> On Wed, Jan 11, 2012 at 09:46:58PM -0700, Grant Likely wrote:
>>> On Tue, Jan 10, 2012 at 2:33 PM, Jamie Iles <jamie@jamieiles.com> wrote:
>>> > On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
>>> > I'm about to start trying to get this and Mike's common struct clk
>>> > patches working on picoxcell, and the one thing I'm not understanding at
>>> > the moment is how to handle the tree itself. Currently I was planning
>>> > on iterating over all clocks and finding a named input clock "ref" or
>>> > "input" perhaps. This would be fine for picoxcell, but I guess more
>>> > complicated chips may need something else.
>>>
>>> It might be useful to have something like of_irq_init() for setting up
>>> initial clocks, but the solution feels inelegant to me. I suspect
>>> that there will be end up being intertwined init order dependencies
>>> between clocks and irqs and other early setup stuff that could be
>>> handled better. Again, I need to think about this some more. There
>>> might need to be something like an of_early_probe() call that accepts
>>> a match table of compatible values and setup functions with some logic
>>> or data to resolve dependencies. The trick will be to not end up with
>>> something complex. I'll need to think about this more...
>>
>> Yes, probably not an easy problem to solve, especially for the platforms
>> where the parent can change at runtime.
>>
>> I wonder if an of_clk_init() could take a platform callback, so that
>> of_clk_init() goes of and creates a struct clk for each clk in the DT,
>> then for each registered clock calls a platform specific callback which
>> returns the parent (if any). It feels like a fairly platform specific
>> problem to me.
>
> Based on Thomas' feedback I'm removing the requirement for clocks to
> be registered in-order with clk_init(). Any clock that cannot resolve
> it's parent within clk_init() (via the .get_parent callback, or
> otherwise having .parent statically initialized) will be put into an
> orphaned clocks list, which will be walked every time a new clock is
> registered. Hurray for n^2 solutions.
>
> Does the above help with the of_clk_init problems?
It does.
> One final data point: I certainly plan on allowing for statically
> allocated clocks to live alongside DT clocks. In fact the clock trees
> on OMAP are so large that there is some discussion about having some
> of the clocks statically allocated and some in DT, but I don't know
> what that split looks like right now. I don't enjoy the idea of
> packing 200+ of any entity into a .dts blob.
I'm okay with that.
^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <CACxGe6vqb8AZcZb5WMvfmLsUZH7=SEtBrJCSfsFmDYpKX42MzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [RFC v2 4/9] of: add clock providers
[not found] ` <CACxGe6vqb8AZcZb5WMvfmLsUZH7=SEtBrJCSfsFmDYpKX42MzA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2012-01-13 12:47 ` Shawn Guo
2012-01-14 4:30 ` Turquette, Mike
0 siblings, 1 reply; 39+ messages in thread
From: Shawn Guo @ 2012-01-13 12:47 UTC (permalink / raw)
To: Grant Likely
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Sascha Hauer,
Mike Turquette
On Wed, Jan 11, 2012 at 09:46:58PM -0700, Grant Likely wrote:
...
> >> +Required properties:
> >> +- compatible : shall be "fixed-clock".
> >> +- #clock-cells : from common clock binding; shall be set to 0.
> >> +- clock-frequency : frequency of clock in Hz. May be multiple cells.
> >> +
> >> +Optional properties:
> >> +- gpios : From common gpio binding; gpio connection to clock enable pin.
> >
> > This seems a little odd to me to have in the common binding, but I'm not
> > that familiar with too many platforms.
> >
> >> +- clock-output-names : From common clock binding
> >> +
> >> +Example:
> >> + clock {
> >> + compatible = "fixed-clock";
> >> + #clock-cells = <0>;
> >> + clock-frequency = <1000000000>;
> >> + };
> >
> > I wonder if this should have an optional clock consumer with a standard
> > name for parenting? For example, on picoxcell there is a fixed 200MHz
> > APB clock that is really a PLL from a 20MHz reference input and I'd like
> > to represent that in the clock tree.
>
> If it depends on a parent clock, then it really isn't a fixed clock,
> is it (from the perspective of the OS). The point of this binding is
> a trivial way to describe clocks that don't change. If that
> assumption doesn't hold true, then this binding isn't suitable for
> that clock. As you point out, even the gpio enable feature is pushing
> things a bit.
>
I recently ran into a use case perfectly fitting into this discussion.
We have audio codec sgtl5000 on imx51-babbage board requiring a 26 MHz
clock input to its SYS_MCLK pin. The board has a 26 MHz oscillator with
a gpio control that outputs 26M_OSC_CLK. And this clock goes through a
3-state buffer component with another gpio control, and then goes to
sgtl5000 SYS_MCLK pin. The following is what I have in my mind to
describe them in device tree.
soc_26m_clk: soc-26m-clk {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <26000000>;
clock-output-names = "soc-26m-clk";
gpios = <&gpio3 1 0>;
};
sgtl5000_clk: sgtl5000-sys-mclk {
#clock-cells = <0>;
gpios = <&gpio4 26 0>;
clocks = <&soc_26m_clk>;
clock-names = "soc-26m-clk";
clock-output-names = "sgtl5000-sys-mclk";
};
codec: sgtl5000@0a {
compatible = "fsl,sgtl5000";
reg = <0x0a>;
clocks = <&sgtl5000_clk>;
clock-names = "sgtl5000-sys-mclk";
};
The sgtl5000-sys-mclk is a clock with fixed rate, but the rate is really
defined by its parent soc-26m-clk, which is anothe fixed-rate clock. We
have no doubt that soc-26m-clk is a "fixed-clock". Is sgtl5000-sys-mclk
a "fixed-clock"? The following is the "fixed-clock" defined by Mike's
implementation.
/*
* DOC: basic fixed-rate clock that cannot gate
*
* Traits of this clock:
* prepare - clock never gates. clk_prepare & clk_unprepare do nothing
* enable - clock never gates. clk_enable & clk_disable do nothing
* rate - rate is always a fixed value. No clk_set_rate support
* parent - fixed parent. No clk_set_parent support
*
* note: parent can be NULL, which implies that this clock is a root clock.
*/
struct clk_hw_ops clk_hw_fixed_ops = {
.recalc_rate = clk_hw_fixed_recalc_rate,
.get_parent = clk_hw_fixed_get_parent,
};
It says that the "fixed-clock" can have a fixed parent. So I should
probably add compatible = "fixed-clock" for node sgtl5000-sys-mclk too.
Then should I add clock-frequency property for it too? I'm not sure
about that, since the frequency is really defined by its parent. IOW,
should the clock-frequency property for "fixed-clock" be optional? On
the other hand, let clock-frequency property be optional seems a little
illogical to the concept of "fixed-clock". So I'm really confused here.
With the proper clock support, I would expect that sgtl5000 driver only
needs to make the following two calls to have its clock enabled.
mclk = clk_get(&dev, "sgtl5000-sys-mclk");
clk_prepare_enable(mclk);
But looking at the current fixed-clock implementation, there is even
no clk_enable operation for fixed-clock. How can we control the clock
with gpio?
All I'm saying is that we need some level of alignment between clock
binding and common-clk implementation.
Regards,
Shawn
> That said, I'm open to extending this binding if something can be
> defined that will have a lot of use cases.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-13 12:47 ` Shawn Guo
@ 2012-01-14 4:30 ` Turquette, Mike
2012-01-14 5:40 ` Shawn Guo
0 siblings, 1 reply; 39+ messages in thread
From: Turquette, Mike @ 2012-01-14 4:30 UTC (permalink / raw)
To: Shawn Guo
Cc: Grant Likely, Jamie Iles, Sascha Hauer, devicetree-discuss,
linux-kernel, Rob Herring
On Fri, Jan 13, 2012 at 4:47 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Wed, Jan 11, 2012 at 09:46:58PM -0700, Grant Likely wrote:
> ...
>> >> +Required properties:
>> >> +- compatible : shall be "fixed-clock".
>> >> +- #clock-cells : from common clock binding; shall be set to 0.
>> >> +- clock-frequency : frequency of clock in Hz. May be multiple cells.
>> >> +
>> >> +Optional properties:
>> >> +- gpios : From common gpio binding; gpio connection to clock enable pin.
>> >
>> > This seems a little odd to me to have in the common binding, but I'm not
>> > that familiar with too many platforms.
>> >
>> >> +- clock-output-names : From common clock binding
>> >> +
>> >> +Example:
>> >> + clock {
>> >> + compatible = "fixed-clock";
>> >> + #clock-cells = <0>;
>> >> + clock-frequency = <1000000000>;
>> >> + };
>> >
>> > I wonder if this should have an optional clock consumer with a standard
>> > name for parenting? For example, on picoxcell there is a fixed 200MHz
>> > APB clock that is really a PLL from a 20MHz reference input and I'd like
>> > to represent that in the clock tree.
>>
>> If it depends on a parent clock, then it really isn't a fixed clock,
>> is it (from the perspective of the OS). The point of this binding is
>> a trivial way to describe clocks that don't change. If that
>> assumption doesn't hold true, then this binding isn't suitable for
>> that clock. As you point out, even the gpio enable feature is pushing
>> things a bit.
>>
> I recently ran into a use case perfectly fitting into this discussion.
>
> We have audio codec sgtl5000 on imx51-babbage board requiring a 26 MHz
> clock input to its SYS_MCLK pin. The board has a 26 MHz oscillator with
> a gpio control that outputs 26M_OSC_CLK. And this clock goes through a
> 3-state buffer component with another gpio control, and then goes to
> sgtl5000 SYS_MCLK pin. The following is what I have in my mind to
> describe them in device tree.
>
> soc_26m_clk: soc-26m-clk {
> compatible = "fixed-clock";
> #clock-cells = <0>;
> clock-frequency = <26000000>;
> clock-output-names = "soc-26m-clk";
> gpios = <&gpio3 1 0>;
> };
>
> sgtl5000_clk: sgtl5000-sys-mclk {
> #clock-cells = <0>;
> gpios = <&gpio4 26 0>;
> clocks = <&soc_26m_clk>;
> clock-names = "soc-26m-clk";
> clock-output-names = "sgtl5000-sys-mclk";
> };
>
> codec: sgtl5000@0a {
> compatible = "fsl,sgtl5000";
> reg = <0x0a>;
> clocks = <&sgtl5000_clk>;
> clock-names = "sgtl5000-sys-mclk";
> };
>
> The sgtl5000-sys-mclk is a clock with fixed rate, but the rate is really
> defined by its parent soc-26m-clk, which is anothe fixed-rate clock. We
> have no doubt that soc-26m-clk is a "fixed-clock". Is sgtl5000-sys-mclk
> a "fixed-clock"? The following is the "fixed-clock" defined by Mike's
> implementation.
>
> /*
> * DOC: basic fixed-rate clock that cannot gate
> *
> * Traits of this clock:
> * prepare - clock never gates. clk_prepare & clk_unprepare do nothing
> * enable - clock never gates. clk_enable & clk_disable do nothing
> * rate - rate is always a fixed value. No clk_set_rate support
> * parent - fixed parent. No clk_set_parent support
> *
> * note: parent can be NULL, which implies that this clock is a root clock.
> */
> struct clk_hw_ops clk_hw_fixed_ops = {
> .recalc_rate = clk_hw_fixed_recalc_rate,
> .get_parent = clk_hw_fixed_get_parent,
> };
>
> It says that the "fixed-clock" can have a fixed parent. So I should
> probably add compatible = "fixed-clock" for node sgtl5000-sys-mclk too.
> Then should I add clock-frequency property for it too? I'm not sure
> about that, since the frequency is really defined by its parent. IOW,
> should the clock-frequency property for "fixed-clock" be optional? On
> the other hand, let clock-frequency property be optional seems a little
> illogical to the concept of "fixed-clock". So I'm really confused here.
I had envisioned fixed clocks as being clocks whose rates could never
change; obviously this is mostly useful for root clocks like
oscillators and whatnot.
There is nothing wrong with using fixed clock for sgtl5000-sys-mclk,
but it's rate *can* change if it's parent rate ever changes. This may
be very unlikely on your platform, in which case again it is OK to use
fixed clock here if you want to.
However... I'm inclined to say that sgtl5000-sys-mclk is good
candidate for a dummy clock: it follows it's parent rate, doesn't
gate, doesn't divide it's parent rate, only has one input. There
isn't a common dummy clock in the v4 patches, but there is in v5. The
key difference between the fixed rate clock and the dummy clock is
that the dummy clock looks at clk->parent->rate in it's .get_rate,
whereas a fixed rate clock will have it's rate cached in struct
clk_fixed.
Thoughts?
Mike
> With the proper clock support, I would expect that sgtl5000 driver only
> needs to make the following two calls to have its clock enabled.
>
> mclk = clk_get(&dev, "sgtl5000-sys-mclk");
> clk_prepare_enable(mclk);
>
> But looking at the current fixed-clock implementation, there is even
> no clk_enable operation for fixed-clock. How can we control the clock
> with gpio?
>
> All I'm saying is that we need some level of alignment between clock
> binding and common-clk implementation.
>
> Regards,
> Shawn
>
>> That said, I'm open to extending this binding if something can be
>> defined that will have a lot of use cases.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-14 4:30 ` Turquette, Mike
@ 2012-01-14 5:40 ` Shawn Guo
0 siblings, 0 replies; 39+ messages in thread
From: Shawn Guo @ 2012-01-14 5:40 UTC (permalink / raw)
To: Turquette, Mike
Cc: Grant Likely, Jamie Iles, Sascha Hauer, devicetree-discuss,
linux-kernel, Rob Herring
On Fri, Jan 13, 2012 at 08:30:30PM -0800, Turquette, Mike wrote:
...
> I had envisioned fixed clocks as being clocks whose rates could never
> change; obviously this is mostly useful for root clocks like
> oscillators and whatnot.
>
> There is nothing wrong with using fixed clock for sgtl5000-sys-mclk,
> but it's rate *can* change if it's parent rate ever changes. This may
> be very unlikely on your platform, in which case again it is OK to use
> fixed clock here if you want to.
>
> However... I'm inclined to say that sgtl5000-sys-mclk is good
> candidate for a dummy clock: it follows it's parent rate, doesn't
> gate, doesn't divide it's parent rate, only has one input. There
> isn't a common dummy clock in the v4 patches, but there is in v5. The
> key difference between the fixed rate clock and the dummy clock is
> that the dummy clock looks at clk->parent->rate in it's .get_rate,
> whereas a fixed rate clock will have it's rate cached in struct
> clk_fixed.
>
> Thoughts?
>
I would say this modeling looks all sane to me, except both the
fixed-clock soc-26m-clk and dummy-clock sgtl5000-sys-mclk in this
case have a gate which is controlled by gpio.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RFC v2 4/9] of: add clock providers
[not found] ` <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
2011-12-12 23:29 ` Jamie Iles
2012-01-10 21:33 ` Jamie Iles
@ 2012-01-13 13:50 ` Shawn Guo
[not found] ` <20120113135036.GC17029-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>
2 siblings, 1 reply; 39+ messages in thread
From: Shawn Guo @ 2012-01-13 13:50 UTC (permalink / raw)
To: Grant Likely
Cc: Sascha Hauer, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mike Turquette
On Mon, Dec 12, 2011 at 03:02:04PM -0700, Grant Likely wrote:
> +==Clock providers==
> +
> +Required properties:
> +#clock-cells: Number of cells in a clock specifier; typically will be
> + set to 1
Shouldn't it be 0 typically, which means it represents only one clock
in the node?
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 39+ messages in thread
* RE: [RFC v2 4/9] of: add clock providers
2011-12-12 22:02 ` [RFC v2 4/9] of: add clock providers Grant Likely
[not found] ` <1323727329-4989-4-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
@ 2012-01-17 20:44 ` Stephen Warren
2012-01-17 22:47 ` Grant Likely
1 sibling, 1 reply; 39+ messages in thread
From: Stephen Warren @ 2012-01-17 20:44 UTC (permalink / raw)
To: Grant Likely, linux-kernel@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org
Cc: Rob Herring, Sascha Hauer, Mike Turquette
Grant Likely wrote at Monday, December 12, 2011 3:02 PM:
> Based on work by Ben Herrenschmidt and Jeremy Kerr, this patch adds an
> of_clk_get function to allow platforms to retrieve clock data from the
> device tree.
>
> Platform register a provider through of_clk_add_provider, which will be
> called when a device references the provider's OF node for a clock
> reference.
...
> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt
...
> +==Example==
> +
> + /* external oscillator */
> + osc: oscillator {
> + compatible = "fixed-clock";
> + #clock-cells = <1>;
> + clock-frequency = <32678>;
> + clock-output-names = "osc";
> + };
> +
> + /* phase-locked-loop device, generates a higher frequency clock
> + * from the external oscillator reference */
> + pll: pll@4c000 {
> + compatible = "vendor,some-pll-interface"
> + #clock-cells = <1>;
> + clocks = <&osc 0>;
> + clock-names = "ref";
> + reg = <0x4c000 0x1000>;
> + clock-output-names = "pll", "pll-switched";
> + };
> +
> + /* UART, using the low frequency oscillator for the baud clock,
> + * and the high frequency switched PLL output for register
> + * clocking */
> + uart@a000 {
> + compatible = "fsl,imx-uart";
> + reg = <0xa000 0x1000>;
> + interrupts = <33>;
> + clocks = <&osc 0>, <&pll 1>;
> + clock-names = "baud", "register";
> + };
> +
> +This DT fragment defines three devices: an external oscillator to provide a
> +low-frequency reference clock, a PLL device to generate a higher frequency
> +clock signal, and a UART.
> +
> +* The oscillator is fixed-frequency, and provides one clock output, named "osc".
> +* The PLL is both a clock provider and a clock consumer. It uses the clock
> + signal generated by the external oscillator, and provides two output signals
> + ("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)
In the example above, the UART's register clock's parent is the PLL, and
the PLL's parent is the OSC. Is the intention to have this parenting set
up automatically by the core OF clock code, or is this something that each
individual driver needs to set up itself.
In other words, does the UART driver need to do something like:
clk_reg = clk_get(dev, "register");
clk_parent = of_clk_get_by_name(np, "register);
clk_set_parent(clk_reg, clk_parent);
Or will that all happen transparently within just the of_clk_get_by_name
call?
(I suppose this question makes slightly more sense for the PLL itself,
since both the upstream and downstream clocks are represented in the PLL
node, whereas the UART's node only represents the clock consumer side,
so the above code isn't really possible automatically).
Somewhat related to this: How does dynamic reparenting interact with
the DT clock binding; is the DT just the default/initial clock setup,
and anything beyond that needs a custom binding and code in the consumer?
I'm thinking of say a system with 1 I2S controller, and both an internal
and external I2S clock source, where perhaps the internal source needs
to be used to capture from an I2S interface on one set of pins (e.g.
analog mic) but the other clock source needs to be used to capture from
I2S on another set of pins (e.g. digital baseband unit connection).
(This example is theoretical, but I'm sure there are other dynamic clock
cases in practice).
--
nvpublic
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-17 20:44 ` Stephen Warren
@ 2012-01-17 22:47 ` Grant Likely
2012-01-17 23:37 ` Turquette, Mike
0 siblings, 1 reply; 39+ messages in thread
From: Grant Likely @ 2012-01-17 22:47 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-kernel@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Rob Herring, Sascha Hauer, Mike Turquette
On Tue, Jan 17, 2012 at 1:44 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Grant Likely wrote at Monday, December 12, 2011 3:02 PM:
>> +This DT fragment defines three devices: an external oscillator to provide a
>> +low-frequency reference clock, a PLL device to generate a higher frequency
>> +clock signal, and a UART.
>> +
>> +* The oscillator is fixed-frequency, and provides one clock output, named "osc".
>> +* The PLL is both a clock provider and a clock consumer. It uses the clock
>> + signal generated by the external oscillator, and provides two output signals
>> + ("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)
>
> In the example above, the UART's register clock's parent is the PLL, and
> the PLL's parent is the OSC. Is the intention to have this parenting set
> up automatically by the core OF clock code, or is this something that each
> individual driver needs to set up itself.
>
> In other words, does the UART driver need to do something like:
>
> clk_reg = clk_get(dev, "register");
> clk_parent = of_clk_get_by_name(np, "register);
> clk_set_parent(clk_reg, clk_parent);
>
> Or will that all happen transparently within just the of_clk_get_by_name
> call?
>
> (I suppose this question makes slightly more sense for the PLL itself,
> since both the upstream and downstream clocks are represented in the PLL
> node, whereas the UART's node only represents the clock consumer side,
> so the above code isn't really possible automatically).
The intent is that device only interacts with the leaf device. If the
clocks are arranged into a hierarchy, then the clock driver is
responsible for any interactions with the parent clock. Requiring the
driver to manipulate parent clocks directly defeats the purpose of
having a clock abstraction.
> Somewhat related to this: How does dynamic reparenting interact with
> the DT clock binding; is the DT just the default/initial clock setup,
> and anything beyond that needs a custom binding and code in the consumer?
As far as the clock binding goes, it only describes provider/consumer
relationships. The fact that such relationships may resolve to a
hierarchy is beyond what the binding describes. If a clock has
multiple possible parents, then that specific clock binding should
document how the multiple parent clocks are described and the clock
driver is responsible for implementing the correct behaviour.
Similarly, the DT clock binding provides no generic mechanism for
walking up the clock tree. That behaviour must also be implemented by
each specific clock driver.
> I'm thinking of say a system with 1 I2S controller, and both an internal
> and external I2S clock source, where perhaps the internal source needs
> to be used to capture from an I2S interface on one set of pins (e.g.
> analog mic) but the other clock source needs to be used to capture from
> I2S on another set of pins (e.g. digital baseband unit connection).
> (This example is theoretical, but I'm sure there are other dynamic clock
> cases in practice).
That is a reasonable example. In this case, the i2c controller would
include both in its clocks property, and the binding would document
when and why each clock source is used.
g.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [RFC v2 4/9] of: add clock providers
2012-01-17 22:47 ` Grant Likely
@ 2012-01-17 23:37 ` Turquette, Mike
[not found] ` <CAJOA=zMgGsZKGQ-EfSnXDY_WoFrzG6XPZtuxANFKqdL8CAaYrw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 39+ messages in thread
From: Turquette, Mike @ 2012-01-17 23:37 UTC (permalink / raw)
To: Grant Likely
Cc: Stephen Warren, linux-kernel@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, Rob Herring, Sascha Hauer
On Tue, Jan 17, 2012 at 2:47 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Tue, Jan 17, 2012 at 1:44 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> In other words, does the UART driver need to do something like:
>>
>> clk_reg = clk_get(dev, "register");
>> clk_parent = of_clk_get_by_name(np, "register);
>> clk_set_parent(clk_reg, clk_parent);
>>
>> Or will that all happen transparently within just the of_clk_get_by_name
>> call?
>>
>> (I suppose this question makes slightly more sense for the PLL itself,
>> since both the upstream and downstream clocks are represented in the PLL
>> node, whereas the UART's node only represents the clock consumer side,
>> so the above code isn't really possible automatically).
>
> The intent is that device only interacts with the leaf device. If the
> clocks are arranged into a hierarchy, then the clock driver is
> responsible for any interactions with the parent clock. Requiring the
> driver to manipulate parent clocks directly defeats the purpose of
> having a clock abstraction.
I don't think that we can get rid of all instances of drivers knowing
a bit about hierarchy. I think we can get rid of most, but there are
cases where it is valid for a driver to know some of the details.
More on that below.
>> Somewhat related to this: How does dynamic reparenting interact with
>> the DT clock binding; is the DT just the default/initial clock setup,
>> and anything beyond that needs a custom binding and code in the consumer?
>
> As far as the clock binding goes, it only describes provider/consumer
> relationships. The fact that such relationships may resolve to a
> hierarchy is beyond what the binding describes. If a clock has
> multiple possible parents, then that specific clock binding should
> document how the multiple parent clocks are described and the clock
> driver is responsible for implementing the correct behaviour.
It also deserves to be said that the DT data says nothing about which
of the possible parents _should_ be the input to a mux clock. It's
pretty common to want to make changes to hierarchy after taking a
device out of reset, since the reset values for a clock management IP
might be pretty conservative. So someone, somewhere must know some
details about hierarchy and set things up correctly. Maybe a "clock
driver" can do this, but for specific IPs such as the audio example
below it makes sense for that driver to have the knowledge.
> Similarly, the DT clock binding provides no generic mechanism for
> walking up the clock tree. That behaviour must also be implemented by
> each specific clock driver.
>
>> I'm thinking of say a system with 1 I2S controller, and both an internal
>> and external I2S clock source, where perhaps the internal source needs
>> to be used to capture from an I2S interface on one set of pins (e.g.
>> analog mic) but the other clock source needs to be used to capture from
>> I2S on another set of pins (e.g. digital baseband unit connection).
>> (This example is theoretical, but I'm sure there are other dynamic clock
>> cases in practice).
>
> That is a reasonable example. In this case, the i2c controller would
> include both in its clocks property, and the binding would document
> when and why each clock source is used.
I'm confused on this point. How does the binding "document when and
why each clock is used"? In the case where this I2S controller
expects to dynamically switch roles at run-time (analog mic versus
baseband) then clk_set_parent must still be invoked by the driver. To
be clear I'm imagining the above example like:
i_I2S e_I2S
\ /
I2S_mux
|
I2S controller IP
Regards,
Mike
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v2 6/9] arm/dt: add devicetree support to sp804 timer support
[not found] ` <1323727329-4989-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
` (2 preceding siblings ...)
2011-12-12 22:02 ` [RFC v2 4/9] of: add clock providers Grant Likely
@ 2011-12-12 22:02 ` Grant Likely
2011-12-12 23:54 ` Rob Herring
2011-12-12 22:02 ` [RFC v2 9/9] arm/highbank: Use clock binding common support code Grant Likely
4 siblings, 1 reply; 39+ messages in thread
From: Grant Likely @ 2011-12-12 22:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Russell King, Rob Herring, Sascha Hauer, Mike Turquette
This patch adds support to the sp804 code for retrieving timer
configuration from the device tree. sp804 channels can be used as
a clock event device or a clock source.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
arch/arm/common/timer-sp.c | 72 ++++++++++++++++++++++++++---
arch/arm/include/asm/hardware/timer-sp.h | 2 +
2 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/arch/arm/common/timer-sp.c b/arch/arm/common/timer-sp.c
index 2393b5b..93aed48 100644
--- a/arch/arm/common/timer-sp.c
+++ b/arch/arm/common/timer-sp.c
@@ -25,16 +25,18 @@
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_clk.h>
+#include <linux/of_irq.h>
#include <asm/hardware/arm_timer.h>
-static long __init sp804_get_clock_rate(const char *name)
+static long __init sp804_get_clock_rate(struct clk *clk, const char *name)
{
- struct clk *clk;
long rate;
int err;
- clk = clk_get_sys("sp804", name);
if (IS_ERR(clk)) {
pr_err("sp804: %s clock not found: %d\n", name,
(int)PTR_ERR(clk));
@@ -67,9 +69,10 @@ static long __init sp804_get_clock_rate(const char *name)
return rate;
}
-void __init sp804_clocksource_init(void __iomem *base, const char *name)
+static void __init __sp804_clocksource_init(void __iomem *base,
+ const char *name, struct clk *clk)
{
- long rate = sp804_get_clock_rate(name);
+ long rate = sp804_get_clock_rate(clk, name);
if (rate < 0)
return;
@@ -85,6 +88,10 @@ void __init sp804_clocksource_init(void __iomem *base, const char *name)
rate, 200, 32, clocksource_mmio_readl_down);
}
+void __init sp804_clocksource_init(void __iomem *base, const char *name)
+{
+ __sp804_clocksource_init(base, name, clk_get_sys("sp804", name));
+}
static void __iomem *clkevt_base;
static unsigned long clkevt_reload;
@@ -158,11 +165,11 @@ static struct irqaction sp804_timer_irq = {
.dev_id = &sp804_clockevent,
};
-void __init sp804_clockevents_init(void __iomem *base, unsigned int irq,
- const char *name)
+static void __init __sp804_clockevents_init(void __iomem *base,
+ unsigned int irq, const char *name, struct clk *clk)
{
struct clock_event_device *evt = &sp804_clockevent;
- long rate = sp804_get_clock_rate(name);
+ long rate = sp804_get_clock_rate(clk, name);
if (rate < 0)
return;
@@ -179,3 +186,52 @@ void __init sp804_clockevents_init(void __iomem *base, unsigned int irq,
setup_irq(irq, &sp804_timer_irq);
clockevents_register_device(evt);
}
+
+void __init sp804_clockevents_init(void __iomem *base, unsigned int irq,
+ const char *name)
+{
+ __sp804_clockevents_init(base, irq, name, clk_get_sys("sp804", name));
+}
+
+#ifdef CONFIG_OF
+void __init sp804_dt_setup(struct device_node *node)
+{
+ struct clk *clk;
+ unsigned int irq;
+ __iomem void * base;
+
+ pr_debug("%s(): Setting up sp804 clock %s\n", __func__, node->name);
+
+ /* Find the base address of the clock */
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("ERROR: Cannot get base address of sp804 %s\n",
+ node->full_name);
+ return;
+ }
+
+ writel(0, base + TIMER_CTRL); /* Disable the clock */
+
+ clk = of_clk_get(node, 0);
+ if (!clk) {
+ pr_err("ERROR: Cannot get clock for sp804 %s\n", node->name);
+ return;
+ }
+
+ /*
+ * Figure out how to use this clock
+ *
+ * Note: This is kind of ugly since it requires linux-specific
+ * properties in the device tree so that Linux knows which sp804
+ * channels can be used as the clock source and the clock events
+ * trigger. Something OS agnostic would be nicer, but it isn't
+ * obvious what that should look like.
+ */
+ if (of_get_property(node, "linux,clock-source", NULL)) {
+ __sp804_clocksource_init(base, node->full_name, clk);
+ } else if (of_get_property(node, "linux,clockevents-device", NULL)) {
+ irq = irq_of_parse_and_map(node, 0);
+ __sp804_clockevents_init(base, irq, node->full_name, clk);
+ };
+}
+#endif /* CONFIG_OF */
diff --git a/arch/arm/include/asm/hardware/timer-sp.h b/arch/arm/include/asm/hardware/timer-sp.h
index 4384d81..edd0ec3 100644
--- a/arch/arm/include/asm/hardware/timer-sp.h
+++ b/arch/arm/include/asm/hardware/timer-sp.h
@@ -1,2 +1,4 @@
+struct device_node;
void sp804_clocksource_init(void __iomem *, const char *);
void sp804_clockevents_init(void __iomem *, unsigned int, const char *);
+void sp804_dt_setup(struct device_node *node);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [RFC v2 6/9] arm/dt: add devicetree support to sp804 timer support
2011-12-12 22:02 ` [RFC v2 6/9] arm/dt: add devicetree support to sp804 timer support Grant Likely
@ 2011-12-12 23:54 ` Rob Herring
[not found] ` <4EE69446.2060009-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 39+ messages in thread
From: Rob Herring @ 2011-12-12 23:54 UTC (permalink / raw)
To: Grant Likely
Cc: linux-kernel, devicetree-discuss, Mike Turquette, Sascha Hauer,
Shawn Guo, Russell King
Grant,
On 12/12/2011 04:02 PM, Grant Likely wrote:
> This patch adds support to the sp804 code for retrieving timer
> configuration from the device tree. sp804 channels can be used as
> a clock event device or a clock source.
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Russell King <linux@arm.linux.org.uk>
> ---
[snip]
> +
> + /*
> + * Figure out how to use this clock
> + *
> + * Note: This is kind of ugly since it requires linux-specific
> + * properties in the device tree so that Linux knows which sp804
> + * channels can be used as the clock source and the clock events
> + * trigger. Something OS agnostic would be nicer, but it isn't
> + * obvious what that should look like.
> + */
> + if (of_get_property(node, "linux,clock-source", NULL)) {
> + __sp804_clocksource_init(base, node->full_name, clk);
> + } else if (of_get_property(node, "linux,clockevents-device", NULL)) {
> + irq = irq_of_parse_and_map(node, 0);
> + __sp804_clockevents_init(base, irq, node->full_name, clk);
At least in the case of highbank, there is no interrupt connected for
2nd timer in the h/w. So we could use that fact and presence of a clock
for each timer to determine which to use. Some of the ARM boards have 2
sp804's (4 timers) though and you could use any combination I think.
Does it really matter which one is selected as long as it meets the
needs of the OS? Yes, we could think of possible scenarios that don't
work, but it's not likely to see a slew of new platforms with sp804's as
new ARM core integrate the timers in. Although, bcmring is a bit strange
setting up 2 clksrc's, but that doesn't really present a problem.
The fact that you split the timer to 2 nodes is a bit of Linux's needs
defining the binding. The h/w block is a block with 2 timers. It's not
really split. The block does have a single set of primecell ID registers
at 0xfe0 for example.
Rob
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RFC v2 9/9] arm/highbank: Use clock binding common support code
[not found] ` <1323727329-4989-1-git-send-email-grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
` (3 preceding siblings ...)
2011-12-12 22:02 ` [RFC v2 6/9] arm/dt: add devicetree support to sp804 timer support Grant Likely
@ 2011-12-12 22:02 ` Grant Likely
4 siblings, 0 replies; 39+ messages in thread
From: Grant Likely @ 2011-12-12 22:02 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Rob Herring, Sascha Hauer, Mike Turquette
This patch makes the Calxeda Highbank platform use the common DT clock
binding support code for setting up timers and fixed clocks.
Signed-off-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/highbank.dts | 32 ++++++++++++++++++++++++-
arch/arm/mach-highbank/clock.c | 19 ---------------
arch/arm/mach-highbank/core.h | 1 -
arch/arm/mach-highbank/highbank.c | 13 +---------
arch/arm/mach-highbank/include/mach/clkdev.h | 11 +++++++++
6 files changed, 44 insertions(+), 33 deletions(-)
create mode 100644 arch/arm/mach-highbank/include/mach/clkdev.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e084b7e..33c2958 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -341,6 +341,7 @@ config ARCH_HIGHBANK
select ARM_GIC
select ARM_TIMER_SP804
select CLKDEV_LOOKUP
+ select HAVE_MACH_CLKDEV
select CPU_V7
select GENERIC_CLOCKEVENTS
select HAVE_ARM_SCU
diff --git a/arch/arm/boot/dts/highbank.dts b/arch/arm/boot/dts/highbank.dts
index aeb1a75..a60e41d 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -25,6 +25,10 @@
#address-cells = <1>;
#size-cells = <1>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
+ clock-ranges;
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
@@ -54,6 +58,20 @@
};
};
+ clocks {
+ eclk: eclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <200000000>;
+ };
+
+ pclk: pclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <150000000>;
+ };
+ };
+
memory {
name = "memory";
device_type = "memory";
@@ -70,6 +88,7 @@
compatible = "simple-bus";
interrupt-parent = <&intc>;
ranges;
+ clock-ranges;
timer@fff10600 {
compatible = "arm,smp-twd";
@@ -117,6 +136,7 @@
compatible = "calxeda,hb-sdhci";
reg = <0xffe0e000 0x1000>;
interrupts = <0 90 4>;
+ clocks = <&eclk>;
};
ipc@fff20000 {
@@ -157,10 +177,18 @@
interrupts = <0 17 4>;
};
- timer {
+ timer@fff34000 {
+ compatible = "arm,sp804", "arm,primecell";
+ reg = <0xfff34000 0x20>;
+ interrupts = <0 18 4>;
+ linux,clockevents-device;
+ };
+
+ timer@fff34020 {
compatible = "arm,sp804", "arm,primecell";
- reg = <0xfff34000 0x1000>;
+ reg = <0xfff34020 0x20>;
interrupts = <0 18 4>;
+ linux,clock-source;
};
rtc@fff35000 {
diff --git a/arch/arm/mach-highbank/clock.c b/arch/arm/mach-highbank/clock.c
index c25a2ae..96f4599 100644
--- a/arch/arm/mach-highbank/clock.c
+++ b/arch/arm/mach-highbank/clock.c
@@ -19,10 +19,6 @@
#include <linux/clk.h>
#include <linux/clkdev.h>
-struct clk {
- unsigned long rate;
-};
-
int clk_enable(struct clk *clk)
{
return 0;
@@ -45,18 +41,3 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
{
return 0;
}
-
-static struct clk eclk = { .rate = 200000000 };
-static struct clk pclk = { .rate = 150000000 };
-
-static struct clk_lookup lookups[] = {
- { .clk = &pclk, .con_id = "apb_pclk", },
- { .clk = &pclk, .dev_id = "sp804", },
- { .clk = &eclk, .dev_id = "ffe0e000.sdhci", },
- { .clk = &pclk, .dev_id = "fff36000.serial", },
-};
-
-void __init highbank_clocks_init(void)
-{
- clkdev_add_table(lookups, ARRAY_SIZE(lookups));
-}
diff --git a/arch/arm/mach-highbank/core.h b/arch/arm/mach-highbank/core.h
index 7e33fc9..490f430 100644
--- a/arch/arm/mach-highbank/core.h
+++ b/arch/arm/mach-highbank/core.h
@@ -1,5 +1,4 @@
extern void highbank_set_cpu_jump(int cpu, void *jump_addr);
-extern void highbank_clocks_init(void);
extern void __iomem *scu_base_addr;
#ifdef CONFIG_DEBUG_HIGHBANK_UART
extern void highbank_lluart_map_io(void);
diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 88660d5..13a3b15 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -95,24 +95,15 @@ static void __init highbank_init_irq(void)
static void __init highbank_timer_init(void)
{
- int irq;
struct device_node *np;
- void __iomem *timer_base;
/* Map system registers */
np = of_find_compatible_node(NULL, NULL, "calxeda,hb-sregs");
sregs_base = of_iomap(np, 0);
WARN_ON(!sregs_base);
- np = of_find_compatible_node(NULL, NULL, "arm,sp804");
- timer_base = of_iomap(np, 0);
- WARN_ON(!timer_base);
- irq = irq_of_parse_and_map(np, 0);
-
- highbank_clocks_init();
-
- sp804_clocksource_init(timer_base + 0x20, "timer1");
- sp804_clockevents_init(timer_base, irq, "timer0");
+ for_each_compatible_node(np, NULL, "arm,sp804")
+ sp804_dt_setup(np);
}
static struct sys_timer highbank_timer = {
diff --git a/arch/arm/mach-highbank/include/mach/clkdev.h b/arch/arm/mach-highbank/include/mach/clkdev.h
new file mode 100644
index 0000000..0fe8f0e
--- /dev/null
+++ b/arch/arm/mach-highbank/include/mach/clkdev.h
@@ -0,0 +1,11 @@
+#ifndef __ASM_MACH_CLKDEV_H
+#define __ASM_MACH_CLKDEV_H
+
+struct clk {
+ unsigned long rate;
+};
+
+#define __clk_get(clk) ({ 1; })
+#define __clk_put(clk) do { } while (0)
+
+#endif
--
1.7.5.4
^ permalink raw reply related [flat|nested] 39+ messages in thread