[PATCH 0/7] Highbank clock support using DT

[PATCH 0/7] Highbank clock support using DT

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

This series adds clock support to highbank using common clock
infrastructure and DT clock bindings. This is based on DT clock
support cherry-picked from Grant's devicetree/test-clocks branch
and Mike T's v6 common clock infrastructure series.

It supports initialization of the clocks in any order at least in my
relatively simple case of 1 clock output per DT node. 

Rob

Grant Likely (3):
  of: add clock providers
  of: Add of_property_match_string() to find index into a string list
  dt/clock: Add handling for fixed clocks and a clock node setup
    iterator

Rob Herring (4):
  clk: fix orphan list iterator to be safe
  dt/clock: add a simple provider get function
  dt/clock: add function to get parent clock name
  clk: add highbank clock support

 .../devicetree/bindings/clock/clock-bindings.txt   |  116 +++++++
 .../devicetree/bindings/clock/fixed-clock.txt      |   21 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/highbank.dts                     |   76 +++++
 arch/arm/boot/dts/testcases/tests-phandle.dtsi     |    2 +
 arch/arm/mach-highbank/Makefile                    |    2 +-
 arch/arm/mach-highbank/clock.c                     |   62 ----
 arch/arm/mach-highbank/highbank.c                  |   16 +
 drivers/clk/Makefile                               |    2 +
 drivers/clk/clk-highbank.c                         |  335 ++++++++++++++++++++
 drivers/clk/clk.c                                  |    4 +-
 drivers/clk/clkdev.c                               |    9 +
 drivers/of/Kconfig                                 |    6 +
 drivers/of/Makefile                                |    1 +
 drivers/of/base.c                                  |   36 ++
 drivers/of/clock.c                                 |  236 ++++++++++++++
 drivers/of/selftest.c                              |   29 ++
 include/linux/of.h                                 |    3 +
 include/linux/of_clk.h                             |   43 +++
 19 files changed, 935 insertions(+), 65 deletions(-)
 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
 create mode 100644 drivers/of/clock.c
 create mode 100644 include/linux/of_clk.h

-- 
1.7.5.4

[PATCH 1/7] clk: fix orphan list iterator to be safe

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

__clk_reparent can remove orphans, so the list iterator needs to be the
safe from removal version.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/clk/clk.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c7c3bc5..802eda4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1172,7 +1172,7 @@ void __clk_init(struct device *dev, struct clk *clk)
 {
 	int i;
 	struct clk *orphan;
-	struct hlist_node *tmp;
+	struct hlist_node *tmp, *tmp2;
 
 	if (!clk)
 		return;
@@ -1246,7 +1246,7 @@ void __clk_init(struct device *dev, struct clk *clk)
 	 * walk the list of orphan clocks and reparent any that are children of
 	 * this clock
 	 */
-	hlist_for_each_entry(orphan, tmp, &clk_orphan_list, child_node)
+	hlist_for_each_entry_safe(orphan, tmp, tmp2, &clk_orphan_list, child_node)
 		__clk_reparent(orphan, __clk_init_parent(orphan));
 
 	/*
-- 
1.7.5.4

Re: [PATCH 1/7] clk: fix orphan list iterator to be safe

[Turquette, Mike]

On Tue, Mar 13, 2012 at 4:22 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> __clk_reparent can remove orphans, so the list iterator needs to be the
> safe from removal version.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>

Thanks Rob.  I've taken this in for the next series.

Regards,
Mike

> ---
>  drivers/clk/clk.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c7c3bc5..802eda4 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1172,7 +1172,7 @@ void __clk_init(struct device *dev, struct clk *clk)
>  {
>        int i;
>        struct clk *orphan;
> -       struct hlist_node *tmp;
> +       struct hlist_node *tmp, *tmp2;
>
>        if (!clk)
>                return;
> @@ -1246,7 +1246,7 @@ void __clk_init(struct device *dev, struct clk *clk)
>         * walk the list of orphan clocks and reparent any that are children of
>         * this clock
>         */
> -       hlist_for_each_entry(orphan, tmp, &clk_orphan_list, child_node)
> +       hlist_for_each_entry_safe(orphan, tmp, tmp2, &clk_orphan_list, child_node)
>                __clk_reparent(orphan, __clk_init_parent(orphan));
>
>        /*
> --
> 1.7.5.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/7] of: add clock providers

[Rob Herring]

From: Grant Likely <grant.likely@secretlab.ca>

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.

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 <grant.likely@secretlab.ca>
Reviewed-by: Shawn Guo <shawn.guo@freescale.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Mike Turquette <mturquette@ti.com>
---
 .../devicetree/bindings/clock/clock-bindings.txt   |  116 ++++++++++++++
 .../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, 355 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..3cb6746
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -0,0 +1,116 @@
+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.  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..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

Re: [PATCH 2/7] of: add clock providers

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1381 bytes --]

* Rob Herring wrote:
> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
[...]
> +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

"clock-output-names"?

> +		    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.

Ditto.

[...]
> 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 document references itself.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/7] of: add clock providers

[Shawn Guo]

On Tue, Mar 13, 2012 at 06:22:22PM -0500, Rob Herring wrote:
> From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> 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.
> 
> 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 <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   |  116 ++++++++++++++
>  .../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, 355 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..3cb6746
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -0,0 +1,116 @@
> +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
> +

We had a discussion[1] regarding it.

Regards,
Shawn

http://article.gmane.org/gmane.linux.drivers.devicetree/11554

Re: [PATCH 2/7] of: add clock providers

[Grant Likely]

On Tue, 13 Mar 2012 18:22:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> 
> 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.
> 
> 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 <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>

Hi Rob,

Thanks for respinning this patch.  Since you're actually using it, do
you want to take over getting it into mainline?

> ---
>  .../devicetree/bindings/clock/clock-bindings.txt   |  116 ++++++++++++++
>  .../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 ++++++++++++++++++++

I would actually like to see this file moved into drivers/clk.  I
don't think there is any need anymore to collect OF support code into
drivers/of.  I plan to move the spi and gpio support code into
drivers/spi and drivers/gpio respectively.

g.


>  include/linux/of_clk.h                             |   37 +++++
>  7 files changed, 355 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..3cb6746
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -0,0 +1,116 @@
> +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.  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..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
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

Re: [PATCH 2/7] of: add clock providers

[Rob Herring]

On 04/06/2012 11:18 PM, Grant Likely wrote:
> On Tue, 13 Mar 2012 18:22:22 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>>
>> 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.
>>
>> 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 <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>
> 
> Hi Rob,
> 
> Thanks for respinning this patch.  Since you're actually using it, do
> you want to take over getting it into mainline?
> 

Yes. Is what you have on your public tree the latest?

>> ---
>>  .../devicetree/bindings/clock/clock-bindings.txt   |  116 ++++++++++++++
>>  .../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 ++++++++++++++++++++
> 
> I would actually like to see this file moved into drivers/clk.  I
> don't think there is any need anymore to collect OF support code into
> drivers/of.  I plan to move the spi and gpio support code into
> drivers/spi and drivers/gpio respectively.
> 

You keep saying that and if you recall, the i2c maintainers objected to
doing that. I'd imagine you'll find the spi and gpio maintainer more
agreeable. ;)

I'll have to think about how to split it as much of it is really clkdev
code.

Rob

Re: [PATCH 2/7] of: add clock providers

[Shawn Guo]

On Tue, Mar 13, 2012 at 06:22:22PM -0500, Rob Herring wrote:
...
> +==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.

Per discussion[1], the parent of the clock could be reasonably
represented in C code instead of in device tree.  Then how can
this "clocks" property be "Required"?  Or to put it another way,
if this is "Required", the whole clock tree will have to be represented
in device tree, no?

> +
> +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";
> +    };

Regards,
Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

Re: [PATCH 2/7] of: add clock providers

[Rob Herring]

On 04/09/2012 06:55 AM, Shawn Guo wrote:
> On Tue, Mar 13, 2012 at 06:22:22PM -0500, Rob Herring wrote:
> ...
>> +==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.
> 
> Per discussion[1], the parent of the clock could be reasonably
> represented in C code instead of in device tree.  Then how can
> this "clocks" property be "Required"?  Or to put it another way,
> if this is "Required", the whole clock tree will have to be represented
> in device tree, no?
> 

I'm not sure I follow. All of this only applies if you are doing clock
setup from DT. Whether you represent all clocks or only the outputs to
devices in DT is up to you. So you could have a CCM node for imx with 50
clock outputs and all the intermediate clocks within the CCM are
represented with C code.

In other words, at minimum you are just replacing the clkdev lookup with
a DT lookup.

Rob

>> +
>> +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";
>> +    };
> 
> Regards,
> Shawn
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/139414/focus=1216423

Re: [PATCH 2/7] of: add clock providers

[Shawn Guo]

On Mon, Apr 09, 2012 at 08:52:35AM -0500, Rob Herring wrote:
> On 04/09/2012 06:55 AM, Shawn Guo wrote:
> > On Tue, Mar 13, 2012 at 06:22:22PM -0500, Rob Herring wrote:
> > ...
> >> +==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.
> > 
> > Per discussion[1], the parent of the clock could be reasonably
> > represented in C code instead of in device tree.  Then how can
> > this "clocks" property be "Required"?  Or to put it another way,
> > if this is "Required", the whole clock tree will have to be represented
> > in device tree, no?
> > 
> 
> I'm not sure I follow. All of this only applies if you are doing clock
> setup from DT. Whether you represent all clocks or only the outputs to
> devices in DT is up to you. So you could have a CCM node for imx with 50
> clock outputs and all the intermediate clocks within the CCM are
> represented with C code.
> 
Ok, now I have uart clock being one of the 50 clocks represented in
device tree, while the parent of uart clock ipg_per is represented in
C code.  That said, I do not have a ipg_per clock node in device tree.
My question is how I should give that required "clocks" property for
the uart clock node.

	uart_serial_clk: uart-baud {
		compatible = "fsl,imx6q-clock";
		#clock-cells = <0>;
		/* clocks = <&ipg_per_clk>; */
		clocks = <???>;
	};

	uart: serial@02020000 {
		compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
		reg = <0x02020000 0x4000>;
		interrupts = <0 26 0x04>;
		clocks = <&uart_serial_clk>
	}

