2012-06-12 14:42:05

by Rob Herring

[permalink] [raw]
Subject: [PATCH v3 0/4] DT clock bindings

From: Rob Herring <[email protected]>

This series defines clock bindings for Device-Tree and adds kernel
support using the common clock infrastructure. The last patch enables
DT clock support for the Calxeda Highbank platform.

I'm posting this again to solicit further review. There has been some
discussion[1], but no definite path forward. This series is not changed
from the last post other than rebasing to v3.5-rc2.

Rob

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html

Grant Likely (2):
clk: add DT clock binding support
clk: add DT fixed-clock binding support

Rob Herring (2):
dt: add clock binding doc to primecell bindings
clk: add highbank clock support

.../devicetree/bindings/arm/primecell.txt | 6 +
.../devicetree/bindings/clock/calxeda.txt | 17 +
.../devicetree/bindings/clock/clock-bindings.txt | 117 +++++++
.../devicetree/bindings/clock/fixed-clock.txt | 21 ++
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/highbank.dts | 91 +++++-
arch/arm/mach-highbank/Makefile | 2 +-
arch/arm/mach-highbank/clock.c | 62 ----
arch/arm/mach-highbank/highbank.c | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-fixed-rate.c | 23 ++
drivers/clk/clk-highbank.c | 345 ++++++++++++++++++++
drivers/clk/clk.c | 140 ++++++++
drivers/clk/clkdev.c | 77 +++++
include/linux/clk-provider.h | 16 +
include/linux/clk.h | 19 ++
16 files changed, 881 insertions(+), 64 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt
create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt
create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt
delete mode 100644 arch/arm/mach-highbank/clock.c
create mode 100644 drivers/clk/clk-highbank.c

--
1.7.9.5


2012-06-12 14:42:08

by Rob Herring

[permalink] [raw]
Subject: [PATCH v3 1/4] clk: add DT clock binding support

From: Grant Likely <[email protected]>

Based on work 1st by Ben Herrenschmidt and Jeremy Kerr, then by Grant
Likely, this patch adds support to clk_get to allow drivers to retrieve
clock data from the device tree.

Platforms scan for clocks in DT with of_clk_init and a match table, and
the register a provider through of_clk_add_provider. The provider's
clk_src_get function will be called when a device references the
provider's OF node for a clock reference.

v5 (Rob Herring):
- Move from drivers/of into common clock subsystem
- Squashed "dt/clock: add a simple provider get function" and
"dt/clock: add function to get parent clock name"
- Rebase to 3.4-rc1
- Drop CONFIG_OF_CLOCK and just use CONFIG_OF
- Add missing EXPORT_SYMBOL to various functions
- s/clock-output-name/clock-output-names/
- Define that fixed-clock binding is a single output

v4 (Rob Herring):
- Rework for common clk subsystem
- Add of_clk_get_parent_name function

v3: - Clarified documentation

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 <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
Reviewed-by: Shawn Guo <[email protected]>
Cc: Sascha Hauer <[email protected]>
Cc: Mike Turquette <[email protected]>
---
.../devicetree/bindings/clock/clock-bindings.txt | 117 ++++++++++++++++
.../devicetree/bindings/clock/fixed-clock.txt | 21 +++
drivers/clk/clk.c | 140 ++++++++++++++++++++
drivers/clk/clkdev.c | 77 +++++++++++
include/linux/clk-provider.h | 14 ++
include/linux/clk.h | 19 +++
6 files changed, 388 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/clock-bindings.txt
create mode 100644 Documentation/devicetree/bindings/clock/fixed-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
new file mode 100644
index 0000000..eb65d41
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -0,0 +1,117 @@
+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 0 for nodes
+ with a single clock output and 1 for nodes with multiple
+ clock outputs.
+
+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-names 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-names 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. Note: if the
+ clock provider specifies '0' for #clock-cells, then
+ only the phandle portion of the pair will appear.
+
+Optional properties:
+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.
+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..0b1fe78
--- /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/clock-bindings.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. Should be a single cell.
+
+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/clk.c b/drivers/clk/clk.c
index 687b00d..456f5fb 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/list.h>
#include <linux/slab.h>
+#include <linux/of.h>

static DEFINE_SPINLOCK(enable_lock);
static DEFINE_MUTEX(prepare_lock);
@@ -1544,3 +1545,142 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb)
return ret;
}
EXPORT_SYMBOL_GPL(clk_notifier_unregister);
+
+#ifdef CONFIG_OF
+/**
+ * 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);
+
+struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
+ void *data)
+{
+ return data;
+}
+EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
+
+/**
+ * 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;
+}
+EXPORT_SYMBOL_GPL(of_clk_add_provider);
+
+/**
+ * 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);
+}
+EXPORT_SYMBOL_GPL(of_clk_del_provider);
+
+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;
+}
+
+const char *of_clk_get_parent_name(struct device_node *np, int index)
+{
+ struct of_phandle_args clkspec;
+ const char *clk_name;
+ int rc;
+
+ if (index < 0)
+ return NULL;
+
+ rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
+ &clkspec);
+ if (rc)
+ return NULL;
+
+ if (of_property_read_string_index(clkspec.np, "clock-output-names",
+ clkspec.args_count ? clkspec.args[0] : 0,
+ &clk_name) < 0)
+ clk_name = clkspec.np->name;
+
+ of_node_put(clkspec.np);
+ return clk_name;
+}
+EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
+
+/**
+ * of_clk_init() - Scan and init clock providers from the DT
+ * @matches: array of compatible values and init functions for providers.
+ *
+ * This function scans the device tree for matching clock providers and
+ * calls their initialization functions
+ */
+void __init of_clk_init(const struct of_device_id *matches)
+{
+ struct device_node *np;
+
+ for_each_matching_node(np, matches) {
+ const struct of_device_id *match = of_match_node(matches, np);
+ of_clk_init_cb_t clk_init_cb = match->data;
+ clk_init_cb(np);
+ }
+}
+#endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index c535cf8..b387435 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -19,10 +19,80 @@
#include <linux/mutex.h>
#include <linux/clk.h>
#include <linux/clkdev.h>
+#include <linux/of.h>

static LIST_HEAD(clocks);
static DEFINE_MUTEX(clocks_mutex);

+#ifdef CONFIG_OF
+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;
+}
+EXPORT_SYMBOL(of_clk_get);
+
+/**
+ * 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;
+}
+EXPORT_SYMBOL(of_clk_get_by_name);
+#endif
+
/*
* Find the correct struct clk for the device and connection ID.
* We do slightly fuzzy matching here:
@@ -83,6 +153,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/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4a0b483..acfef45 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -334,5 +334,19 @@ void __clk_unprepare(struct clk *clk);
void __clk_reparent(struct clk *clk, struct clk *new_parent);
unsigned long __clk_round_rate(struct clk *clk, unsigned long rate);

+struct of_device_id;
+
+typedef void (*of_clk_init_cb_t)(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_src_simple_get(struct of_phandle_args *clkspec,
+ void *data);
+const char *of_clk_get_parent_name(struct device_node *np, int index);
+void of_clk_init(const struct of_device_id *matches);
+
#endif /* CONFIG_COMMON_CLK */
#endif /* CLK_PROVIDER_H */
diff --git a/include/linux/clk.h b/include/linux/clk.h
index ad5c43e..8b70342 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -310,4 +310,23 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id);
int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,
struct device *dev);

+struct device_node;
+struct of_phandle_args;
+
+#ifdef CONFIG_OF
+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);
+struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
+#else
+static inline struct clk *of_clk_get(struct device_node *np, int index)
+{
+ return NULL;
+}
+static inline struct clk *of_clk_get_by_name(struct device_node *np,
+ const char *name)
+{
+ return NULL;
+}
+#endif
+
#endif
--
1.7.9.5

2012-06-12 14:42:18

by Rob Herring

[permalink] [raw]
Subject: [PATCH v3 4/4] clk: add highbank clock support

From: Rob Herring <[email protected]>

This adds real clock support to Calxeda Highbank SOC using the common
clock infrastructure.

Signed-off-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/clock/calxeda.txt | 17 +
arch/arm/Kconfig | 1 +
arch/arm/boot/dts/highbank.dts | 91 +++++-
arch/arm/mach-highbank/Makefile | 2 +-
arch/arm/mach-highbank/clock.c | 62 ----
arch/arm/mach-highbank/highbank.c | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-highbank.c | 345 ++++++++++++++++++++
8 files changed, 462 insertions(+), 64 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/calxeda.txt
delete mode 100644 arch/arm/mach-highbank/clock.c
create mode 100644 drivers/clk/clk-highbank.c