Regards,
Shawn

> In other words, at minimum you are just replacing the clkdev lookup with
> a DT lookup.
> 

Re: [PATCH 2/7] of: add clock providers

[Rob Herring]

On 04/09/2012 09:13 AM, Shawn Guo wrote:
> On Mon, Apr 09, 2012 at 08:52:35AM -0500, Rob Herring wrote:
>> On 04/09/2012 06:55 AM, Shawn Guo wrote:
>>> On Tue, Mar 13, 2012 at 06:22:22PM -0500, Rob Herring wrote:
>>> ...
>>>> +==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.
>>>
>>> Per discussion[1], the parent of the clock could be reasonably
>>> represented in C code instead of in device tree.  Then how can
>>> this "clocks" property be "Required"?  Or to put it another way,
>>> if this is "Required", the whole clock tree will have to be represented
>>> in device tree, no?
>>>
>>
>> I'm not sure I follow. All of this only applies if you are doing clock
>> setup from DT. Whether you represent all clocks or only the outputs to
>> devices in DT is up to you. So you could have a CCM node for imx with 50
>> clock outputs and all the intermediate clocks within the CCM are
>> represented with C code.
>>
> Ok, now I have uart clock being one of the 50 clocks represented in
> device tree, while the parent of uart clock ipg_per is represented in
> C code.  That said, I do not have a ipg_per clock node in device tree.
> My question is how I should give that required "clocks" property for
> the uart clock node.
>
> 	uart_serial_clk: uart-baud {
> 		compatible = "fsl,imx6q-clock";
> 		#clock-cells = <0>;
> 		/* clocks = <&ipg_per_clk>; */
> 		clocks = <???>;

This is a provider node not a consumer, so you don't have a clocks
property. You could, but then you need the parent clock defined in the DT.

The compatible property here looks too generic. In the CCM code, how do
you match it to the uart baud clock? You shouldn't be using the name.

You need to first decide if you are going to define each clock in the DT
or just the whole CCM as a node. You can't really do something in the
middle or be evolving it over time. You could perhaps make the PLLs be
separate nodes and inputs to the CCM. As they really are separate h/w
blocks.

Rob

> 	};
> 
> 	uart: serial@02020000 {
> 		compatible = "fsl,imx6q-uart", "fsl,imx21-uart";
> 		reg = <0x02020000 0x4000>;
> 		interrupts = <0 26 0x04>;
> 		clocks = <&uart_serial_clk>
> 	}
> 
> Regards,
> Shawn
> 
>> In other words, at minimum you are just replacing the clkdev lookup with
>> a DT lookup.
>>

Re: [PATCH 2/7] of: add clock providers

[Shawn Guo]

On Mon, Apr 09, 2012 at 09:34:46AM -0500, Rob Herring wrote:
> On 04/09/2012 09:13 AM, Shawn Guo wrote:
...
> >
> > 	uart_serial_clk: uart-baud {
> > 		compatible = "fsl,imx6q-clock";
> > 		#clock-cells = <0>;
> > 		/* clocks = <&ipg_per_clk>; */
> > 		clocks = <???>;
> 
> This is a provider node not a consumer, so you don't have a clocks
> property.

To me, any clock except root is not only a provider but also a consumer.
If "clocks" property is required for a consumer ...

> You could, but then you need the parent clock defined in the DT.
> 
..., then we need to define the entire clock tree in the DT.

> The compatible property here looks too generic. In the CCM code, how do
> you match it to the uart baud clock? You shouldn't be using the name.
> 
The code was copied from my early implementation which gets the entire
clock tree and every clock (reg, bits, shift) represented in DT, and I
do not need to map it to CCM code at all.

> You need to first decide if you are going to define each clock in the DT

That was my preference, but it brings too many clock nodes into DT.

> or just the whole CCM as a node.
> You can't really do something in the
> middle or be evolving it over time. You could perhaps make the PLLs be
> separate nodes and inputs to the CCM. As they really are separate h/w
> blocks.

I do not understand this part.

-- 
Regards,
Shawn

[PATCH 3/7] of: Add of_property_match_string() to find index into a string list

[Rob Herring]

From: Grant Likely <grant.likely@secretlab.ca>

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@secretlab.ca>
---
 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 133908a..13ba728 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 a75a831..5a4a3ad 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -217,6 +217,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

Re: [PATCH 3/7] of: Add of_property_match_string() to find index into a string list

[Grant Likely]

On Tue, 13 Mar 2012 18:22:23 -0500, Rob Herring <robherring2@gmail.com> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> 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@secretlab.ca>

BTW, this patch got merged for v3.4 so you'll be able to drop it in
the next posting.

g.

[PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Rob Herring]

From: Grant Likely <grant.likely@secretlab.ca>

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
[Rob Herring] Rework to use common clock infrastructure
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/clock.c     |   45 ++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/of_clk.h |    4 ++++
 2 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/drivers/of/clock.c b/drivers/of/clock.c
index 6519e96..06fd5ad 100644
--- a/drivers/of/clock.c
+++ b/drivers/of/clock.c
@@ -2,7 +2,8 @@
  * Clock infrastructure for device tree platforms
  */
 
-#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/module.h>
@@ -163,3 +164,45 @@ struct clk *of_clk_get_by_name(struct device_node *np, const char *name)
 
 	return clk;
 }