diff --git a/Documentation/devicetree/bindings/clock/calxeda.txt b/Documentation/devicetree/bindings/clock/calxeda.txt
new file mode 100644
index 0000000..0a6ac1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/calxeda.txt
@@ -0,0 +1,17 @@
+Device Tree Clock bindings for Calxeda highbank platform
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be one of the following:
+ "calxeda,hb-pll-clock" - for a PLL clock
+ "calxeda,hb-a9periph-clock" - The A9 peripheral clock divided from the
+ A9 clock.
+ "calxeda,hb-a9bus-clock" - The A9 bus clock divided from the A9 clock.
+ "calxeda,hb-emmc-clock" - Divided clock for MMC/SD controller.
+- reg : shall be the control register offset from SYSREGs base for the clock.
+- clocks : shall be the input parent clock phandle for the clock. This is
+ either an oscillator or a pll output.
+- #clock-cells : from common clock binding; shall be set to 0.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 84449dd..00ce396 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -348,6 +348,7 @@ config ARCH_HIGHBANK
select ARM_TIMER_SP804
select CACHE_L2X0
select CLKDEV_LOOKUP
+ select COMMON_CLK
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 83e7229..2e1cfa0 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -1,5 +1,5 @@
/*
- * Copyright 2011 Calxeda, Inc.
+ * Copyright 2011-2012 Calxeda, Inc.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms and conditions of the GNU General Public License,
@@ -24,6 +24,7 @@
compatible = "calxeda,highbank";
#address-cells = <1>;
#size-cells = <1>;
+ clock-ranges;

cpus {
#address-cells = <1>;
@@ -33,24 +34,32 @@
compatible = "arm,cortex-a9";
reg = <0>;
next-level-cache = <&L2>;
+ clocks = <&a9pll>;
+ clock-names = "cpu";
};

cpu@1 {
compatible = "arm,cortex-a9";
reg = <1>;
next-level-cache = <&L2>;
+ clocks = <&a9pll>;
+ clock-names = "cpu";
};

cpu@2 {
compatible = "arm,cortex-a9";
reg = <2>;
next-level-cache = <&L2>;
+ clocks = <&a9pll>;
+ clock-names = "cpu";
};

cpu@3 {
compatible = "arm,cortex-a9";
reg = <3>;
next-level-cache = <&L2>;
+ clocks = <&a9pll>;
+ clock-names = "cpu";
};
};

@@ -75,12 +84,14 @@
compatible = "arm,cortex-a9-twd-timer";
reg = <0xfff10600 0x20>;
interrupts = <1 13 0xf01>;
+ clocks = <&a9periphclk>;
};

watchdog@fff10620 {
compatible = "arm,cortex-a9-twd-wdt";
reg = <0xfff10620 0x20>;
interrupts = <1 14 0xf01>;
+ clocks = <&a9periphclk>;
};

intc: interrupt-controller@fff11000 {
@@ -116,12 +127,15 @@
compatible = "calxeda,hb-sdhci";
reg = <0xffe0e000 0x1000>;
interrupts = <0 90 4>;
+ clocks = <&eclk>;
};

ipc@fff20000 {
compatible = "arm,pl320", "arm,primecell";
reg = <0xfff20000 0x1000>;
interrupts = <0 7 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

gpioe: gpio@fff30000 {
@@ -130,6 +144,8 @@
gpio-controller;
reg = <0xfff30000 0x1000>;
interrupts = <0 14 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

gpiof: gpio@fff31000 {
@@ -138,6 +154,8 @@
gpio-controller;
reg = <0xfff31000 0x1000>;
interrupts = <0 15 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

gpiog: gpio@fff32000 {
@@ -146,6 +164,8 @@
gpio-controller;
reg = <0xfff32000 0x1000>;
interrupts = <0 16 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

gpioh: gpio@fff33000 {
@@ -154,24 +174,32 @@
gpio-controller;
reg = <0xfff33000 0x1000>;
interrupts = <0 17 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

timer {
compatible = "arm,sp804", "arm,primecell";
reg = <0xfff34000 0x1000>;
interrupts = <0 18 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

rtc@fff35000 {
compatible = "arm,pl031", "arm,primecell";
reg = <0xfff35000 0x1000>;
interrupts = <0 19 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

serial@fff36000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0xfff36000 0x1000>;
interrupts = <0 20 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

smic@fff3a000 {
@@ -186,12 +214,73 @@
sregs@fff3c000 {
compatible = "calxeda,hb-sregs";
reg = <0xfff3c000 0x1000>;
+
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ osc: oscillator {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <33333000>;
+ };
+
+ ddrpll: ddrpll {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-pll-clock";
+ clocks = <&osc>;
+ reg = <0x108>;
+ };
+
+ a9pll: a9pll {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-pll-clock";
+ clocks = <&osc>;
+ reg = <0x100>;
+ };
+
+ a9periphclk: a9periphclk {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-a9periph-clock";
+ clocks = <&a9pll>;
+ reg = <0x104>;
+ };
+
+ a9bclk: a9bclk {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-a9bus-clock";
+ clocks = <&a9pll>;
+ reg = <0x104>;
+ };
+
+ emmcpll: emmcpll {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-pll-clock";
+ clocks = <&osc>;
+ reg = <0x10C>;
+ };
+
+ eclk: eclk {
+ #clock-cells = <0>;
+ compatible = "calxeda,hb-emmc-clock";
+ clocks = <&emmcpll>;
+ reg = <0x114>;
+ };
+
+ pclk: pclk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <150000000>;
+ };
+ };
};

dma@fff3d000 {
compatible = "arm,pl330", "arm,primecell";
reg = <0xfff3d000 0x1000>;
interrupts = <0 92 4>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
};

ethernet@fff50000 {
diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
index f8437dd..6ca6afa 100644
--- a/arch/arm/mach-highbank/Makefile
+++ b/arch/arm/mach-highbank/Makefile
@@ -1,4 +1,4 @@
-obj-y := clock.o highbank.o system.o
+obj-y := highbank.o system.o
obj-$(CONFIG_DEBUG_HIGHBANK_UART) += lluart.o
obj-$(CONFIG_SMP) += platsmp.o
obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
diff --git a/arch/arm/mach-highbank/clock.c b/arch/arm/mach-highbank/clock.c
deleted file mode 100644
index c25a2ae..0000000
--- a/arch/arm/mach-highbank/clock.c
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright 2011 Calxeda, Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program. If not, see <http://www.gnu.org/licenses/>.
- */
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/errno.h>
-#include <linux/clk.h>
-#include <linux/clkdev.h>
-
-struct clk {
- unsigned long rate;
-};
-
-int clk_enable(struct clk *clk)
-{
- return 0;
-}
-
-void clk_disable(struct clk *clk)
-{}
-
-unsigned long clk_get_rate(struct clk *clk)
-{
- return clk->rate;
-}
-
-long clk_round_rate(struct clk *clk, unsigned long rate)
-{
- return clk->rate;
-}
-
-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/highbank.c b/arch/arm/mach-highbank/highbank.c
index 410a112..ce72d78 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -91,6 +91,11 @@ static void __init highbank_init_irq(void)
l2x0_of_init(0, ~0UL);
}