+
+/**
+ * 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);
+	}
+}
+
+static struct clk *of_fixed_clk_get(struct of_phandle_args *a, void *data)
+{
+	return data;
+}
+
+/**
+ * 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_fixed_clk_get, clk);
+}
diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h
index dcbd27b..02ecc98 100644
--- a/include/linux/of_clk.h
+++ b/include/linux/of_clk.h
@@ -10,6 +10,7 @@ struct clk;
 #ifdef CONFIG_OF_CLOCK
 
 struct device_node;
+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,
@@ -21,6 +22,9 @@ 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);
 
+void of_clk_init(const struct of_device_id *matches);
+extern void of_fixed_clk_setup(struct device_node *np);
+
 #else
 static struct clk *of_clk_get(struct device_node *np, int index)
 {
-- 
1.7.5.4

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Shawn Guo]

On Tue, Mar 13, 2012 at 06:22:24PM -0500, Rob Herring wrote:
...
> +/**
> + * 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_fixed_clk_get, clk);
> +}

It seems my comment[1] did not get addressed in this post.

Regards,
Shawn

[1] http://article.gmane.org/gmane.linux.drivers.devicetree/10589

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Rob Herring]

On 03/14/2012 02:59 AM, Shawn Guo wrote:
> On Tue, Mar 13, 2012 at 06:22:24PM -0500, Rob Herring wrote:
> ...
>> +/**
>> + * 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_fixed_clk_get, clk);
>> +}
> 
> It seems my comment[1] did not get addressed in this post.
> 
> Regards,
> Shawn
> 
> [1] http://article.gmane.org/gmane.linux.drivers.devicetree/10589

I just took Grant's commit as is.

It also only works for root fixed clocks, but the common clock allows
for non-root fixed clocks. I don't think there's much use case for that
at least as a DT clock node.

Rob

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Shawn Guo]

On Wed, Mar 14, 2012 at 08:26:17AM -0500, Rob Herring wrote:
...
> It also only works for root fixed clocks, but the common clock allows
> for non-root fixed clocks. I don't think there's much use case for that
> at least as a DT clock node.
> 
What my example demonstrates are root fixed clocks.

-- 
Regards,
Shawn

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Rob Herring]

Shawn,

On 03/14/2012 02:59 AM, Shawn Guo wrote:
> On Tue, Mar 13, 2012 at 06:22:24PM -0500, Rob Herring wrote:
> ...
>> +/**
>> + * 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_fixed_clk_get, clk);
>> +}
> 
> It seems my comment[1] did not get addressed in this post.
> 
> Regards,
> Shawn
> 
> [1] http://article.gmane.org/gmane.linux.drivers.devicetree/10589

So I started implementing support for multiple outputs, but ran into a
complication. If you have multiple clocks on a node, then you have to
have a clk_src_get function to translate the clock cell value to a
struct clk. This would also require allocating an array of struct clk's
to do the lookup. This is all doable, but I don't see the benefit over
having a single node per fixed clock. We're not likely to have *lots* of
fixed clocks. I think we should leave fixed-clock defined as a single
output. If you really see the benefit, you can add a new binding
"multiple-fixed-clocks" or something platform specific.

Rob

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Shawn Guo]

On Sun, Apr 08, 2012 at 09:48:27AM -0500, Rob Herring wrote:
...
> So I started implementing support for multiple outputs, but ran into a
> complication. If you have multiple clocks on a node, then you have to
> have a clk_src_get function to translate the clock cell value to a
> struct clk.

So this is how clk_src_get looks like.

struct clk *clk_src_get(struct of_phandle_args *a, void *data)
{
        struct clk **clks = data;
        return clks[a->args[0]];
}

> This would also require allocating an array of struct clk's
> to do the lookup.

And this how allocating looks like.

        clks = kzalloc(sizeof(*clks)  * num, GFP_KERNEL);
        if (!clks)
                return -ENOMEM;

Seriously, not a big complication, right?

> This is all doable, but I don't see the benefit over
> having a single node per fixed clock. We're not likely to have *lots* of
> fixed clocks. I think we should leave fixed-clock defined as a single
> output.

The problem is there is nothing specific to fixed-clock.  If you have
some reason to not support blob node for fixed-clock, the reason will
apply on clk_gate, clk_divider and clk_mux too.  Then, which clocks
will support the #clock-cells in the binding document?

So to me, you need to either have it implemented or remove it from the
binding document completely.

Regards,
Shawn

> If you really see the benefit, you can add a new binding
> "multiple-fixed-clocks" or something platform specific.
> 

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Rob Herring]

On 04/09/2012 03:49 AM, Shawn Guo wrote:
> On Sun, Apr 08, 2012 at 09:48:27AM -0500, Rob Herring wrote:
> ...
>> So I started implementing support for multiple outputs, but ran into a
>> complication. If you have multiple clocks on a node, then you have to
>> have a clk_src_get function to translate the clock cell value to a
>> struct clk.
> 
> So this is how clk_src_get looks like.
> 
> struct clk *clk_src_get(struct of_phandle_args *a, void *data)
> {
>         struct clk **clks = data;
>         return clks[a->args[0]];
> }
> 
>> This would also require allocating an array of struct clk's
>> to do the lookup.
> 
> And this how allocating looks like.
> 
>         clks = kzalloc(sizeof(*clks)  * num, GFP_KERNEL);
>         if (!clks)
>                 return -ENOMEM;
> 
> Seriously, not a big complication, right?

I didn't say it can't be done. I'm saying I don't see the benefit to
supporting it vs. the added code to iterate over the array, handle
errors in the middle, and added matching function.

You still haven't given any benefits of supporting multiple clocks. It's
slightly fewer dts lines, but not really anything else.

> 
>> This is all doable, but I don't see the benefit over
>> having a single node per fixed clock. We're not likely to have *lots* of
>> fixed clocks. I think we should leave fixed-clock defined as a single
>> output.
> 
> The problem is there is nothing specific to fixed-clock.  If you have
> some reason to not support blob node for fixed-clock, the reason will
> apply on clk_gate, clk_divider and clk_mux too.  Then, which clocks
> will support the #clock-cells in the binding document?
> 

But fixed-clock is a specific binding and each binding can define
#clock-cells however they want. New bindings can define whatever they want.

If you did a clk mux binding, you're not going to have multiple lists of
mux selections for multiple outputs, so we're going to effectively
limited there to 1 output as well.

I don't think people are going to define clocks generically in DT at the
mux, clk gate and divider level anyway. If you only have a few clocks
then you may, and 1 clock output per node is probably okay. If you have
hundreds of clocks then you probably won't, and will have a SOC specific
binding anyway.

Rob

> So to me, you need to either have it implemented or remove it from the
> binding document completely.
> 
> Regards,
> Shawn
> 
>> If you really see the benefit, you can add a new binding
>> "multiple-fixed-clocks" or something platform specific.
>>

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Shawn Guo]

On Mon, Apr 09, 2012 at 09:18:27AM -0500, Rob Herring wrote:
...
> You still haven't given any benefits of supporting multiple clocks. It's
> slightly fewer dts lines, but not really anything else.
> 
What's more important than fewer dts lines is fewer nodes.  Isn't it
the whole point of #clock-cells?  In the real imx example I gave, with
#clock-cells support, I can have only one node to represent 3 fixed
clocks.

...
> I don't think people are going to define clocks generically in DT at the
> mux, clk gate and divider level anyway. If you only have a few clocks
> then you may, and 1 clock output per node is probably okay. If you have
> hundreds of clocks then you probably won't, and will have a SOC specific
> binding anyway.
> 
Why?

Take a look at clk-imx6q.c[1], you will find except pll and pfd, all
the imx6q clocks are represented as gate, divider and mux, and we
should not need a SoC specific binding for them.

-- 
Regards,
Shawn

[1] http://git.pengutronix.de/?p=imx/linux-2.6.git;a=blob;f=arch/arm/mach-imx/clk-imx6q.c;h=d5aaced3496439653238f18a399c466fc7264f4e;hb=1a9cf33bf1df7336b75235ae22fde83c6341c38e

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Rob Herring]

On 04/09/2012 06:27 PM, Shawn Guo wrote:
> On Mon, Apr 09, 2012 at 09:18:27AM -0500, Rob Herring wrote:
> ...
>> You still haven't given any benefits of supporting multiple clocks. It's
>> slightly fewer dts lines, but not really anything else.
>>
> What's more important than fewer dts lines is fewer nodes.  Isn't it
> the whole point of #clock-cells?  In the real imx example I gave, with
> #clock-cells support, I can have only one node to represent 3 fixed
> clocks.

So you want to have generic bindings for every clock which implies a
bunch of nodes if not a node per clock anyway, but then object to 3
nodes for fixed clocks.

I don't think I've ever seen a chip with more than 3 clock inputs
anyway. There are cases with additional clocks like a CMOS sensor or
audio chip which provides a fixed input clock, but those clocks should
reside with the nodes that generate them.

> ...
>> I don't think people are going to define clocks generically in DT at the
>> mux, clk gate and divider level anyway. If you only have a few clocks
>> then you may, and 1 clock output per node is probably okay. If you have
>> hundreds of clocks then you probably won't, and will have a SOC specific
>> binding anyway.
>>
> Why?
> 
> Take a look at clk-imx6q.c[1], you will find except pll and pfd, all
> the imx6q clocks are represented as gate, divider and mux, and we
> should not need a SoC specific binding for them.
> 

I'm talking about the bindings and you are pointing me to an
implementation with no bindings. It's 2 different things. The
implementation can fully be the generic clk implementations, but the
clock bindings can still be either a node per clock or a monolithic
clock module node. There is no fixed rule here and there should not be.
We can each chose what works best for us.

I see a couple of examples that your clocks are still SoC specific
despite your claims.

Your clock gates may reuse clk_gate struct, but they still have custom
ops to handle the 2-bit field. Obviously, you decided not to merge 2-bit
field support into the existing clk gate code (the correct decision
IMHO). I made a similar decision.

For practically every clock, you need to set the spinlock to
imx_ccm_lock. How are you going to know which clocks to set this lock
for with a "generic" binding? You have to distinguish those from IPU
clocks for example.

Rob

Re: [PATCH 4/7] dt/clock: Add handling for fixed clocks and a clock node setup iterator

[Shawn Guo]

On Sat, Apr 14, 2012 at 10:04:26PM -0500, Rob Herring wrote:
...
> So you want to have generic bindings for every clock which implies a
> bunch of nodes if not a node per clock anyway, but then object to 3
> nodes for fixed clocks.
> 
I do not really care about that now, as long as you clearly document
in fixed clock binding that there is no multiple outputs support for
fixed-clock, and #clock-cells has to be 0.

...
> I'm talking about the bindings and you are pointing me to an
> implementation with no bindings. It's 2 different things. The
> implementation can fully be the generic clk implementations, but the
> clock bindings can still be either a node per clock or a monolithic
> clock module node. There is no fixed rule here and there should not be.
> We can each chose what works best for us.
> 
It will be a pity if we can not have a generic binding for a generic
implementation.

> I see a couple of examples that your clocks are still SoC specific
> despite your claims.
> 
> Your clock gates may reuse clk_gate struct, but they still have custom
> ops to handle the 2-bit field. Obviously, you decided not to merge 2-bit
> field support into the existing clk gate code (the correct decision
> IMHO). I made a similar decision.
> 
Actually, Sascha decided.

> For practically every clock, you need to set the spinlock to
> imx_ccm_lock. How are you going to know which clocks to set this lock
> for with a "generic" binding? You have to distinguish those from IPU
> clocks for example.
> 
Something like this:

void __init of_clk_divider_setup(struct device_node *node, spinlock_t *lock)
{
        [ parse clock-divider properties from node ]

        clk = clk_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT,
                        reg, shift, width, 0, &lock);
        if (clk)
                of_clk_add_provider(node, NULL, clk);
}

void __init imx_clk_divider_init(struct device_node *node)
{
        return of_clk_divider_setup(node, &imx_ccm_lock);
}

static const struct of_device_id imx_clk_match[] __initconst = {
        { .compatible = "fsl,clock-divider", .data = imx_clk_divider_init, },
        {}
};

void __init imx_clocks_init(void)
{
        of_clk_init(imx_clk_match);
}

When I say generic binding, it does not necessarily mean the generic
compatible string, but the property definition and generic DT setup
code for them, something similar as generic regulator binding
Documentation/devicetree/bindings/regulator/regulator.txt and the
setup/parse function of_get_regulator_init_data.

Honestly, I'm losing my interest on clock binding.  I will stay with
clk_lookup before I see a solution to how we represent part of the
clock tree in DT while their input/parent clocks in the clock code.

-- 
Regards,
Shawn

[PATCH 5/7] dt/clock: add a simple provider get function

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

For simple cases, the clock provider data can simply be the struct clk ptr.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/clock.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/of/clock.c b/drivers/of/clock.c
index 06fd5ad..f4a0965 100644
--- a/drivers/of/clock.c
+++ b/drivers/of/clock.c
@@ -33,6 +33,12 @@ struct of_clk_provider {
 static LIST_HEAD(of_clk_providers);
 static DEFINE_MUTEX(of_clk_lock);
 
+static struct clk *of_clk_simple_get(struct of_phandle_args *clkspec,
+				     void *data)
+{
+	return data;
+}
+
 /**
  * of_clk_add_provider() - Register a clock provider for a node
  * @np: Device node pointer associated with clock provider
@@ -52,7 +58,10 @@ int of_clk_add_provider(struct device_node *np,
 
 	cp->node = of_node_get(np);
 	cp->data = data;
-	cp->get = clk_src_get;
+	if (clk_src_get)
+		cp->get = clk_src_get;
+	else
+		cp->get = of_clk_simple_get;
 
 	mutex_lock(&of_clk_lock);
 	list_add(&cp->link, &of_clk_providers);
@@ -183,11 +192,6 @@ void __init of_clk_init(const struct of_device_id *matches)
 	}
 }
 
-static struct clk *of_fixed_clk_get(struct of_phandle_args *a, void *data)
-{
-	return data;
-}
-
 /**
  * of_fixed_clk_setup() - Setup function for simple fixed rate clock
  */
@@ -204,5 +208,5 @@ void __init of_fixed_clk_setup(struct device_node *node)
 
 	clk = clk_register_fixed_rate(NULL, clk_name, NULL, CLK_IS_ROOT, rate);
 	if (clk)
-		of_clk_add_provider(node, of_fixed_clk_get, clk);
+		of_clk_add_provider(node, NULL, clk);
 }
-- 
1.7.5.4

Re: [PATCH 5/7] dt/clock: add a simple provider get function

[Grant Likely]

On Tue, 13 Mar 2012 18:22:25 -0500, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> For simple cases, the clock provider data can simply be the struct clk ptr.
> 
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/of/clock.c |   18 +++++++++++-------
>  1 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/of/clock.c b/drivers/of/clock.c
> index 06fd5ad..f4a0965 100644
> --- a/drivers/of/clock.c
> +++ b/drivers/of/clock.c
> @@ -33,6 +33,12 @@ struct of_clk_provider {
>  static LIST_HEAD(of_clk_providers);
>  static DEFINE_MUTEX(of_clk_lock);
>  
> +static struct clk *of_clk_simple_get(struct of_phandle_args *clkspec,
> +				     void *data)
> +{
> +	return data;
> +}
> +
>  /**
>   * of_clk_add_provider() - Register a clock provider for a node
>   * @np: Device node pointer associated with clock provider
> @@ -52,7 +58,10 @@ int of_clk_add_provider(struct device_node *np,
>  
>  	cp->node = of_node_get(np);
>  	cp->data = data;
> -	cp->get = clk_src_get;
> +	if (clk_src_get)
> +		cp->get = clk_src_get;
> +	else
> +		cp->get = of_clk_simple_get;

Personally, I would prefer callers to be explicit about the
translation functions they need, but it isn't a big enough concern for
me to make a big deal about it.

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

>  
>  	mutex_lock(&of_clk_lock);
>  	list_add(&cp->link, &of_clk_providers);
> @@ -183,11 +192,6 @@ void __init of_clk_init(const struct of_device_id *matches)
>  	}
>  }
>  
> -static struct clk *of_fixed_clk_get(struct of_phandle_args *a, void *data)
> -{
> -	return data;
> -}
> -
>  /**
>   * of_fixed_clk_setup() - Setup function for simple fixed rate clock
>   */
> @@ -204,5 +208,5 @@ void __init of_fixed_clk_setup(struct device_node *node)
>  
>  	clk = clk_register_fixed_rate(NULL, clk_name, NULL, CLK_IS_ROOT, rate);
>  	if (clk)
> -		of_clk_add_provider(node, of_fixed_clk_get, clk);
> +		of_clk_add_provider(node, NULL, clk);
>  }
> -- 
> 1.7.5.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

[PATCH 6/7] dt/clock: add function to get parent clock name

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 drivers/of/clock.c     |   24 ++++++++++++++++++++++++
 include/linux/of_clk.h |    2 ++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/of/clock.c b/drivers/of/clock.c
index f4a0965..8ad2dbd 100644
--- a/drivers/of/clock.c
+++ b/drivers/of/clock.c
@@ -128,6 +128,30 @@ struct clk *of_clk_get(struct device_node *np, int index)
 	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;