+static struct clk_lookup lookup = {
+ .dev_id = "sp804",
+ .con_id = NULL,
+};
+
static void __init highbank_timer_init(void)
{
int irq;
@@ -108,6 +113,8 @@ static void __init highbank_timer_init(void)
irq = irq_of_parse_and_map(np, 0);

highbank_clocks_init();
+ lookup.clk = of_clk_get(np, 0);
+ clkdev_add(&lookup);

sp804_clocksource_and_sched_clock_init(timer_base + 0x20, "timer1");
sp804_clockevents_init(timer_base, irq, "timer0");
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index b9a5158..ce384f3 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -3,5 +3,6 @@ obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
obj-$(CONFIG_COMMON_CLK) += clk.o clk-fixed-rate.o clk-gate.o \
clk-mux.o clk-divider.o clk-fixed-factor.o
# SoCs specific
+obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
obj-$(CONFIG_ARCH_MXS) += mxs/
obj-$(CONFIG_PLAT_SPEAR) += spear/
diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
new file mode 100644
index 0000000..2f61065
--- /dev/null
+++ b/drivers/clk/clk-highbank.c
@@ -0,0 +1,345 @@
+/*
+ * Copyright 2011-2012 Calxeda, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+
+extern void __iomem *sregs_base;
+
+#define HB_PLL_LOCK_500 0x20000000
+#define HB_PLL_LOCK 0x10000000
+#define HB_PLL_DIVF_SHIFT 20
+#define HB_PLL_DIVF_MASK 0x0ff00000
+#define HB_PLL_DIVQ_SHIFT 16
+#define HB_PLL_DIVQ_MASK 0x00070000
+#define HB_PLL_DIVR_SHIFT 8
+#define HB_PLL_DIVR_MASK 0x00001f00
+#define HB_PLL_RANGE_SHIFT 4
+#define HB_PLL_RANGE_MASK 0x00000070
+#define HB_PLL_BYPASS 0x00000008
+#define HB_PLL_RESET 0x00000004
+#define HB_PLL_EXT_BYPASS 0x00000002
+#define HB_PLL_EXT_ENA 0x00000001
+
+#define HB_PLL_VCO_MIN_FREQ 2133000000
+#define HB_PLL_MAX_FREQ HB_PLL_VCO_MIN_FREQ
+#define HB_PLL_MIN_FREQ (HB_PLL_VCO_MIN_FREQ / 64)
+
+#define HB_A9_BCLK_DIV_MASK 0x00000006
+#define HB_A9_BCLK_DIV_SHIFT 1
+#define HB_A9_PCLK_DIV 0x00000001
+
+struct hb_clk {
+ struct clk_hw hw;
+ void __iomem *reg;
+ char *parent_name;
+};
+#define to_hb_clk(p) container_of(p, struct hb_clk, hw)
+
+static int clk_pll_prepare(struct clk_hw *hwclk)
+ {
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 reg;
+
+ reg = readl(hbclk->reg);
+ reg &= ~HB_PLL_RESET;
+ writel(reg, hbclk->reg);
+
+ while ((readl(hbclk->reg) & HB_PLL_LOCK) == 0)
+ ;
+ while ((readl(hbclk->reg) & HB_PLL_LOCK_500) == 0)
+ ;
+
+ return 0;
+}
+
+static void clk_pll_unprepare(struct clk_hw *hwclk)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 reg;
+
+ reg = readl(hbclk->reg);
+ reg |= HB_PLL_RESET;
+ writel(reg, hbclk->reg);
+}
+
+static int clk_pll_enable(struct clk_hw *hwclk)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 reg;
+
+ reg = readl(hbclk->reg);
+ reg |= HB_PLL_EXT_ENA;
+ writel(reg, hbclk->reg);
+
+ return 0;
+}
+
+static void clk_pll_disable(struct clk_hw *hwclk)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 reg;
+
+ reg = readl(hbclk->reg);
+ reg &= ~HB_PLL_EXT_ENA;
+ writel(reg, hbclk->reg);
+}
+
+static unsigned long clk_pll_recalc_rate(struct clk_hw *hwclk,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ unsigned long divf, divq, vco_freq, reg;
+
+ reg = readl(hbclk->reg);
+ if (reg & HB_PLL_EXT_BYPASS)
+ return parent_rate;
+
+ divf = (reg & HB_PLL_DIVF_MASK) >> HB_PLL_DIVF_SHIFT;
+ divq = (reg & HB_PLL_DIVQ_MASK) >> HB_PLL_DIVQ_SHIFT;
+ vco_freq = parent_rate * (divf + 1);
+
+ return vco_freq / (1 << divq);
+}
+
+static void clk_pll_calc(unsigned long rate, unsigned long ref_freq,
+ u32 *pdivq, u32 *pdivf)
+{
+ u32 divq, divf;
+ unsigned long vco_freq;
+
+ if (rate < HB_PLL_MIN_FREQ)
+ rate = HB_PLL_MIN_FREQ;
+ if (rate > HB_PLL_MAX_FREQ)
+ rate = HB_PLL_MAX_FREQ;
+
+ for (divq = 1; divq <= 6; divq++) {
+ if ((rate * (1 << divq)) >= HB_PLL_VCO_MIN_FREQ)
+ break;
+ }
+
+ vco_freq = rate * (1 << divq);
+ divf = (vco_freq + (ref_freq / 2)) / ref_freq;
+ divf--;
+
+ *pdivq = divq;
+ *pdivf = divf;
+}
+
+static long clk_pll_round_rate(struct clk_hw *hwclk, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ u32 divq, divf;
+ unsigned long ref_freq = *parent_rate;
+
+ clk_pll_calc(rate, ref_freq, &divq, &divf);
+
+ return (ref_freq * (divf + 1)) / (1 << divq);
+}
+
+static int clk_pll_set_rate(struct clk_hw *hwclk, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 divq, divf;
+ u32 reg;
+
+ clk_pll_calc(rate, parent_rate, &divq, &divf);
+
+ reg = readl(hbclk->reg);
+ if (divf != ((reg & HB_PLL_DIVF_MASK) >> HB_PLL_DIVF_SHIFT)) {
+ /* Need to re-lock PLL, so put it into bypass mode */
+ reg |= HB_PLL_EXT_BYPASS;
+ writel(reg | HB_PLL_EXT_BYPASS, hbclk->reg);
+
+ writel(reg | HB_PLL_RESET, hbclk->reg);
+ reg &= ~(HB_PLL_DIVF_MASK | HB_PLL_DIVQ_MASK);
+ reg |= (divf << HB_PLL_DIVF_SHIFT) | (divq << HB_PLL_DIVQ_SHIFT);
+ writel(reg | HB_PLL_RESET, hbclk->reg);
+ writel(reg, hbclk->reg);
+
+ while ((readl(hbclk->reg) & HB_PLL_LOCK) == 0)
+ ;
+ while ((readl(hbclk->reg) & HB_PLL_LOCK_500) == 0)
+ ;
+ reg |= HB_PLL_EXT_ENA;
+ reg &= ~HB_PLL_EXT_BYPASS;
+ } else {
+ reg &= ~HB_PLL_DIVQ_MASK;
+ reg |= divq << HB_PLL_DIVQ_SHIFT;
+ }
+ writel(reg, hbclk->reg);
+
+ return 0;
+}
+
+static const struct clk_ops clk_pll_ops = {
+ .prepare = clk_pll_prepare,
+ .unprepare = clk_pll_unprepare,
+ .enable = clk_pll_enable,
+ .disable = clk_pll_disable,
+ .recalc_rate = clk_pll_recalc_rate,
+ .round_rate = clk_pll_round_rate,
+ .set_rate = clk_pll_set_rate,
+};
+
+static unsigned long clk_cpu_periphclk_recalc_rate(struct clk_hw *hwclk,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 div = (readl(hbclk->reg) & HB_A9_PCLK_DIV) ? 8 : 4;
+ return parent_rate / div;
+}
+
+static const struct clk_ops a9periphclk_ops = {
+ .recalc_rate = clk_cpu_periphclk_recalc_rate,
+};
+
+static unsigned long clk_cpu_a9bclk_recalc_rate(struct clk_hw *hwclk,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 div = (readl(hbclk->reg) & HB_A9_BCLK_DIV_MASK) >> HB_A9_BCLK_DIV_SHIFT;
+
+ return parent_rate / (div + 2);
+}
+
+static const struct clk_ops a9bclk_ops = {
+ .recalc_rate = clk_cpu_a9bclk_recalc_rate,
+};
+
+static unsigned long clk_periclk_recalc_rate(struct clk_hw *hwclk,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 div;
+
+ div = readl(hbclk->reg) & 0x1f;
+ div++;
+ div *= 2;
+
+ return parent_rate / div;
+}
+
+static long clk_periclk_round_rate(struct clk_hw *hwclk, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ u32 div;
+
+ div = *parent_rate / rate;
+ div++;
+ div &= ~0x1;
+
+ return *parent_rate / div;
+}
+
+static int clk_periclk_set_rate(struct clk_hw *hwclk, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct hb_clk *hbclk = to_hb_clk(hwclk);
+ u32 div;
+
+ div = parent_rate / rate;
+ if (div & 0x1)
+ return -EINVAL;
+
+ writel(div >> 1, hbclk->reg);
+ return 0;
+}
+
+static const struct clk_ops periclk_ops = {
+ .recalc_rate = clk_periclk_recalc_rate,
+ .round_rate = clk_periclk_round_rate,
+ .set_rate = clk_periclk_set_rate,
+};
+
+static __init struct clk *hb_clk_init(struct device_node *node, const struct clk_ops *ops)
+{
+ u32 reg;
+ struct clk *clk;
+ struct hb_clk *hb_clk;
+ const char *clk_name = node->name;
+ const char *parent_name;
+ struct clk_init_data init_data;
+ int rc;
+
+ rc = of_property_read_u32(node, "reg", &reg);
+ if (WARN_ON(rc))
+ return NULL;
+
+ hb_clk = kzalloc(sizeof(*hb_clk), GFP_KERNEL);
+ if (WARN_ON(!hb_clk))
+ return NULL;
+
+ hb_clk->reg = sregs_base + reg;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ hb_clk->hw.init = &init_data;
+ hb_clk->hw.init->name = clk_name;
+ hb_clk->hw.init->num_parents = 1;
+ parent_name = of_clk_get_parent_name(node, 0);
+ hb_clk->hw.init->parent_names = &parent_name;
+ hb_clk->hw.init->ops = ops;
+ hb_clk->hw.init->flags = 0;
+
+ clk = clk_register(NULL, &hb_clk->hw);
+ if (WARN_ON(IS_ERR(clk))) {
+ kfree(hb_clk);
+ return NULL;
+ }
+ rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+ return clk;
+}
+
+static void __init hb_pll_init(struct device_node *node)
+{
+ hb_clk_init(node, &clk_pll_ops);
+}
+
+static void __init hb_a9periph_init(struct device_node *node)
+{
+ hb_clk_init(node, &a9periphclk_ops);
+}
+
+static void __init hb_a9bus_init(struct device_node *node)
+{
+ struct clk *clk = hb_clk_init(node, &a9bclk_ops);
+ clk_prepare_enable(clk);
+}
+
+static void __init hb_emmc_init(struct device_node *node)
+{
+ hb_clk_init(node, &periclk_ops);
+}
+
+static const __initconst struct of_device_id clk_match[] = {
+ { .compatible = "fixed-clock", .data = of_fixed_clk_setup, },
+ { .compatible = "calxeda,hb-pll-clock", .data = hb_pll_init, },
+ { .compatible = "calxeda,hb-a9periph-clock", .data = hb_a9periph_init, },
+ { .compatible = "calxeda,hb-a9bus-clock", .data = hb_a9bus_init, },
+ { .compatible = "calxeda,hb-emmc-clock", .data = hb_emmc_init, },
+ {}
+};
+
+void __init highbank_clocks_init(void)
+{
+ of_clk_init(clk_match);
+}
--
1.7.9.5

2012-06-12 14:42:14

by Rob Herring

[permalink] [raw]
Subject: [PATCH v3 2/4] clk: add DT fixed-clock binding support

From: Grant Likely <[email protected]>

Add support for DT "fixed-clock" binding to the common fixed rate clock
support.

Signed-off-by: Grant Likely <[email protected]>
[Rob Herring] Rework and move into common clock infrastructure
Signed-off-by: Rob Herring <[email protected]>
---
drivers/clk/clk-fixed-rate.c | 23 +++++++++++++++++++++++
include/linux/clk-provider.h | 2 ++
2 files changed, 25 insertions(+)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index cbd2462..94493f1 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -14,6 +14,7 @@
#include <linux/slab.h>
#include <linux/io.h>
#include <linux/err.h>
+#include <linux/of.h>

/*
* DOC: basic fixed-rate clock that cannot gate
@@ -79,3 +80,25 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,

return clk;
}
+
+#ifdef CONFIG_OF
+/**
+ * of_fixed_clk_setup() - Setup function for simple fixed rate clock
+ */
+void __init of_fixed_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ const char *clk_name = node->name;
+ u32 rate;
+
+ if (of_property_read_u32(node, "clock-frequency", &rate))
+ return;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ clk = clk_register_fixed_rate(NULL, clk_name, NULL, CLK_IS_ROOT, rate);
+ if (clk)
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+EXPORT_SYMBOL_GPL(of_fixed_clk_setup);
+#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index acfef45..b97f61e 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -171,6 +171,8 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned long fixed_rate);