+}
+
+
 /**
  * of_clk_get_by_name() - Parse and lookup a clock referenced by a device node
  * @np: pointer to clock consumer node
diff --git a/include/linux/of_clk.h b/include/linux/of_clk.h
index 02ecc98..6a07071 100644
--- a/include/linux/of_clk.h
+++ b/include/linux/of_clk.h
@@ -22,6 +22,8 @@ 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);
 
+const char *of_clk_get_parent_name(struct device_node *np, int index);
+
 void of_clk_init(const struct of_device_id *matches);
 extern void of_fixed_clk_setup(struct device_node *np);
 
-- 
1.7.5.4

[PATCH 7/7] clk: add highbank clock support

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/Kconfig                  |    1 +
 arch/arm/boot/dts/highbank.dts    |   76 +++++++++
 arch/arm/mach-highbank/Makefile   |    2 +-
 arch/arm/mach-highbank/clock.c    |   62 -------
 arch/arm/mach-highbank/highbank.c |   16 ++
 drivers/clk/Makefile              |    2 +
 drivers/clk/clk-highbank.c        |  335 +++++++++++++++++++++++++++++++++++++
 7 files changed, 431 insertions(+), 63 deletions(-)
 delete mode 100644 arch/arm/mach-highbank/clock.c
 create mode 100644 drivers/clk/clk-highbank.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a48aecc..2019548 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -346,6 +346,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 305635b..06194d5 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -24,6 +24,7 @@
 	compatible = "calxeda,highbank";
 	#address-cells = <1>;
 	#size-cells = <1>;
+	clock-ranges;
 
 	cpus {
 		#address-cells = <1>;
@@ -75,12 +76,14 @@
 			compatible = "arm,smp-twd";
 			reg = <0xfff10600 0x20>;
 			interrupts = <1 13 0xf04>;
+			clocks = <&a9periphclk>;
 		};
 
 		watchdog@fff10620 {
 			compatible = "arm,cortex-a9-wdt";
 			reg = <0xfff10620 0x20>;
 			interrupts = <1 14 0xf04>;
+			clocks = <&a9periphclk>;
 		};
 
 		intc: interrupt-controller@fff11000 {
@@ -117,12 +120,14 @@
 			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>;
 		};
 
 		gpioe: gpio@fff30000 {
@@ -131,6 +136,7 @@
 			gpio-controller;
 			reg = <0xfff30000 0x1000>;
 			interrupts = <0 14 4>;
+			clocks = <&pclk>;
 		};
 
 		gpiof: gpio@fff31000 {
@@ -139,6 +145,7 @@
 			gpio-controller;
 			reg = <0xfff31000 0x1000>;
 			interrupts = <0 15 4>;
+			clocks = <&pclk>;
 		};
 
 		gpiog: gpio@fff32000 {
@@ -147,6 +154,7 @@
 			gpio-controller;
 			reg = <0xfff32000 0x1000>;
 			interrupts = <0 16 4>;
+			clocks = <&pclk>;
 		};
 
 		gpioh: gpio@fff33000 {
@@ -155,24 +163,30 @@
 			gpio-controller;
 			reg = <0xfff33000 0x1000>;
 			interrupts = <0 17 4>;
+			clocks = <&pclk>;
 		};
 
 		timer {
 			compatible = "arm,sp804", "arm,primecell";
 			reg = <0xfff34000 0x1000>;
 			interrupts = <0 18 4>;
+			clocks = <&pclk>, <&pclk>;
+			clock-names = "apb_pclk", "bus";
 		};
 
 		rtc@fff35000 {
 			compatible = "arm,pl031", "arm,primecell";
 			reg = <0xfff35000 0x1000>;
 			interrupts = <0 19 4>;
+			clocks = <&pclk>;
 		};
 
 		serial@fff36000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0xfff36000 0x1000>;
 			interrupts = <0 20 4>;
+			clocks = <&pclk>, <&pclk>;
+			clock-names = "apb_pclk", "bus";
 		};
 
 		smic@fff3a000 {
@@ -187,12 +201,74 @@
 		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>;
+					clock-divider = <4>;
+				};
+
+				a9bclk: a9bclk {
+					#clock-cells = <0>;
+					compatible = "calxeda,hb-a9bus-clock";
+					clocks = <&a9pll>;
+					reg = <0x104>;
+					clock-divider = <4>;
+				};
+
+				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>;
 		};
 
 		ethernet@fff50000 {
diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
index 986958a..8bf3868 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_LOCAL_TIMERS)		+= localtimer.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 8394d51..5c0cdaa 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -23,6 +23,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_address.h>
 #include <linux/smp.h>
+#include <linux/of_clk.h>
 
 #include <asm/cacheflush.h>
 #include <asm/smp_plat.h>
@@ -91,6 +92,16 @@ static void __init highbank_init_irq(void)
 	l2x0_of_init(0, ~0UL);
 }
 
+static struct clk_lookup lookups[] = {
+	{
+		.dev_id = "sp804",
+		.con_id = NULL,
+	}, {
+		.dev_id = "smp_twd",
+		.con_id = NULL,
+	}
+};
+
 static void __init highbank_timer_init(void)
 {
 	int irq;
@@ -108,6 +119,11 @@ static void __init highbank_timer_init(void)
 	irq = irq_of_parse_and_map(np, 0);
 
 	highbank_clocks_init();
+	lookups[0].clk = of_clk_get(np, 0);
+
+	np = of_find_compatible_node(NULL, NULL, "arm,smp-twd");
+	lookups[1].clk = of_clk_get(np, 0);
+	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
 
 	sp804_clocksource_init(timer_base + 0x20, "timer1");
 	sp804_clockevents_init(timer_base, irq, "timer0");
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 1f736bc..70c713f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -2,3 +2,5 @@
 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
+
+obj-$(CONFIG_ARCH_HIGHBANK)	+= clk-highbank.o
diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
new file mode 100644
index 0000000..722fc67
--- /dev/null
+++ b/drivers/clk/clk-highbank.c
@@ -0,0 +1,335 @@
+/*
+ * 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/errno.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_clk.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
+
+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 *clk)
+	{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	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 *clk)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	u32 reg;
+
+	reg = readl(hbclk->reg);
+	reg |= HB_PLL_RESET;
+	writel(reg, hbclk->reg);
+}
+
+static int clk_pll_enable(struct clk_hw *clk)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	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 *clk)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	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 *clk,
+					 unsigned long parent_rate)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	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 *clk, unsigned long rate,
+			       unsigned long *unused)
+{
+	u32 divq, divf;
+	unsigned long ref_freq = clk_get_rate(clk_get_parent(clk->clk));
+
+	clk_pll_calc(rate, ref_freq, &divq, &divf);
+
+	return (ref_freq * (divf + 1)) / (1 << divq);
+}
+
+static int clk_pll_set_rate(struct clk_hw *clk, unsigned long rate)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	unsigned long parent_rate;
+	u32 divq, divf;
+	u32 reg;
+
+	parent_rate = clk_get_rate(clk_get_parent(clk->clk));
+	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);
+
+		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 *clk,
+						   unsigned long parent_rate)
+{
+	return parent_rate / 2;
+}
+
+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 *clk,
+						unsigned long parent_rate)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	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 *clk,
+					     unsigned long parent_rate)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	u32 div;
+
+	div = readl(hbclk->reg);
+	div++;
+	div *= 2;
+
+	return parent_rate / div;
+}
+
+static long clk_periclk_round_rate(struct clk_hw *clk, unsigned long rate,
+				   unsigned long *unused)
+{
+	unsigned long parent_rate = clk_get_rate(clk_get_parent(clk->clk));
+	u32 div;
+
+	div = parent_rate / rate;
+	div++;
+	div &= ~0x1;
+
+	return parent_rate / div;
+}
+
+static int clk_periclk_set_rate(struct clk_hw *clk, unsigned long rate)
+{
+	struct hb_clk *hbclk = to_hb_clk(clk);
+	u32 div;
+
+	div = clk_get_rate(clk_get_parent(clk->clk)) / 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 void __init 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;
+	int rc;
+
+	rc = of_property_read_u32(node, "reg", &reg);
+	if (WARN_ON(rc))
+		return;
+
+	hb_clk = kzalloc(sizeof(*hb_clk), GFP_KERNEL);
+	if (WARN_ON(!hb_clk))
+		return;
+
+	hb_clk->reg = sregs_base + reg;
+
+	of_property_read_string(node, "clock-outputs", &clk_name);
+
+	hb_clk->parent_name = of_clk_get_parent_name(node, 0);
+
+	printk("registering clk %s, parent = %s\n", clk_name, hb_clk->parent_name);
+	clk = clk_register(NULL, clk_name, ops, &hb_clk->hw,
+			   &hb_clk->parent_name, 1, 0);
+	if (WARN_ON(!clk)) {
+		kfree(hb_clk);
+		return;
+	}
+	rc = of_clk_add_provider(node, NULL, 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)
+{
+	hb_clk_init(node, &a9bclk_ops);
+}
+
+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.5.4

Re: [PATCH 7/7] clk: add highbank clock support

[Shawn Guo]

On Tue, Mar 13, 2012 at 06:22:27PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> This adds real clock support to Calxeda Highbank SOC using the common
> clock infrastructure.
> 
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/Kconfig                  |    1 +
>  arch/arm/boot/dts/highbank.dts    |   76 +++++++++
>  arch/arm/mach-highbank/Makefile   |    2 +-
>  arch/arm/mach-highbank/clock.c    |   62 -------
>  arch/arm/mach-highbank/highbank.c |   16 ++
>  drivers/clk/Makefile              |    2 +
>  drivers/clk/clk-highbank.c        |  335 +++++++++++++++++++++++++++++++++++++
>  7 files changed, 431 insertions(+), 63 deletions(-)
>  delete mode 100644 arch/arm/mach-highbank/clock.c
>  create mode 100644 drivers/clk/clk-highbank.c
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a48aecc..2019548 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -346,6 +346,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 305635b..06194d5 100644
> --- a/arch/arm/boot/dts/highbank.dts
> +++ b/arch/arm/boot/dts/highbank.dts
> @@ -24,6 +24,7 @@
>  	compatible = "calxeda,highbank";
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> +	clock-ranges;
>  
>  	cpus {
>  		#address-cells = <1>;
> @@ -75,12 +76,14 @@
>  			compatible = "arm,smp-twd";
>  			reg = <0xfff10600 0x20>;
>  			interrupts = <1 13 0xf04>;
> +			clocks = <&a9periphclk>;
>  		};
>  
>  		watchdog@fff10620 {
>  			compatible = "arm,cortex-a9-wdt";
>  			reg = <0xfff10620 0x20>;
>  			interrupts = <1 14 0xf04>;
> +			clocks = <&a9periphclk>;
>  		};
>  
>  		intc: interrupt-controller@fff11000 {
> @@ -117,12 +120,14 @@
>  			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>;
>  		};
>  
>  		gpioe: gpio@fff30000 {
> @@ -131,6 +136,7 @@
>  			gpio-controller;
>  			reg = <0xfff30000 0x1000>;
>  			interrupts = <0 14 4>;
> +			clocks = <&pclk>;
>  		};
>  
>  		gpiof: gpio@fff31000 {
> @@ -139,6 +145,7 @@
>  			gpio-controller;
>  			reg = <0xfff31000 0x1000>;
>  			interrupts = <0 15 4>;
> +			clocks = <&pclk>;
>  		};
>  
>  		gpiog: gpio@fff32000 {
> @@ -147,6 +154,7 @@
>  			gpio-controller;
>  			reg = <0xfff32000 0x1000>;
>  			interrupts = <0 16 4>;
> +			clocks = <&pclk>;
>  		};
>  
>  		gpioh: gpio@fff33000 {
> @@ -155,24 +163,30 @@
>  			gpio-controller;
>  			reg = <0xfff33000 0x1000>;
>  			interrupts = <0 17 4>;
> +			clocks = <&pclk>;
>  		};
>  
>  		timer {
>  			compatible = "arm,sp804", "arm,primecell";
>  			reg = <0xfff34000 0x1000>;
>  			interrupts = <0 18 4>;
> +			clocks = <&pclk>, <&pclk>;
> +			clock-names = "apb_pclk", "bus";
>  		};
>  
>  		rtc@fff35000 {
>  			compatible = "arm,pl031", "arm,primecell";
>  			reg = <0xfff35000 0x1000>;
>  			interrupts = <0 19 4>;
> +			clocks = <&pclk>;
>  		};
>  
>  		serial@fff36000 {
>  			compatible = "arm,pl011", "arm,primecell";
>  			reg = <0xfff36000 0x1000>;
>  			interrupts = <0 20 4>;
> +			clocks = <&pclk>, <&pclk>;
> +			clock-names = "apb_pclk", "bus";
>  		};
>  
>  		smic@fff3a000 {
> @@ -187,12 +201,74 @@
>  		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";

Where are all these "calxeda,*-clock' compatible documented?

> +					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>;
> +					clock-divider = <4>;

Where is this "clock-divider" binding documented?

> +				};
> +
> +				a9bclk: a9bclk {
> +					#clock-cells = <0>;
> +					compatible = "calxeda,hb-a9bus-clock";
> +					clocks = <&a9pll>;
> +					reg = <0x104>;
> +					clock-divider = <4>;
> +				};
> +
> +				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>;
>  		};
>  
>  		ethernet@fff50000 {
> diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
> index 986958a..8bf3868 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_LOCAL_TIMERS)		+= localtimer.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 8394d51..5c0cdaa 100644
> --- a/arch/arm/mach-highbank/highbank.c
> +++ b/arch/arm/mach-highbank/highbank.c
> @@ -23,6 +23,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_address.h>
>  #include <linux/smp.h>
> +#include <linux/of_clk.h>
>  
>  #include <asm/cacheflush.h>
>  #include <asm/smp_plat.h>
> @@ -91,6 +92,16 @@ static void __init highbank_init_irq(void)
>  	l2x0_of_init(0, ~0UL);
>  }
>  
> +static struct clk_lookup lookups[] = {
> +	{
> +		.dev_id = "sp804",
> +		.con_id = NULL,
> +	}, {
> +		.dev_id = "smp_twd",
> +		.con_id = NULL,
> +	}
> +};
> +
>  static void __init highbank_timer_init(void)
>  {
>  	int irq;
> @@ -108,6 +119,11 @@ static void __init highbank_timer_init(void)
>  	irq = irq_of_parse_and_map(np, 0);
>  
>  	highbank_clocks_init();
> +	lookups[0].clk = of_clk_get(np, 0);
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,smp-twd");
> +	lookups[1].clk = of_clk_get(np, 0);
> +	clkdev_add_table(lookups, ARRAY_SIZE(lookups));
>  
>  	sp804_clocksource_init(timer_base + 0x20, "timer1");
>  	sp804_clockevents_init(timer_base, irq, "timer0");
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 1f736bc..70c713f 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -2,3 +2,5 @@
>  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
> +
> +obj-$(CONFIG_ARCH_HIGHBANK)	+= clk-highbank.o
> diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
> new file mode 100644
> index 0000000..722fc67
> --- /dev/null
> +++ b/drivers/clk/clk-highbank.c
> @@ -0,0 +1,335 @@
> +/*
> + * 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/errno.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_clk.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
> +
> +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 *clk)
> +	{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	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 *clk)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	u32 reg;
> +
> +	reg = readl(hbclk->reg);
> +	reg |= HB_PLL_RESET;
> +	writel(reg, hbclk->reg);
> +}
> +
> +static int clk_pll_enable(struct clk_hw *clk)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	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 *clk)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	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 *clk,
> +					 unsigned long parent_rate)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	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 *clk, unsigned long rate,
> +			       unsigned long *unused)
> +{
> +	u32 divq, divf;
> +	unsigned long ref_freq = clk_get_rate(clk_get_parent(clk->clk));
> +
> +	clk_pll_calc(rate, ref_freq, &divq, &divf);
> +
> +	return (ref_freq * (divf + 1)) / (1 << divq);
> +}
> +
> +static int clk_pll_set_rate(struct clk_hw *clk, unsigned long rate)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	unsigned long parent_rate;
> +	u32 divq, divf;
> +	u32 reg;
> +
> +	parent_rate = clk_get_rate(clk_get_parent(clk->clk));
> +	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);
> +
> +		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 *clk,
> +						   unsigned long parent_rate)
> +{
> +	return parent_rate / 2;
> +}
> +
> +static const struct clk_ops a9periphclk_ops = {
> +	.recalc_rate = clk_cpu_periphclk_recalc_rate,
> +};
> +

This basically is a clk-fixed-factor added by Sascha.

> +static unsigned long clk_cpu_a9bclk_recalc_rate(struct clk_hw *clk,
> +						unsigned long parent_rate)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	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,

Since there is a divider for the clock, the ops should have .round_rate
and .set_rate, no?

> +};
> +
> +static unsigned long clk_periclk_recalc_rate(struct clk_hw *clk,
> +					     unsigned long parent_rate)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	u32 div;
> +
> +	div = readl(hbclk->reg);
> +	div++;
> +	div *= 2;
> +
> +	return parent_rate / div;
> +}
> +
> +static long clk_periclk_round_rate(struct clk_hw *clk, unsigned long rate,
> +				   unsigned long *unused)
> +{
> +	unsigned long parent_rate = clk_get_rate(clk_get_parent(clk->clk));
> +	u32 div;
> +
> +	div = parent_rate / rate;
> +	div++;
> +	div &= ~0x1;
> +
> +	return parent_rate / div;
> +}
> +
> +static int clk_periclk_set_rate(struct clk_hw *clk, unsigned long rate)
> +{
> +	struct hb_clk *hbclk = to_hb_clk(clk);
> +	u32 div;
> +
> +	div = clk_get_rate(clk_get_parent(clk->clk)) / 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 void __init 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;
> +	int rc;
> +
> +	rc = of_property_read_u32(node, "reg", &reg);
> +	if (WARN_ON(rc))
> +		return;
> +
> +	hb_clk = kzalloc(sizeof(*hb_clk), GFP_KERNEL);
> +	if (WARN_ON(!hb_clk))
> +		return;
> +
> +	hb_clk->reg = sregs_base + reg;
> +
> +	of_property_read_string(node, "clock-outputs", &clk_name);
> +
> +	hb_clk->parent_name = of_clk_get_parent_name(node, 0);
> +
> +	printk("registering clk %s, parent = %s\n", clk_name, hb_clk->parent_name);
> +	clk = clk_register(NULL, clk_name, ops, &hb_clk->hw,
> +			   &hb_clk->parent_name, 1, 0);
> +	if (WARN_ON(!clk)) {
> +		kfree(hb_clk);
> +		return;
> +	}
> +	rc = of_clk_add_provider(node, NULL, 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)
> +{
> +	hb_clk_init(node, &a9bclk_ops);
> +}
> +
> +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, },

So that's the difference between your clock and mine.  You do not reuse
any basic clk, except clk_fixed_rate, while my clocks are all about
basic clk except pll.  That's why you need a long list of SoC specific
binding, while I do not.  All I need is the binding for those basic
clks.

Regards,
Shawn

> +	{}
> +};
> +
> +void __init highbank_clocks_init(void)
> +{
> +	of_clk_init(clk_match);
> +}

Re: [PATCH 7/7] clk: add highbank clock support

[Rob Herring]

On 04/09/2012 09:06 PM, Shawn Guo wrote:
> On Tue, Mar 13, 2012 at 06:22:27PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> This adds real clock support to Calxeda Highbank SOC using the common
>> clock infrastructure.
>>
>> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> ---

[snip]

>> +
>> +				osc: oscillator {
>> +					#clock-cells = <0>;
>> +					compatible = "fixed-clock";
>> +					clock-frequency = <33333000>;
>> +				};
>> +
>> +				ddrpll: ddrpll {
>> +					#clock-cells = <0>;
>> +					compatible = "calxeda,hb-pll-clock";
> 
> Where are all these "calxeda,*-clock' compatible documented?

Right. Need to add that.

> 
>> +					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>;
>> +					clock-divider = <4>;
> 
> Where is this "clock-divider" binding documented?

This should be deleted.



>> +static unsigned long clk_cpu_periphclk_recalc_rate(struct clk_hw *clk,
>> +						   unsigned long parent_rate)
>> +{
>> +	return parent_rate / 2;
>> +}
>> +
>> +static const struct clk_ops a9periphclk_ops = {
>> +	.recalc_rate = clk_cpu_periphclk_recalc_rate,
>> +};
>> +
> 
> This basically is a clk-fixed-factor added by Sascha.
> 

Well that didn't exist when I sent this out based on v6 of Mike's patches.

Anyway, it is not really fixed divider either. I need to add reading the
register value.

>> +static unsigned long clk_cpu_a9bclk_recalc_rate(struct clk_hw *clk,
>> +						unsigned long parent_rate)
>> +{
>> +	struct hb_clk *hbclk = to_hb_clk(clk);
>> +	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,
> 
> Since there is a divider for the clock, the ops should have .round_rate
> and .set_rate, no?
> 

Except I have no need or desire to support changing it. When and if
there is a need I will add that.

>> +};
>> +
>> +static unsigned long clk_periclk_recalc_rate(struct clk_hw *clk,
>> +					     unsigned long parent_rate)
>> +{
>> +	struct hb_clk *hbclk = to_hb_clk(clk);
>> +	u32 div;
>> +
>> +	div = readl(hbclk->reg);
>> +	div++;
>> +	div *= 2;
>> +
>> +	return parent_rate / div;
>> +}


>> +
>> +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, },
> 
> So that's the difference between your clock and mine.  You do not reuse
> any basic clk, except clk_fixed_rate, while my clocks are all about
> basic clk except pll.  That's why you need a long list of SoC specific
> binding, while I do not.  All I need is the binding for those basic
> clks.

Because there are not any I can use. I don't have any muxes or clk
gates. The only one that exists in mainline is divider, but our divider
is not n, n+1, or n^2. It is 2(n+1).

Rob

> 
> Regards,
> Shawn
> 
>> +	{}
>> +};
>> +
>> +void __init highbank_clocks_init(void)
>> +{
>> +	of_clk_init(clk_match);
>> +}