+void of_fixed_clk_setup(struct device_node *np);
+
/**
* struct clk_gate - gating clock
*
--
1.7.9.5

2012-06-12 14:42:46

by Rob Herring

[permalink] [raw]
Subject: [PATCH v3 3/4] dt: add clock binding doc to primecell bindings

From: Rob Herring <[email protected]>

Add clock binding information for primecell peripherals. For most, a
clock input name of "apb_pclk" is required. Any primecell peripherals
which are different will need to be documented separately.

Signed-off-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/arm/primecell.txt | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/primecell.txt b/Documentation/devicetree/bindings/arm/primecell.txt
index 951ca46..64fc82b 100644
--- a/Documentation/devicetree/bindings/arm/primecell.txt
+++ b/Documentation/devicetree/bindings/arm/primecell.txt
@@ -13,11 +13,17 @@ Required properties:
Optional properties:

- arm,primecell-periphid : Value to override the h/w value with
+- clocks : From common clock binding. First clock is phandle to clock for apb
+ pclk. Additional clocks are optional and specific to those peripherals.
+- clock-names : From common clock binding. Shall be "apb_pclk" for first clock.

Example:

serial@fff36000 {
compatible = "arm,pl011", "arm,primecell";
arm,primecell-periphid = <0x00341011>;
+ clocks = <&pclk>;
+ clock-names = "apb_pclk";
+
};

--
1.7.9.5

2012-06-12 15:47:44

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 20120612-09:41, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> This series defines clock bindings for Device-Tree and adds kernel
> support using the common clock infrastructure. The last patch enables
> DT clock support for the Calxeda Highbank platform.
>
> I'm posting this again to solicit further review. There has been some
> discussion[1], but no definite path forward. This series is not changed
> from the last post other than rebasing to v3.5-rc2.
>

Hi Rob,

Just FYI, I was following the first thread but it did not seem to reach
resolution. Namely Saravana seemed to have some lingering concerns.
But I'm definitely not ignoring this series. I assume you still want
this to go through me once everyone is on board?

Regards,
Mike

2012-06-12 16:23:25

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 06/12/2012 10:47 AM, Mike Turquette wrote:
> On 20120612-09:41, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> This series defines clock bindings for Device-Tree and adds kernel
>> support using the common clock infrastructure. The last patch enables
>> DT clock support for the Calxeda Highbank platform.
>>
>> I'm posting this again to solicit further review. There has been some
>> discussion[1], but no definite path forward. This series is not changed
>> from the last post other than rebasing to v3.5-rc2.
>>
>
> Hi Rob,
>
> Just FYI, I was following the first thread but it did not seem to reach
> resolution. Namely Saravana seemed to have some lingering concerns.
> But I'm definitely not ignoring this series. I assume you still want
> this to go through me once everyone is on board?

Right. This is why I have reposted and copied those whom commented on my
pull request. I think at least some of Saravana's concerns boiled down
to not requiring using DT clock bindings and not requiring driver
changes to move to DT bindings. Both of these are met with this series.
A platform can still use clkdev tables for binding clocks to devices and
does not have to use DT for clocks. Drivers generally should not require
any changes that I'm aware of, but if there are any concrete examples
I'd like to know.

Yes, I think this should go thru you.

Rob

2012-06-13 15:27:59

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On Tue, Jun 12, 2012 at 04:41:47PM +0200, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> This series defines clock bindings for Device-Tree and adds kernel
> support using the common clock infrastructure. The last patch enables
> DT clock support for the Calxeda Highbank platform.
>
> I'm posting this again to solicit further review. There has been some
> discussion[1], but no definite path forward. This series is not changed
> from the last post other than rebasing to v3.5-rc2.
>
> Rob
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html
>

How would this binding work for clocks which can take a spinlock pointer as a
init parameter (eg. clk-gate).

Cheers,

Peter.

2012-06-13 18:09:41

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

Peter,

On 06/13/2012 10:26 AM, Peter De Schrijver wrote:
> On Tue, Jun 12, 2012 at 04:41:47PM +0200, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> This series defines clock bindings for Device-Tree and adds kernel
>> support using the common clock infrastructure. The last patch enables
>> DT clock support for the Calxeda Highbank platform.
>>
>> I'm posting this again to solicit further review. There has been some
>> discussion[1], but no definite path forward. This series is not changed
>> from the last post other than rebasing to v3.5-rc2.
>>
>> Rob
>>
>> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/100932.html
>>
>
> How would this binding work for clocks which can take a spinlock pointer as a
> init parameter (eg. clk-gate).

Other than fixed clocks, this makes no attempt at generic bindings.
There is still per clock init required which can allocate a spinlock if
necessary. If you look at the highbank code, it would be trivial to
initialize spinlocks as needed.

Rob

> Cheers,
>
> Peter.

2012-06-14 08:49:14

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On Tue, Jun 12, 2012 at 11:23:18AM -0500, Rob Herring wrote:
> Right. This is why I have reposted and copied those whom commented on my
> pull request. I think at least some of Saravana's concerns boiled down
> to not requiring using DT clock bindings and not requiring driver
> changes to move to DT bindings. Both of these are met with this series.
> A platform can still use clkdev tables for binding clocks to devices and
> does not have to use DT for clocks. Drivers generally should not require
> any changes that I'm aware of, but if there are any concrete examples
> I'd like to know.
>
> Yes, I think this should go thru you.
>
I still have a little doubt about how this binding will best work for
imx clock. I'm trying to cook a RFC patch discussing.

--
Regards,
Shawn

2012-06-15 03:32:53

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] clk: add DT clock binding support

On Tue, Jun 12, 2012 at 09:41:48AM -0500, Rob Herring wrote:
> +struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
> +{
> + struct of_clk_provider *provider;
> + struct clk *clk = NULL;

Both clk and clkdev treat NULL as a valid clock and return ERR_PTR for
error case, while all the codes in this patch just return NULL for
error and check (clk != NULL) for valid clock.

Should we force the consistent behavior between DT and non-DT on this?

> +
> + /* 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;
> +}

--
Regards,
Shawn

2012-06-15 04:32:58

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] clk: add DT clock binding support

On 06/14/2012 10:17 PM, Shawn Guo wrote:
> On Tue, Jun 12, 2012 at 09:41:48AM -0500, Rob Herring wrote:
>> +struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
>> +{
>> + struct of_clk_provider *provider;
>> + struct clk *clk = NULL;
>
> Both clk and clkdev treat NULL as a valid clock and return ERR_PTR for
> error case, while all the codes in this patch just return NULL for
> error and check (clk != NULL) for valid clock.

Because Grant hates ERR_PTR... :)

>
> Should we force the consistent behavior between DT and non-DT on this?

Yes, I agree and will change it.

Rob

>
>> +
>> + /* 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;
>> +}
>

2012-06-15 08:39:43

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On Tue, Jun 12, 2012 at 09:41:47AM -0500, Rob Herring wrote:
> I'm posting this again to solicit further review. There has been some
> discussion[1], but no definite path forward. This series is not changed
> from the last post other than rebasing to v3.5-rc2.
>
Hi Rob,

Per your comment[1], the patch below takes imx6q as example to define
single CCM node with a whole bunch of outputs to support clk lookup
with device tree. (Only uart and usdhc clocks are being put there for
demonstration.) Though it seems working, going through the patch you
will see a couple problems which may need to be solved to make the
binding useful for cases like imx.

* Different clk matching behavior between DT and non-DT lookup

Let's take usdhc example (drivers/mmc/host/sdhci-esdhc-imx.c) to
elaborate the problem. The driver is being shared by several imx
SoCs, imx25, imx35, imx5 and imx6. It needs to get 3 clocks below.

imx_data->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
imx_data->clk_ahb = devm_clk_get(&pdev->dev, "ahb");
imx_data->clk_per = devm_clk_get(&pdev->dev, "per");

But not every single imx SoC clock tree implementation has all these 3
clocks for sdhc available. The imx6q happens to have only "per" clock,
and clk-imx6q driver registers one clkdev for each usdhc instance with
con_id being NULL.

clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");

The non-DT lookup will match all those three clk_get calls to that
single clkdev entry. That said, the NULL con_id is treated as wildcard.
When translating above clk_register_clkdev calls into DT lookup, I
would expect having "clock-names" absent should just work with all
those 3 clk_get calls to get <&clks 2>.

usdhc@02190000 {
...
clocks = <&clks 2>;
};

But with the current of_clk implementation, we will have to have
something like below to keep the usdhc behavior same as non-DT lookup.

usdhc@02190000 {
...
clocks = <&clks 2>, <&clks 2>, <&clks 2>;
clock-names = "ipg", "ahb", "per";
};

Can we force all the clk_get calls with different con_id to get the
only clk listed in "clocks" when "clock-names" is absent, so that we
can somehow match the behavior with non-DT lookup?

* phandle argument is not easy for engineering

As we will have a whole bunch of clock outputs listed in ccm node,
which will be referenced by peripheral phandle in form of <&clks index>.
When the list gets long, it becomes hard for people to find the correct
index number for the clock referenced.

Regards,
Shawn

[1] http://thread.gmane.org/gmane.linux.kernel/1300259/focus=1301107

8<---
.../devicetree/bindings/clk/clk-imx6q.txt | 58 ++++++++++++++++++++
arch/arm/boot/dts/imx6q.dtsi | 27 +++++++++-
arch/arm/mach-imx/clk-imx6q.c | 31 ++++++-----
3 files changed, 101 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clk/clk-imx6q.txt

diff --git a/Documentation/devicetree/bindings/clk/clk-imx6q.txt b/Documentation/devicetree/bindings/clk/clk-imx6q.txt
new file mode 100644
index 0000000..c5698a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clk/clk-imx6q.txt
@@ -0,0 +1,58 @@
+* Clock bindings for Freescale i.MX6Q
+
+Required properties:
+- compatible: Should be "fsl,imx6q-ccm"
+- reg: Address and length of the register set
+- interrupts: Should contain CCM interrupt
+- #clock-cells: Should be <1>
+- clock-output-names: A list of clock output that CCM provides. The valid
+ clock names include:
+
+ dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
+ pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m,
+ pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw,
+ periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel,
+ esai_sel, asrc_sel, spdif_sel, gpu2d_axi, gpu3d_axi, gpu2d_core_sel,
+ gpu3d_core_sel, gpu3d_shader_sel, ipu1_sel, ipu2_sel, ldb_di0_sel,
+ ldb_di1_sel, ipu1_di0_pre_sel, ipu1_di1_pre_sel, ipu2_di0_pre_sel,
+ ipu2_di1_pre_sel, ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel,
+ ipu2_di1_sel, hsi_tx_sel, pcie_axi_sel, ssi1_sel, ssi2_sel, ssi3_sel,
+ usdhc1_sel, usdhc2_sel, usdhc3_sel, usdhc4_sel, enfc_sel, emi_sel,
+ emi_slow_sel, vdo_axi_sel, vpu_axi_sel, cko1_sel, periph, periph2,
+ periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf,
+ asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root,
+ gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf,
+ ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre,
+ ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf,
+ ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf,
+ usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf,
+ emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf,
+ mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial,
+ can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet,
+ esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb,
+ hdmi_isfr, i2c1, i2c2, i2c3, iim, enfc, ipu1, ipu1_di0, ipu1_di1, ipu2,
+ ipu2_di0, ldb_di0, ldb_di1, ipu2_di1, hsi_tx, mlb, mmdc_ch0_axi,
+ mmdc_ch1_axi, ocram, openvg_axi, pcie_axi, pwm1, pwm2, pwm3, pwm4,
+ gpmi_bch_apb, gpmi_bch, gpmi_io, gpmi_apb, sata, sdma, spba, ssi1,
+ ssi2, ssi3, uart_ipg, uart_serial, usboh3, usdhc1, usdhc2, usdhc3,
+ usdhc4, vdo_axi, vpu_axi, cko1, pll1_sys, pll2_bus, pll3_usb_otg,
+ pll4_audio, pll5_video, pll6_mlb, pll7_usb_host, pll8_enet, ssi1_ipg,
+ ssi2_ipg, ssi3_ipg, rom,
+
+ But generally, only the clocks that peripheral nodes have a phandle pointing
+ to need to be listed there.
+
+Examples:
+
+clks: ccm@020c4000 {
+ compatible = "fsl,imx6q-ccm";
+ reg = <0x020c4000 0x4000>;
+ interrupts = <0 87 0x04 0 88 0x04>;
+ #clock-cells = <1>;
+ clock-output-names = "uart_ipg",
+ "uart_serial",
+ "usdhc1",
+ "usdhc2",
+ "usdhc3",
+ "usdhc4";
+};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 8c90cba..51b915835 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -169,6 +169,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x02020000 0x4000>;
interrupts = <0 26 0x04>;
+ clocks = <&clks 0>, <&clks 1>;
+ clock-names = "ipg", "per";
status = "disabled";
};

@@ -348,10 +350,17 @@
status = "disabled";
};

- ccm@020c4000 {
+ clks: ccm@020c4000 {
compatible = "fsl,imx6q-ccm";
reg = <0x020c4000 0x4000>;
interrupts = <0 87 0x04 0 88 0x04>;
+ #clock-cells = <1>;
+ clock-output-names = "uart_ipg",
+ "uart_serial",
+ "usdhc1",
+ "usdhc2",
+ "usdhc3",
+ "usdhc4";
};

anatop@020c8000 {
@@ -589,6 +598,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02190000 0x4000>;
interrupts = <0 22 0x04>;
+ clocks = <&clks 2>, <&clks 2>, <&clks 2>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};

@@ -596,6 +607,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02194000 0x4000>;
interrupts = <0 23 0x04>;
+ clocks = <&clks 3>, <&clks 3>, <&clks 3>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};

@@ -603,6 +616,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x02198000 0x4000>;
interrupts = <0 24 0x04>;
+ clocks = <&clks 4>, <&clks 4>, <&clks 4>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};

@@ -610,6 +625,8 @@
compatible = "fsl,imx6q-usdhc";
reg = <0x0219c000 0x4000>;
interrupts = <0 25 0x04>;
+ clocks = <&clks 5>, <&clks 5>, <&clks 5>;
+ clock-names = "ipg", "ahb", "per";
status = "disabled";
};

@@ -700,6 +717,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021e8000 0x4000>;
interrupts = <0 27 0x04>;
+ clocks = <&clks 0>, <&clks 1>;
+ clock-names = "ipg", "per";
status = "disabled";
};

@@ -707,6 +726,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021ec000 0x4000>;
interrupts = <0 28 0x04>;
+ clocks = <&clks 0>, <&clks 1>;
+ clock-names = "ipg", "per";
status = "disabled";
};

@@ -714,6 +735,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021f0000 0x4000>;
interrupts = <0 29 0x04>;
+ clocks = <&clks 0>, <&clks 1>;
+ clock-names = "ipg", "per";
status = "disabled";
};

@@ -721,6 +744,8 @@
compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
reg = <0x021f4000 0x4000>;
interrupts = <0 30 0x04>;
+ clocks = <&clks 0>, <&clks 1>;
+ clock-names = "ipg", "per";
status = "disabled";
};
};
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index abb42e7..87b134e 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -57,6 +57,21 @@ static void __iomem *ccm_base;

void __init imx6q_clock_map_io(void) { }

+static struct clk *imx_clk_src_get(struct of_phandle_args *clkspec,
+ void *data)
+{
+ const char *clk_name;
+ int idx = clkspec->args[0];
+ int ret;
+
+ ret = of_property_read_string_index(clkspec->np, "clock-output-names",
+ idx, &clk_name);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ return __clk_lookup(clk_name);
+}
+
int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
{
u32 val = readl_relaxed(ccm_base + CLPCR);
@@ -391,21 +406,7 @@ int __init mx6q_clocks_init(void)
clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
clk_register_clkdev(clk[twd], NULL, "smp_twd");
- clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21e8000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21ec000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21ec000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21f0000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21f0000.serial");
- clk_register_clkdev(clk[uart_serial], "per", "21f4000.serial");
- clk_register_clkdev(clk[uart_ipg], "ipg", "21f4000.serial");
clk_register_clkdev(clk[enet], NULL, "2188000.ethernet");
- clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
- clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
- clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
- clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
clk_register_clkdev(clk[i2c1], NULL, "21a0000.i2c");
clk_register_clkdev(clk[i2c2], NULL, "21a4000.i2c");
clk_register_clkdev(clk[i2c3], NULL, "21a8000.i2c");
@@ -422,6 +423,8 @@ int __init mx6q_clocks_init(void)
clk_register_clkdev(clk[ahb], "ahb", NULL);
clk_register_clkdev(clk[cko1], "cko1", NULL);

+ of_clk_add_provider(np, imx_clk_src_get, NULL);
+
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
clk_prepare_enable(clk[clks_init_on[i]]);

--
1.7.5.4

2012-06-15 15:40:13

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 06/15/2012 02:39 AM, Shawn Guo wrote:
> On Tue, Jun 12, 2012 at 09:41:47AM -0500, Rob Herring wrote:
>> I'm posting this again to solicit further review. There has been some
>> discussion[1], but no definite path forward. This series is not changed
>> from the last post other than rebasing to v3.5-rc2.
>>
> Hi Rob,
>
> Per your comment[1], the patch below takes imx6q as example to define
> single CCM node with a whole bunch of outputs to support clk lookup
> with device tree. (Only uart and usdhc clocks are being put there for
> demonstration.) Though it seems working, going through the patch you
> will see a couple problems which may need to be solved to make the
> binding useful for cases like imx.
...
> * phandle argument is not easy for engineering
>
> As we will have a whole bunch of clock outputs listed in ccm node,
> which will be referenced by peripheral phandle in form of <&clks index>.
> When the list gets long, it becomes hard for people to find the correct
> index number for the clock referenced.

You can (well, probably /should/) list the clock IDs as well as names in
the binding documentation. Then, there's no guess-work or counting
involved. I did this in a very early Tegra clock binding proposal:

http://patchwork.ozlabs.org/patch/141359/

Hopefully, dtc will grow named-constants or a pre-processor sometime and
help this too.

2012-06-15 21:08:06

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 06/15/2012 03:39 AM, Shawn Guo wrote:
> On Tue, Jun 12, 2012 at 09:41:47AM -0500, Rob Herring wrote:
>> I'm posting this again to solicit further review. There has been some
>> discussion[1], but no definite path forward. This series is not changed
>> from the last post other than rebasing to v3.5-rc2.
>>
> Hi Rob,
>
> Per your comment[1], the patch below takes imx6q as example to define
> single CCM node with a whole bunch of outputs to support clk lookup
> with device tree. (Only uart and usdhc clocks are being put there for
> demonstration.) Though it seems working, going through the patch you
> will see a couple problems which may need to be solved to make the
> binding useful for cases like imx.
>
> * Different clk matching behavior between DT and non-DT lookup
>
> Let's take usdhc example (drivers/mmc/host/sdhci-esdhc-imx.c) to
> elaborate the problem. The driver is being shared by several imx
> SoCs, imx25, imx35, imx5 and imx6. It needs to get 3 clocks below.
>
> imx_data->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
> imx_data->clk_ahb = devm_clk_get(&pdev->dev, "ahb");
> imx_data->clk_per = devm_clk_get(&pdev->dev, "per");
>
> But not every single imx SoC clock tree implementation has all these 3
> clocks for sdhc available. The imx6q happens to have only "per" clock,
> and clk-imx6q driver registers one clkdev for each usdhc instance with
> con_id being NULL.
>
> clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
> clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
> clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
> clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
>
> The non-DT lookup will match all those three clk_get calls to that
> single clkdev entry. That said, the NULL con_id is treated as wildcard.
> When translating above clk_register_clkdev calls into DT lookup, I
> would expect having "clock-names" absent should just work with all
> those 3 clk_get calls to get <&clks 2>.
>
> usdhc@02190000 {
> ...
> clocks = <&clks 2>;
> };
>
> But with the current of_clk implementation, we will have to have
> something like below to keep the usdhc behavior same as non-DT lookup.
>
> usdhc@02190000 {
> ...
> clocks = <&clks 2>, <&clks 2>, <&clks 2>;
> clock-names = "ipg", "ahb", "per";
> };

This looks correct to me. The module always has 3 clock inputs, but some
chips may hook up the same clock to all 3 inputs. If you had different
versions of the module, then that would imply different compatible strings.

Which clock input is which should be defined by the order listed and the
names are supposed to be optional, but the current clk_get API doesn't
have any way to specify the index. So either you have to put in the
names or convert the driver to lookup by index. It's really no different
than irqs.

>
> Can we force all the clk_get calls with different con_id to get the
> only clk listed in "clocks" when "clock-names" is absent, so that we
> can somehow match the behavior with non-DT lookup?
>
> * phandle argument is not easy for engineering
>
> As we will have a whole bunch of clock outputs listed in ccm node,
> which will be referenced by peripheral phandle in form of <&clks index>.
> When the list gets long, it becomes hard for people to find the correct
> index number for the clock referenced.

As Stephen pointed out, you just have to document them until we have
defines in dts.

Rob

>
> Regards,
> Shawn
>
> [1] http://thread.gmane.org/gmane.linux.kernel/1300259/focus=1301107
>
> 8<---
> .../devicetree/bindings/clk/clk-imx6q.txt | 58 ++++++++++++++++++++
> arch/arm/boot/dts/imx6q.dtsi | 27 +++++++++-
> arch/arm/mach-imx/clk-imx6q.c | 31 ++++++-----
> 3 files changed, 101 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clk/clk-imx6q.txt
>
> diff --git a/Documentation/devicetree/bindings/clk/clk-imx6q.txt b/Documentation/devicetree/bindings/clk/clk-imx6q.txt
> new file mode 100644
> index 0000000..c5698a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clk/clk-imx6q.txt
> @@ -0,0 +1,58 @@
> +* Clock bindings for Freescale i.MX6Q
> +
> +Required properties:
> +- compatible: Should be "fsl,imx6q-ccm"
> +- reg: Address and length of the register set
> +- interrupts: Should contain CCM interrupt
> +- #clock-cells: Should be <1>
> +- clock-output-names: A list of clock output that CCM provides. The valid
> + clock names include:
> +
> + dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
> + pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m,
> + pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw,
> + periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel,
> + esai_sel, asrc_sel, spdif_sel, gpu2d_axi, gpu3d_axi, gpu2d_core_sel,
> + gpu3d_core_sel, gpu3d_shader_sel, ipu1_sel, ipu2_sel, ldb_di0_sel,
> + ldb_di1_sel, ipu1_di0_pre_sel, ipu1_di1_pre_sel, ipu2_di0_pre_sel,
> + ipu2_di1_pre_sel, ipu1_di0_sel, ipu1_di1_sel, ipu2_di0_sel,
> + ipu2_di1_sel, hsi_tx_sel, pcie_axi_sel, ssi1_sel, ssi2_sel, ssi3_sel,
> + usdhc1_sel, usdhc2_sel, usdhc3_sel, usdhc4_sel, enfc_sel, emi_sel,
> + emi_slow_sel, vdo_axi_sel, vpu_axi_sel, cko1_sel, periph, periph2,
> + periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf,
> + asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root,
> + gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf,
> + ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre,
> + ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf,
> + ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf,
> + usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf,
> + emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf,
> + mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial,
> + can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet,
> + esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb,
> + hdmi_isfr, i2c1, i2c2, i2c3, iim, enfc, ipu1, ipu1_di0, ipu1_di1, ipu2,
> + ipu2_di0, ldb_di0, ldb_di1, ipu2_di1, hsi_tx, mlb, mmdc_ch0_axi,
> + mmdc_ch1_axi, ocram, openvg_axi, pcie_axi, pwm1, pwm2, pwm3, pwm4,
> + gpmi_bch_apb, gpmi_bch, gpmi_io, gpmi_apb, sata, sdma, spba, ssi1,
> + ssi2, ssi3, uart_ipg, uart_serial, usboh3, usdhc1, usdhc2, usdhc3,
> + usdhc4, vdo_axi, vpu_axi, cko1, pll1_sys, pll2_bus, pll3_usb_otg,
> + pll4_audio, pll5_video, pll6_mlb, pll7_usb_host, pll8_enet, ssi1_ipg,
> + ssi2_ipg, ssi3_ipg, rom,
> +
> + But generally, only the clocks that peripheral nodes have a phandle pointing
> + to need to be listed there.
> +
> +Examples:
> +
> +clks: ccm@020c4000 {
> + compatible = "fsl,imx6q-ccm";
> + reg = <0x020c4000 0x4000>;
> + interrupts = <0 87 0x04 0 88 0x04>;
> + #clock-cells = <1>;
> + clock-output-names = "uart_ipg",
> + "uart_serial",
> + "usdhc1",
> + "usdhc2",
> + "usdhc3",
> + "usdhc4";
> +};
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 8c90cba..51b915835 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -169,6 +169,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x02020000 0x4000>;
> interrupts = <0 26 0x04>;
> + clocks = <&clks 0>, <&clks 1>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -348,10 +350,17 @@
> status = "disabled";
> };
>
> - ccm@020c4000 {
> + clks: ccm@020c4000 {
> compatible = "fsl,imx6q-ccm";
> reg = <0x020c4000 0x4000>;
> interrupts = <0 87 0x04 0 88 0x04>;
> + #clock-cells = <1>;
> + clock-output-names = "uart_ipg",
> + "uart_serial",
> + "usdhc1",
> + "usdhc2",
> + "usdhc3",
> + "usdhc4";
> };
>
> anatop@020c8000 {
> @@ -589,6 +598,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02190000 0x4000>;
> interrupts = <0 22 0x04>;
> + clocks = <&clks 2>, <&clks 2>, <&clks 2>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -596,6 +607,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02194000 0x4000>;
> interrupts = <0 23 0x04>;
> + clocks = <&clks 3>, <&clks 3>, <&clks 3>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -603,6 +616,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x02198000 0x4000>;
> interrupts = <0 24 0x04>;
> + clocks = <&clks 4>, <&clks 4>, <&clks 4>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -610,6 +625,8 @@
> compatible = "fsl,imx6q-usdhc";
> reg = <0x0219c000 0x4000>;
> interrupts = <0 25 0x04>;
> + clocks = <&clks 5>, <&clks 5>, <&clks 5>;
> + clock-names = "ipg", "ahb", "per";
> status = "disabled";
> };
>
> @@ -700,6 +717,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021e8000 0x4000>;
> interrupts = <0 27 0x04>;
> + clocks = <&clks 0>, <&clks 1>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -707,6 +726,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021ec000 0x4000>;
> interrupts = <0 28 0x04>;
> + clocks = <&clks 0>, <&clks 1>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -714,6 +735,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021f0000 0x4000>;
> interrupts = <0 29 0x04>;
> + clocks = <&clks 0>, <&clks 1>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
>
> @@ -721,6 +744,8 @@
> compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> reg = <0x021f4000 0x4000>;
> interrupts = <0 30 0x04>;
> + clocks = <&clks 0>, <&clks 1>;
> + clock-names = "ipg", "per";
> status = "disabled";
> };
> };
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index abb42e7..87b134e 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -57,6 +57,21 @@ static void __iomem *ccm_base;
>
> void __init imx6q_clock_map_io(void) { }
>
> +static struct clk *imx_clk_src_get(struct of_phandle_args *clkspec,
> + void *data)
> +{
> + const char *clk_name;
> + int idx = clkspec->args[0];
> + int ret;
> +
> + ret = of_property_read_string_index(clkspec->np, "clock-output-names",
> + idx, &clk_name);
> + if (ret < 0)
> + return ERR_PTR(ret);
> +
> + return __clk_lookup(clk_name);
> +}
> +
> int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
> {
> u32 val = readl_relaxed(ccm_base + CLPCR);
> @@ -391,21 +406,7 @@ int __init mx6q_clocks_init(void)
> clk_register_clkdev(clk[gpt_ipg], "ipg", "imx-gpt.0");
> clk_register_clkdev(clk[gpt_ipg_per], "per", "imx-gpt.0");
> clk_register_clkdev(clk[twd], NULL, "smp_twd");
> - clk_register_clkdev(clk[uart_serial], "per", "2020000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "2020000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21e8000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21e8000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21ec000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21ec000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21f0000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f0000.serial");
> - clk_register_clkdev(clk[uart_serial], "per", "21f4000.serial");
> - clk_register_clkdev(clk[uart_ipg], "ipg", "21f4000.serial");
> clk_register_clkdev(clk[enet], NULL, "2188000.ethernet");
> - clk_register_clkdev(clk[usdhc1], NULL, "2190000.usdhc");
> - clk_register_clkdev(clk[usdhc2], NULL, "2194000.usdhc");
> - clk_register_clkdev(clk[usdhc3], NULL, "2198000.usdhc");
> - clk_register_clkdev(clk[usdhc4], NULL, "219c000.usdhc");
> clk_register_clkdev(clk[i2c1], NULL, "21a0000.i2c");
> clk_register_clkdev(clk[i2c2], NULL, "21a4000.i2c");
> clk_register_clkdev(clk[i2c3], NULL, "21a8000.i2c");
> @@ -422,6 +423,8 @@ int __init mx6q_clocks_init(void)
> clk_register_clkdev(clk[ahb], "ahb", NULL);
> clk_register_clkdev(clk[cko1], "cko1", NULL);
>
> + of_clk_add_provider(np, imx_clk_src_get, NULL);
> +
> for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> clk_prepare_enable(clk[clks_init_on[i]]);
>

2012-06-21 07:27:21

by Chris Ball

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

Hi Rob,

On Tue, Jun 12 2012, Rob Herring wrote:
> This series defines clock bindings for Device-Tree and adds kernel
> support using the common clock infrastructure. The last patch enables
> DT clock support for the Calxeda Highbank platform.
>
> I'm posting this again to solicit further review. There has been some
> discussion[1], but no definite path forward. This series is not changed
> from the last post other than rebasing to v3.5-rc2.

This is a very useful patchset, thanks! Mitch Bradley and I have been
hooking up the mach-mmp ARM subarchitecture to it and we needed to
make a few changes, mainly because mach-mmp doesn't use COMMON_CLK.
The changes are:

1) Remove the COMMON_CLK dependency that the of_clk_* functions have by
moving them into a new file, drivers/clk/clk-of.c. The OF functions
in drivers/clk/clk.c aren't dependent on anything else in clk.c
(since you pass them a struct clk) so they can be moved out easily.
The of_clk_* entries in clk-provider.h move to clk.h.

2) Use alloc_bootmem() instead of kzalloc() in of_clk_add_provider(),
because we need to set up clocks during .init_early on ARM (which
happens pre-slab) so that they are available for platform init.

I'll send our patches in a reply to this mail. Are you okay with these
changes, and would you like to include them in your patchset?

Thanks very much,

- Chris.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2012-06-21 07:30:36

by Chris Ball

[permalink] [raw]
Subject: [PATCH 01/02] clk: Refactor of_clk_* into a new clk-of.c

This removes the dependency on CONFIG_COMMON_CLK, which is unnecessary
for the of_clk_* functions -- these functions are passed a struct clk,
which can either be a COMMON_CLK struct or an arch-specific one.

Signed-off-by: Chris Ball <[email protected]>
---
drivers/clk/Makefile | 2 +
drivers/clk/clk-of.c | 167 ++++++++++++++++++++++++++++++++++++++++++
drivers/clk/clk.c | 139 -----------------------------------
include/linux/clk-provider.h | 14 ----
include/linux/clk.h | 17 ++++-
5 files changed, 184 insertions(+), 155 deletions(-)
create mode 100644 drivers/clk/clk-of.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 553b30b..9584801 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -2,6 +2,8 @@
obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
obj-$(CONFIG_COMMON_CLK) += clk.o clk-fixed-rate.o clk-gate.o \
clk-mux.o clk-divider.o clk-fixed-factor.o
+obj-$(CONFIG_OF) += clk-of.o
+
# SoCs specific
obj-$(CONFIG_ARCH_MXS) += mxs/
obj-$(CONFIG_PLAT_SPEAR) += spear/
diff --git a/drivers/clk/clk-of.c b/drivers/clk/clk-of.c
new file mode 100644
index 0000000..8b43416
--- /dev/null
+++ b/drivers/clk/clk-of.c
@@ -0,0 +1,167 @@
+/*
+ * Copyright 2011-2012 Calxeda, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk-private.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+
+#ifdef CONFIG_OF
+
+struct of_device_id;
+
+typedef void (*of_clk_init_cb_t)(struct device_node *);
+/**
+ * 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);
+
+struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
+ void *data)
+{
+ return data;
+}
+EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
+
+/**
+ * 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;
+}
+EXPORT_SYMBOL_GPL(of_clk_add_provider);
+
+/**
+ * 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);
+}
+EXPORT_SYMBOL_GPL(of_clk_del_provider);
+
+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;
+}
+
+const char *of_clk_get_parent_name(struct device_node *np, int index)
+{
+ struct of_phandle_args clkspec;
+ const char *clk_name;
+ int rc;
+
+ if (index < 0)
+ return NULL;
+
+ rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
+ &clkspec);
+ if (rc)
+ return NULL;
+
+ if (of_property_read_string_index(clkspec.np, "clock-output-names",
+ clkspec.args_count ? clkspec.args[0] : 0,
+ &clk_name) < 0)
+ clk_name = clkspec.np->name;
+
+ of_node_put(clkspec.np);
+ return clk_name;
+}
+EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
+
+/**
+ * of_clk_init() - Scan and init clock providers from the DT
+ * @matches: array of compatible values and init functions for providers.
+ *
+ * This function scans the device tree for matching clock providers and
+ * calls their initialization functions
+ */
+void __init of_clk_init(const struct of_device_id *matches)
+{
+ struct device_node *np;
+
+ for_each_matching_node(np, matches) {
+ const struct of_device_id *match = of_match_node(matches, np);
+ of_clk_init_cb_t clk_init_cb = match->data;
+ clk_init_cb(np);
+ }
+}
+#endif
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 456f5fb..c773e5a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1545,142 +1545,3 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb)
return ret;
}
EXPORT_SYMBOL_GPL(clk_notifier_unregister);
-
-#ifdef CONFIG_OF
-/**
- * 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);
-
-struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
- void *data)
-{
- return data;
-}
-EXPORT_SYMBOL_GPL(of_clk_src_simple_get);
-
-/**
- * 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;
-}
-EXPORT_SYMBOL_GPL(of_clk_add_provider);
-
-/**
- * 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);
-}
-EXPORT_SYMBOL_GPL(of_clk_del_provider);
-
-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;
-}
-
-const char *of_clk_get_parent_name(struct device_node *np, int index)
-{
- struct of_phandle_args clkspec;
- const char *clk_name;
- int rc;
-
- if (index < 0)
- return NULL;
-
- rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
- &clkspec);
- if (rc)
- return NULL;
-
- if (of_property_read_string_index(clkspec.np, "clock-output-names",
- clkspec.args_count ? clkspec.args[0] : 0,
- &clk_name) < 0)
- clk_name = clkspec.np->name;
-
- of_node_put(clkspec.np);
- return clk_name;
-}
-EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
-
-/**
- * of_clk_init() - Scan and init clock providers from the DT
- * @matches: array of compatible values and init functions for providers.
- *
- * This function scans the device tree for matching clock providers and
- * calls their initialization functions
- */
-void __init of_clk_init(const struct of_device_id *matches)
-{
- struct device_node *np;
-
- for_each_matching_node(np, matches) {
- const struct of_device_id *match = of_match_node(matches, np);
- of_clk_init_cb_t clk_init_cb = match->data;
- clk_init_cb(np);
- }
-}
-#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b97f61e..2d5b1bf 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -336,19 +336,5 @@ void __clk_unprepare(struct clk *clk);
void __clk_reparent(struct clk *clk, struct clk *new_parent);
unsigned long __clk_round_rate(struct clk *clk, unsigned long rate);

-struct of_device_id;
-
-typedef void (*of_clk_init_cb_t)(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_src_simple_get(struct of_phandle_args *clkspec,
- void *data);
-const char *of_clk_get_parent_name(struct device_node *np, int index);
-void of_clk_init(const struct of_device_id *matches);
-
#endif /* CONFIG_COMMON_CLK */
#endif /* CLK_PROVIDER_H */
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 8b70342..d9293e7 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -312,8 +312,21 @@ int clk_add_alias(const char *alias, const char *alias_dev_name, char *id,

struct device_node;
struct of_phandle_args;
+struct of_device_id;

#ifdef CONFIG_OF
+typedef void (*of_clk_init_cb_t)(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_src_simple_get(struct of_phandle_args *clkspec,
+ void *data);
+const char *of_clk_get_parent_name(struct device_node *np, int index);
+void of_clk_init(const struct of_device_id *matches);
+
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);
struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec);
@@ -327,6 +340,6 @@ static inline struct clk *of_clk_get_by_name(struct device_node *np,
{
return NULL;
}
-#endif
+#endif /* CONFIG_OF */

-#endif
+#endif /* __LINUX_CLK_H */
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2012-06-21 07:32:14

by Chris Ball

[permalink] [raw]
Subject: [PATCH 02/02] clk: clk-of: Use alloc_bootmem() instead of kzalloc()

When of_clk_add_provider() is used at boot time (during .init_early on
ARM, in the motivating case for this patch), kzalloc() cannot be used
because slab isn't up yet.

Signed-off-by: Chris Ball <[email protected]>
---
drivers/clk/clk-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-of.c b/drivers/clk/clk-of.c
index 8b43416..adbb56b 100644
--- a/drivers/clk/clk-of.c
+++ b/drivers/clk/clk-of.c
@@ -22,6 +22,7 @@
#include <linux/list.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/bootmem.h>

#ifdef CONFIG_OF

@@ -67,7 +68,7 @@ int of_clk_add_provider(struct device_node *np,
{
struct of_clk_provider *cp;

- cp = kzalloc(sizeof(struct of_clk_provider), GFP_KERNEL);
+ cp = alloc_bootmem(sizeof(struct of_clk_provider));
if (!cp)
return -ENOMEM;

--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child

2012-06-21 12:19:31

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH 02/02] clk: clk-of: Use alloc_bootmem() instead of kzalloc()

On Thu, Jun 21, 2012 at 03:32:04AM -0400, Chris Ball wrote:
> When of_clk_add_provider() is used at boot time (during .init_early on
> ARM, in the motivating case for this patch), kzalloc() cannot be used
> because slab isn't up yet.
>
> Signed-off-by: Chris Ball <[email protected]>

I wouldn't use alloc_bootmem() unconditionally for this, as you have no
idea where other platforms may wire the call site up. slab is also
available a lot earlier now than it used to be, so many places that
required bootmem pages previously can get away with slab allocations now.

You could rework this as:

if (slab_is_available())
cp = kzalloc(...)
else
cp = alloc_bootmem(...)

and then it doesn't matter when and where it gets called.

2012-06-21 15:00:56

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

Chris,

On 06/21/2012 02:27 AM, Chris Ball wrote:
> Hi Rob,
>
> On Tue, Jun 12 2012, Rob Herring wrote:
>> This series defines clock bindings for Device-Tree and adds kernel
>> support using the common clock infrastructure. The last patch enables
>> DT clock support for the Calxeda Highbank platform.
>>
>> I'm posting this again to solicit further review. There has been some
>> discussion[1], but no definite path forward. This series is not changed
>> from the last post other than rebasing to v3.5-rc2.
>
> This is a very useful patchset, thanks! Mitch Bradley and I have been
> hooking up the mach-mmp ARM subarchitecture to it and we needed to
> make a few changes, mainly because mach-mmp doesn't use COMMON_CLK.
> The changes are:
>
> 1) Remove the COMMON_CLK dependency that the of_clk_* functions have by
> moving them into a new file, drivers/clk/clk-of.c. The OF functions
> in drivers/clk/clk.c aren't dependent on anything else in clk.c
> (since you pass them a struct clk) so they can be moved out easily.
> The of_clk_* entries in clk-provider.h move to clk.h.

Well, if you look at prior versions, it was separate being in
drivers/of/. I'm not sure we want to encourage/allow not converting to
COMMON_CLK. I would consider not using COMMON_CLK is deprecated. Having
a single definition of struct clk is the primary reason for COMMON_CLK.
What is the reason for not converting mmp to COMMON_CLK? It appears to
be a pretty trivial clock implementation, and will be required for
multi-platform kernels.

>
> 2) Use alloc_bootmem() instead of kzalloc() in of_clk_add_provider(),
> because we need to set up clocks during .init_early on ARM (which
> happens pre-slab) so that they are available for platform init.

This depends on 1 as the common clock code would have the same issue.
Generally, the first place clocks are needed is the timer init. At that
point, you can call kzalloc. This is where all the clock init used to be
done until init_early was added and some platforms have moved their
clock init. I don't think there was really ever much reason to move it
other than to make the timer init function only deal with timer setup.

Is alloc_bootmem available on all arches? There's 3 occurrences in
drivers/* which tells me that is the wrong thing to do.

Rob

> I'll send our patches in a reply to this mail. Are you okay with these
> changes, and would you like to include them in your patchset?
>
> Thanks very much,
>
> - Chris.

2012-06-21 17:54:56

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 20120621-10:00, Rob Herring wrote:
> On 06/21/2012 02:27 AM, Chris Ball wrote:
> >
> > 2) Use alloc_bootmem() instead of kzalloc() in of_clk_add_provider(),
> > because we need to set up clocks during .init_early on ARM (which
> > happens pre-slab) so that they are available for platform init.
>
> This depends on 1 as the common clock code would have the same issue.
> Generally, the first place clocks are needed is the timer init. At that
> point, you can call kzalloc. This is where all the clock init used to be
> done until init_early was added and some platforms have moved their
> clock init. I don't think there was really ever much reason to move it
> other than to make the timer init function only deal with timer setup.
>

Hi Rob,

Just FYI I've been looking at using alloc_bootmem in the common clk code
as a way to get rid of the static initialization stuff (which only
existed due to very early initialization of timers).

The suggested change above to of_clk_add_provider would jive well with
my change to the common clk code.

Regards,
Mike

2012-06-27 12:54:19

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] DT clock bindings

On 06/21/2012 12:54 PM, Mike Turquette wrote:
> On 20120621-10:00, Rob Herring wrote:
>> On 06/21/2012 02:27 AM, Chris Ball wrote:
>>>
>>> 2) Use alloc_bootmem() instead of kzalloc() in of_clk_add_provider(),
>>> because we need to set up clocks during .init_early on ARM (which
>>> happens pre-slab) so that they are available for platform init.
>>
>> This depends on 1 as the common clock code would have the same issue.
>> Generally, the first place clocks are needed is the timer init. At that
>> point, you can call kzalloc. This is where all the clock init used to be
>> done until init_early was added and some platforms have moved their
>> clock init. I don't think there was really ever much reason to move it
>> other than to make the timer init function only deal with timer setup.
>>
>
> Hi Rob,
>
> Just FYI I've been looking at using alloc_bootmem in the common clk code
> as a way to get rid of the static initialization stuff (which only
> existed due to very early initialization of timers).

The slab is up at the time timers are initialized. The only real problem
is mixing clock init into the timer init functions and clk init in
init_early is cleaner in that regard.

> The suggested change above to of_clk_add_provider would jive well with
> my change to the common clk code.

Are you planning this for 3.6? If not, then this can be addressed at the
time the clk framework supports bootmem.

I'm not so sure more users of bootmem are desired. There seems to be
some effort/desire to remove it:

http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-June/000562.html
https://lkml.org/lkml/2012/3/13/586
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-December/074886.html

Rob