[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Mike Turquette]

This series introduces binding definitions for common register-mapped
clock multiplexer, divider and gate IP blocks along with the
corresponding setup functions for matching DT data.  The bindings are
similar to the struct definitions but please don't hold that against the
binding: the struct definitions closely model the hardware register
layout.

This version fixes bugs and incorporates support for the hiword-mask
property needed on Hisilicon and Rockchip platforms.

Tested on OMAP4460 Panda ES.

Mike Turquette (5):
  clk: divider: replace bitfield width with mask
  clk: of: helper for determining number of parent clocks
  clk: dt: binding for basic multiplexer clock
  clk: dt: binding for basic divider clock
  clk: dt: binding for basic gate clock

 .../devicetree/bindings/clock/divider-clock.txt    |  90 +++++++++++++++
 .../devicetree/bindings/clock/gate-clock.txt       |  36 ++++++
 .../devicetree/bindings/clock/mux-clock.txt        |  79 +++++++++++++
 arch/arm/mach-imx/clk-busy.c                       |   2 +-
 drivers/clk/clk-divider.c                          | 127 ++++++++++++++++++---
 drivers/clk/clk-gate.c                             |  43 +++++++
 drivers/clk/clk-mux.c                              |  65 ++++++++++-
 drivers/clk/clk.c                                  |   6 +
 drivers/clk/mxs/clk-div.c                          |   2 +-
 include/linux/clk-private.h                        |   2 +-
 include/linux/clk-provider.h                       |  12 +-
 11 files changed, 441 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/divider-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt

-- 
1.8.1.2

[PATCH v3 1/5] clk: divider: replace bitfield width with mask

[Mike Turquette]

The forthcoming Device Tree binding for the divider clock type will use
a bitfield mask instead of bitfield width, which is what the current
basic divider implementation uses.

This patch replaces the u8 width in struct clk_divider with a u32 mask.
The divider code is updated to use the bit mask internally but the two
registration functions still accept the width to maintain compatibility
with existing users.

Also updated in this patch is the clk-private.h divider macro and two
Freescale clock divider implementations that are based on struct
clk_divider.

Cc: Sascha Hauer <kernel@pengutronix.de>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
No change since v2

 arch/arm/mach-imx/clk-busy.c |  2 +-
 drivers/clk/clk-divider.c    | 31 +++++++++++++++----------------
 drivers/clk/mxs/clk-div.c    |  2 +-
 include/linux/clk-private.h  |  2 +-
 include/linux/clk-provider.h |  2 +-
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-imx/clk-busy.c b/arch/arm/mach-imx/clk-busy.c
index 4bb1bc4..bc88e38 100644
--- a/arch/arm/mach-imx/clk-busy.c
+++ b/arch/arm/mach-imx/clk-busy.c
@@ -95,7 +95,7 @@ struct clk *imx_clk_busy_divider(const char *name, const char *parent_name,
 
 	busy->div.reg = reg;
 	busy->div.shift = shift;
-	busy->div.width = width;
+	busy->div.mask = BIT(width) - 1;
 	busy->div.lock = &imx_ccm_lock;
 	busy->div_ops = &clk_divider_ops;
 
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 6d55eb2..ac9cb7f 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -30,8 +30,6 @@
 
 #define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw)
 
-#define div_mask(d)	((1 << ((d)->width)) - 1)
-
 static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
 {
 	unsigned int maxdiv = 0;
@@ -46,12 +44,12 @@ static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
 static unsigned int _get_maxdiv(struct clk_divider *divider)
 {
 	if (divider->flags & CLK_DIVIDER_ONE_BASED)
-		return div_mask(divider);
+		return divider->mask;
 	if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
-		return 1 << div_mask(divider);
+		return 1 << divider->mask;
 	if (divider->table)
 		return _get_table_maxdiv(divider->table);
-	return div_mask(divider) + 1;
+	return divider->mask + 1;
 }
 
 static unsigned int _get_table_div(const struct clk_div_table *table,
@@ -105,7 +103,7 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
 	unsigned int div, val;
 
 	val = readl(divider->reg) >> divider->shift;
-	val &= div_mask(divider);
+	val &= divider->mask;
 
 	div = _get_div(divider, val);
 	if (!div) {
@@ -221,17 +219,17 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	div = parent_rate / rate;
 	value = _get_val(divider, div);
 
-	if (value > div_mask(divider))
-		value = div_mask(divider);
+	if (value > divider->mask)
+		value = divider->mask;
 
 	if (divider->lock)
 		spin_lock_irqsave(divider->lock, flags);
 
 	if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
-		val = div_mask(divider) << (divider->shift + 16);
+		val = divider->mask << (divider->shift + 16);
 	} else {
 		val = readl(divider->reg);
-		val &= ~(div_mask(divider) << divider->shift);
+		val &= ~(divider->mask << divider->shift);
 	}
 	val |= value << divider->shift;
 	writel(val, divider->reg);
@@ -251,7 +249,7 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
 
 static struct clk *_register_divider(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width,
+		void __iomem *reg, u8 shift, u32 mask,
 		u8 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock)
 {
@@ -260,8 +258,9 @@ static struct clk *_register_divider(struct device *dev, const char *name,
 	struct clk_init_data init;
 
 	if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
-		if (width + shift > 16) {
-			pr_warn("divider value exceeds LOWORD field\n");
+		if ((mask << shift) & 0xffff0000) {
+			pr_warn("%s: divider value exceeds LOWORD field\n",
+					__func__);
 			return ERR_PTR(-EINVAL);
 		}
 	}
@@ -282,7 +281,7 @@ static struct clk *_register_divider(struct device *dev, const char *name,
 	/* struct clk_divider assignments */
 	div->reg = reg;
 	div->shift = shift;
-	div->width = width;
+	div->mask = mask;
 	div->flags = clk_divider_flags;
 	div->lock = lock;
 	div->hw.init = &init;
@@ -315,7 +314,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
 		u8 clk_divider_flags, spinlock_t *lock)
 {
 	return _register_divider(dev, name, parent_name, flags, reg, shift,
-			width, clk_divider_flags, NULL, lock);
+			((1 << width) - 1), clk_divider_flags, NULL, lock);
 }
 
 /**
@@ -339,5 +338,5 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		spinlock_t *lock)
 {
 	return _register_divider(dev, name, parent_name, flags, reg, shift,
-			width, clk_divider_flags, table, lock);
+			((1 << width) - 1), clk_divider_flags, table, lock);
 }
diff --git a/drivers/clk/mxs/clk-div.c b/drivers/clk/mxs/clk-div.c
index 90e1da9..af2428e 100644
--- a/drivers/clk/mxs/clk-div.c
+++ b/drivers/clk/mxs/clk-div.c
@@ -96,7 +96,7 @@ struct clk *mxs_clk_div(const char *name, const char *parent_name,
 
 	div->divider.reg = reg;
 	div->divider.shift = shift;
-	div->divider.width = width;
+	div->divider.mask = BIT(width) - 1;
 	div->divider.flags = CLK_DIVIDER_ONE_BASED;
 	div->divider.lock = &mxs_lock;
 	div->divider.hw.init = &init;
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index dd7adff..67a3069 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -119,7 +119,7 @@ struct clk {
 		},						\
 		.reg = _reg,					\
 		.shift = _shift,				\
-		.width = _width,				\
+		.mask = ((1 << _width) - 1),			\
 		.flags = _divider_flags,			\
 		.table = _table,				\
 		.lock = _lock,					\
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1ec14a7..4920fd1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -271,7 +271,7 @@ struct clk_divider {
 	struct clk_hw	hw;
 	void __iomem	*reg;
 	u8		shift;
-	u8		width;
+	u32		mask;
 	u8		flags;
 	const struct clk_div_table	*table;
 	spinlock_t	*lock;
-- 
1.8.1.2

[PATCH v3 2/5] clk: of: helper for determining number of parent clocks

[Mike Turquette]

Walks the "clocks" array of parent clock phandles and returns the
number.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
No change since v2

 drivers/clk/clk.c            | 6 ++++++
 include/linux/clk-provider.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index edf3fe1..a9feab6 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2081,6 +2081,12 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec)
 	return clk;
 }
 
+int of_clk_get_parent_count(struct device_node *np)
+{
+	return of_count_phandle_with_args(np, "clocks", "#clock-cells");
+}
+EXPORT_SYMBOL_GPL(of_clk_get_parent_count);
+
 const char *of_clk_get_parent_name(struct device_node *np, int index)
 {
 	struct of_phandle_args clkspec;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4920fd1..8730cb9 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -457,6 +457,7 @@ void of_clk_del_provider(struct device_node *np);
 struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
 				  void *data);
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
+int of_clk_get_parent_count(struct device_node *np);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
 
 void of_clk_init(const struct of_device_id *matches);
-- 
1.8.1.2

[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock

[Mike Turquette]

Device Tree binding for the basic clock multiplexer, plus the setup
function to register the clock.  Based on the existing fixed-clock
binding.

Includes minor beautification of clk-provider.h where some whitespace is
added and of_fixed_factor_clock_setup is relocated to maintain a
consistent style.

Tero Kristo contributed helpful bug fixes to this patch.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
Changes since v2:
* added hiword-mask property to the binding
* changed bit-shift property from u8 to u32 in the dt binding

Changes since v1:
* pass shift value into clk_register_mux_table
* s/multiplexor/multiplexer/
* removed debug prints
* mask is u32, shift is u8
* DT property names use dashes instead of underscores
* DT property names are more verbose
* shift property is optional in binding and can be auto-generated from a
  full 32-bit mask

 .../devicetree/bindings/clock/mux-clock.txt        | 79 ++++++++++++++++++++++
 drivers/clk/clk-mux.c                              | 65 +++++++++++++++++-
 include/linux/clk-provider.h                       |  5 +-
 3 files changed, 147 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
new file mode 100644
index 0000000..54cb8d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
@@ -0,0 +1,79 @@
+Binding for simple mux clock.
+
+This binding uses the common clock binding[1].  It assumes a
+register-mapped multiplexer with multiple input clock signals or
+parents, one of which can be selected as output.  This clock does not
+gate or adjust the parent rate via a divider or multiplier.
+
+By default the "clocks" property lists the parents in the same order
+as they are programmed into the regster.  E.g:
+
+	clocks = <&foo_clock>, <&bar_clock>, <&baz_clock>;
+
+results in programming the register as follows:
+
+register value		selected parent clock
+0			foo_clock
+1			bar_clock
+2			baz_clock
+
+Some clock controller IPs do not allow a value of zero to be programmed
+into the register, instead indexing begins at 1.  The optional property
+"index_one" modified the scheme as follows:
+
+register value		selected clock parent
+1			foo_clock
+2			bar_clock
+3			baz_clock
+
+Additionally an optional table of bit and parent pairs may be supplied
+like so:
+
+	table = <&foo_clock 0x0>, <&bar_clock, 0x2>, <&baz_clock, 0x4>;
+
+where the first value in the pair is the parent clock and the second
+value is the bitfield to be programmed into the register.
+
+The binding must provide the register to control the mux and the mask
+for the corresponding control bits.  Optionally the number of bits to
+shift that mask if necessary.  If the shift value is missing it is the
+same as supplying a zero shift.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "mux-clock".
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of parent clocks
+- reg : base address for register controlling adjustable mux
+- bit-mask : arbitrary bitmask for programming the adjustable mux
+
+Optional properties:
+- clock-output-names : From common clock binding.
+- table : array of integer pairs defining parents & bitfield values
+- bit-shift : number of bits to shift the bit-mask, defaults to
+  (ffs(mask) - 1) if not present
+- index-starts-at-one : valid input select programming starts at 1, not
+  zero
+- hiword-mask : lower half of the register programs the mux, upper half
+  of the register indicates bits that were updated in the lower half
+
+Examples:
+	clock: clock at 4a008100 {
+		compatible = "mux-clock";
+		#clock-cells = <0>;
+		clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
+		reg = <0x4a008100 0x4>
+		mask = <0x3>;
+		index_one;
+	};
+
+	clock: clock at 4a008100 {
+		#clock-cells = <0>;
+		compatible = "mux-clock";
+		clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
+		reg = <0x4a008100 0x4>;
+		mask = <0x3>;
+		shift = <0>;
+		table = <&clock_foo 1>, <&clock_bar 2>, <&clock_baz 3>;
+	};
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 614444c..4751bce 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
  * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao@linaro.org>
- * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
+ * Copyright (C) 2011-2013 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -16,6 +16,8 @@
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 /*
  * DOC: basic adjustable multiplexer clock that cannot gate
@@ -166,3 +168,64 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 				      flags, reg, shift, mask, clk_mux_flags,
 				      NULL, lock);
 }
+
+#ifdef CONFIG_OF
+/**
+ * of_mux_clk_setup() - Setup function for simple mux rate clock
+ */
+void of_mux_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	int num_parents;
+	const char **parent_names;
+	int i;
+	u8 clk_mux_flags = 0;
+	u32 mask = 0;
+	u32 shift = 0;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("%s: mux-clock %s must have parent(s)\n",
+				__func__, node->name);
+		return;
+	}
+
+	parent_names = kzalloc((sizeof(char*) * num_parents),
+			GFP_KERNEL);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	reg = of_iomap(node, 0);
+
+	if (of_property_read_u32(node, "bit-mask", &mask)) {
+		pr_err("%s: missing bit-mask property for %s\n", __func__, node->name);
+		return;
+	}
+
+	if (of_property_read_u32(node, "bit-shift", &shift)) {
+		shift = __ffs(mask);
+		pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
+				__func__, shift, node->name);
+	}
+
+	if (of_property_read_bool(node, "index-starts-at-one"))
+		clk_mux_flags |= CLK_MUX_INDEX_ONE;
+
+	if (of_property_read_bool(node, "hiword-mask"))
+		clk_mux_flags |= CLK_MUX_HIWORD_MASK;
+
+	clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
+			0, reg, (u8) shift, mask, clk_mux_flags,
+			NULL, NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+EXPORT_SYMBOL_GPL(of_mux_clk_setup);
+CLK_OF_DECLARE(mux_clk, "mux-clock", of_mux_clk_setup);
+#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 8730cb9..24a04b8 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -340,7 +340,7 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 		void __iomem *reg, u8 shift, u32 mask,
 		u8 clk_mux_flags, u32 *table, spinlock_t *lock);
 
-void of_fixed_factor_clk_setup(struct device_node *node);
+void of_mux_clk_setup(struct device_node *node);
 
 /**
  * struct clk_fixed_factor - fixed multiplier and divider clock
@@ -361,10 +361,13 @@ struct clk_fixed_factor {
 };
 
 extern struct clk_ops clk_fixed_factor_ops;
+
 struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		unsigned int mult, unsigned int div);
 
+void of_fixed_factor_clk_setup(struct device_node *node);
+
 /***
  * struct clk_composite - aggregate clock of mux, divider and gate clocks
  *
-- 
1.8.1.2

[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock

[Haojian Zhuang]

On 21 June 2013 14:14, Mike Turquette <mturquette@linaro.org> wrote:
> Device Tree binding for the basic clock multiplexer, plus the setup
> function to register the clock.  Based on the existing fixed-clock
> binding.
>
> Includes minor beautification of clk-provider.h where some whitespace is
> added and of_fixed_factor_clock_setup is relocated to maintain a
> consistent style.
>
> Tero Kristo contributed helpful bug fixes to this patch.
>
> Signed-off-by: Mike Turquette <mturquette@linaro.org>
> ---
> Changes since v2:
> * added hiword-mask property to the binding
> * changed bit-shift property from u8 to u32 in the dt binding
>
> Changes since v1:
> * pass shift value into clk_register_mux_table
> * s/multiplexor/multiplexer/
> * removed debug prints
> * mask is u32, shift is u8
> * DT property names use dashes instead of underscores
> * DT property names are more verbose
> * shift property is optional in binding and can be auto-generated from a
>   full 32-bit mask
>
>  .../devicetree/bindings/clock/mux-clock.txt        | 79 ++++++++++++++++++++++
>  drivers/clk/clk-mux.c                              | 65 +++++++++++++++++-
>  include/linux/clk-provider.h                       |  5 +-
>  3 files changed, 147 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
> new file mode 100644
> index 0000000..54cb8d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
> @@ -0,0 +1,79 @@
> +Binding for simple mux clock.
> +
> +This binding uses the common clock binding[1].  It assumes a
> +register-mapped multiplexer with multiple input clock signals or
> +parents, one of which can be selected as output.  This clock does not
> +gate or adjust the parent rate via a divider or multiplier.
> +
> +By default the "clocks" property lists the parents in the same order
> +as they are programmed into the regster.  E.g:
> +
> +       clocks = <&foo_clock>, <&bar_clock>, <&baz_clock>;
> +
> +results in programming the register as follows:
> +
> +register value         selected parent clock
> +0                      foo_clock
> +1                      bar_clock
> +2                      baz_clock
> +
> +Some clock controller IPs do not allow a value of zero to be programmed
> +into the register, instead indexing begins at 1.  The optional property
> +"index_one" modified the scheme as follows:
> +
> +register value         selected clock parent
> +1                      foo_clock
> +2                      bar_clock
> +3                      baz_clock
> +
> +Additionally an optional table of bit and parent pairs may be supplied
> +like so:
> +
> +       table = <&foo_clock 0x0>, <&bar_clock, 0x2>, <&baz_clock, 0x4>;
> +
> +where the first value in the pair is the parent clock and the second
> +value is the bitfield to be programmed into the register.
> +
> +The binding must provide the register to control the mux and the mask
> +for the corresponding control bits.  Optionally the number of bits to
> +shift that mask if necessary.  If the shift value is missing it is the
> +same as supplying a zero shift.
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : shall be "mux-clock".
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- clocks : link phandles of parent clocks
> +- reg : base address for register controlling adjustable mux
> +- bit-mask : arbitrary bitmask for programming the adjustable mux
> +
> +Optional properties:
> +- clock-output-names : From common clock binding.
> +- table : array of integer pairs defining parents & bitfield values
> +- bit-shift : number of bits to shift the bit-mask, defaults to
> +  (ffs(mask) - 1) if not present
> +- index-starts-at-one : valid input select programming starts at 1, not
> +  zero
> +- hiword-mask : lower half of the register programs the mux, upper half
> +  of the register indicates bits that were updated in the lower half
> +
> +Examples:
> +       clock: clock at 4a008100 {
> +               compatible = "mux-clock";
> +               #clock-cells = <0>;
> +               clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> +               reg = <0x4a008100 0x4>
> +               mask = <0x3>;
> +               index_one;
> +       };
> +
> +       clock: clock at 4a008100 {
> +               #clock-cells = <0>;
> +               compatible = "mux-clock";
> +               clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> +               reg = <0x4a008100 0x4>;
> +               mask = <0x3>;
> +               shift = <0>;
> +               table = <&clock_foo 1>, <&clock_bar 2>, <&clock_baz 3>;
> +       };
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 614444c..4751bce 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -1,7 +1,7 @@
>  /*
>   * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
>   * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao@linaro.org>
> - * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
> + * Copyright (C) 2011-2013 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -16,6 +16,8 @@
>  #include <linux/slab.h>
>  #include <linux/io.h>
>  #include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>
>  /*
>   * DOC: basic adjustable multiplexer clock that cannot gate
> @@ -166,3 +168,64 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
>                                       flags, reg, shift, mask, clk_mux_flags,
>                                       NULL, lock);
>  }
> +
> +#ifdef CONFIG_OF
> +/**
> + * of_mux_clk_setup() - Setup function for simple mux rate clock
> + */
> +void of_mux_clk_setup(struct device_node *node)
> +{
> +       struct clk *clk;
> +       const char *clk_name = node->name;
> +       void __iomem *reg;
> +       int num_parents;
> +       const char **parent_names;
> +       int i;
> +       u8 clk_mux_flags = 0;
> +       u32 mask = 0;
> +       u32 shift = 0;
> +
> +       of_property_read_string(node, "clock-output-names", &clk_name);
> +
> +       num_parents = of_clk_get_parent_count(node);
> +       if (num_parents < 1) {
> +               pr_err("%s: mux-clock %s must have parent(s)\n",
> +                               __func__, node->name);
> +               return;
> +       }
> +
> +       parent_names = kzalloc((sizeof(char*) * num_parents),
> +                       GFP_KERNEL);
> +
> +       for (i = 0; i < num_parents; i++)
> +               parent_names[i] = of_clk_get_parent_name(node, i);
> +
> +       reg = of_iomap(node, 0);

As you mentioned above,  reg : base address for register controlling
adjustable mux.

Each clock node has the reg property. It means that we have to
allocate one page memory
for each register since of_iomap() is used at here. We always have
lots of registers with
continuous address in SoC, like 0x40a00040, 0x40a00044, .... Then
we'll waste too much
memory in system.

How about only define common function to parse property? Make register
mapping handled
by vendor's clock driver.

Regards
Haojian

[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock

[Mike Turquette]

Quoting Haojian Zhuang (2013-06-25 01:27:58)
> On 21 June 2013 14:14, Mike Turquette <mturquette@linaro.org> wrote:
> > Device Tree binding for the basic clock multiplexer, plus the setup
> > function to register the clock.  Based on the existing fixed-clock
> > binding.
> >
> > Includes minor beautification of clk-provider.h where some whitespace is
> > added and of_fixed_factor_clock_setup is relocated to maintain a
> > consistent style.
> >
> > Tero Kristo contributed helpful bug fixes to this patch.
> >
> > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> > ---
> > Changes since v2:
> > * added hiword-mask property to the binding
> > * changed bit-shift property from u8 to u32 in the dt binding
> >
> > Changes since v1:
> > * pass shift value into clk_register_mux_table
> > * s/multiplexor/multiplexer/
> > * removed debug prints
> > * mask is u32, shift is u8
> > * DT property names use dashes instead of underscores
> > * DT property names are more verbose
> > * shift property is optional in binding and can be auto-generated from a
> >   full 32-bit mask
> >
> >  .../devicetree/bindings/clock/mux-clock.txt        | 79 ++++++++++++++++++++++
> >  drivers/clk/clk-mux.c                              | 65 +++++++++++++++++-
> >  include/linux/clk-provider.h                       |  5 +-
> >  3 files changed, 147 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt
> >
> > diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
> > new file mode 100644
> > index 0000000..54cb8d1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
> > @@ -0,0 +1,79 @@
> > +Binding for simple mux clock.
> > +
> > +This binding uses the common clock binding[1].  It assumes a
> > +register-mapped multiplexer with multiple input clock signals or
> > +parents, one of which can be selected as output.  This clock does not
> > +gate or adjust the parent rate via a divider or multiplier.
> > +
> > +By default the "clocks" property lists the parents in the same order
> > +as they are programmed into the regster.  E.g:
> > +
> > +       clocks = <&foo_clock>, <&bar_clock>, <&baz_clock>;
> > +
> > +results in programming the register as follows:
> > +
> > +register value         selected parent clock
> > +0                      foo_clock
> > +1                      bar_clock
> > +2                      baz_clock
> > +
> > +Some clock controller IPs do not allow a value of zero to be programmed
> > +into the register, instead indexing begins at 1.  The optional property
> > +"index_one" modified the scheme as follows:
> > +
> > +register value         selected clock parent
> > +1                      foo_clock
> > +2                      bar_clock
> > +3                      baz_clock
> > +
> > +Additionally an optional table of bit and parent pairs may be supplied
> > +like so:
> > +
> > +       table = <&foo_clock 0x0>, <&bar_clock, 0x2>, <&baz_clock, 0x4>;
> > +
> > +where the first value in the pair is the parent clock and the second
> > +value is the bitfield to be programmed into the register.
> > +
> > +The binding must provide the register to control the mux and the mask
> > +for the corresponding control bits.  Optionally the number of bits to
> > +shift that mask if necessary.  If the shift value is missing it is the
> > +same as supplying a zero shift.
> > +
> > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +Required properties:
> > +- compatible : shall be "mux-clock".
> > +- #clock-cells : from common clock binding; shall be set to 0.
> > +- clocks : link phandles of parent clocks
> > +- reg : base address for register controlling adjustable mux
> > +- bit-mask : arbitrary bitmask for programming the adjustable mux
> > +
> > +Optional properties:
> > +- clock-output-names : From common clock binding.
> > +- table : array of integer pairs defining parents & bitfield values
> > +- bit-shift : number of bits to shift the bit-mask, defaults to
> > +  (ffs(mask) - 1) if not present
> > +- index-starts-at-one : valid input select programming starts at 1, not
> > +  zero
> > +- hiword-mask : lower half of the register programs the mux, upper half
> > +  of the register indicates bits that were updated in the lower half
> > +
> > +Examples:
> > +       clock: clock at 4a008100 {
> > +               compatible = "mux-clock";
> > +               #clock-cells = <0>;
> > +               clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> > +               reg = <0x4a008100 0x4>
> > +               mask = <0x3>;
> > +               index_one;
> > +       };
> > +
> > +       clock: clock at 4a008100 {
> > +               #clock-cells = <0>;
> > +               compatible = "mux-clock";
> > +               clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> > +               reg = <0x4a008100 0x4>;
> > +               mask = <0x3>;
> > +               shift = <0>;
> > +               table = <&clock_foo 1>, <&clock_bar 2>, <&clock_baz 3>;
> > +       };
> > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> > index 614444c..4751bce 100644
> > --- a/drivers/clk/clk-mux.c
> > +++ b/drivers/clk/clk-mux.c
> > @@ -1,7 +1,7 @@
> >  /*
> >   * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
> >   * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao@linaro.org>
> > - * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
> > + * Copyright (C) 2011-2013 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
> >   *
> >   * This program is free software; you can redistribute it and/or modify
> >   * it under the terms of the GNU General Public License version 2 as
> > @@ -16,6 +16,8 @@
> >  #include <linux/slab.h>
> >  #include <linux/io.h>
> >  #include <linux/err.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> >
> >  /*
> >   * DOC: basic adjustable multiplexer clock that cannot gate
> > @@ -166,3 +168,64 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
> >                                       flags, reg, shift, mask, clk_mux_flags,
> >                                       NULL, lock);
> >  }
> > +
> > +#ifdef CONFIG_OF
> > +/**
> > + * of_mux_clk_setup() - Setup function for simple mux rate clock
> > + */
> > +void of_mux_clk_setup(struct device_node *node)
> > +{
> > +       struct clk *clk;
> > +       const char *clk_name = node->name;
> > +       void __iomem *reg;
> > +       int num_parents;
> > +       const char **parent_names;
> > +       int i;
> > +       u8 clk_mux_flags = 0;
> > +       u32 mask = 0;
> > +       u32 shift = 0;
> > +
> > +       of_property_read_string(node, "clock-output-names", &clk_name);
> > +
> > +       num_parents = of_clk_get_parent_count(node);
> > +       if (num_parents < 1) {
> > +               pr_err("%s: mux-clock %s must have parent(s)\n",
> > +                               __func__, node->name);
> > +               return;
> > +       }
> > +
> > +       parent_names = kzalloc((sizeof(char*) * num_parents),
> > +                       GFP_KERNEL);
> > +
> > +       for (i = 0; i < num_parents; i++)
> > +               parent_names[i] = of_clk_get_parent_name(node, i);
> > +
> > +       reg = of_iomap(node, 0);
> 
> As you mentioned above,  reg : base address for register controlling
> adjustable mux.
> 
> Each clock node has the reg property. It means that we have to
> allocate one page memory
> for each register since of_iomap() is used at here. We always have
> lots of registers with
> continuous address in SoC, like 0x40a00040, 0x40a00044, .... Then
> we'll waste too much
> memory in system.
> 
> How about only define common function to parse property? Make register
> mapping handled
> by vendor's clock driver.

This is probably a good idea. It is independent of the binding though.
The reg property can still be used as described above and however the OS
interacts with it is an implementation detail that can be sorted out
later.

I'll look into your suggestion though, since it could save some memory.

Regards,
Mike

> 
> Regards
> Haojian

[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock

[Mark Rutland]

Hi,

I have a couple of minor comments.

On Fri, Jun 21, 2013 at 07:14:14AM +0100, Mike Turquette wrote:
> Device Tree binding for the basic clock multiplexer, plus the setup
> function to register the clock.  Based on the existing fixed-clock
> binding.
> 
> Includes minor beautification of clk-provider.h where some whitespace is
> added and of_fixed_factor_clock_setup is relocated to maintain a
> consistent style.
> 
> Tero Kristo contributed helpful bug fixes to this patch.
> 
> Signed-off-by: Mike Turquette <mturquette@linaro.org>
> ---
> Changes since v2:
> * added hiword-mask property to the binding
> * changed bit-shift property from u8 to u32 in the dt binding
> 
> Changes since v1:
> * pass shift value into clk_register_mux_table
> * s/multiplexor/multiplexer/
> * removed debug prints
> * mask is u32, shift is u8
> * DT property names use dashes instead of underscores
> * DT property names are more verbose
> * shift property is optional in binding and can be auto-generated from a
>   full 32-bit mask
> 
>  .../devicetree/bindings/clock/mux-clock.txt        | 79 ++++++++++++++++++++++
>  drivers/clk/clk-mux.c                              | 65 +++++++++++++++++-
>  include/linux/clk-provider.h                       |  5 +-
>  3 files changed, 147 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
> new file mode 100644
> index 0000000..54cb8d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
> @@ -0,0 +1,79 @@
> +Binding for simple mux clock.
> +
> +This binding uses the common clock binding[1].  It assumes a
> +register-mapped multiplexer with multiple input clock signals or
> +parents, one of which can be selected as output.  This clock does not
> +gate or adjust the parent rate via a divider or multiplier.
> +
> +By default the "clocks" property lists the parents in the same order
> +as they are programmed into the regster.  E.g:
> +
> +	clocks = <&foo_clock>, <&bar_clock>, <&baz_clock>;
> +
> +results in programming the register as follows:
> +
> +register value		selected parent clock
> +0			foo_clock
> +1			bar_clock
> +2			baz_clock
> +
> +Some clock controller IPs do not allow a value of zero to be programmed
> +into the register, instead indexing begins at 1.  The optional property
> +"index_one" modified the scheme as follows:

I assume this is meant to be "index-starts-at-one", given the code and
changelog?

> +
> +register value		selected clock parent
> +1			foo_clock
> +2			bar_clock
> +3			baz_clock
> +
> +Additionally an optional table of bit and parent pairs may be supplied
> +like so:
> +
> +	table = <&foo_clock 0x0>, <&bar_clock, 0x2>, <&baz_clock, 0x4>;
> +
> +where the first value in the pair is the parent clock and the second
> +value is the bitfield to be programmed into the register.
> +
> +The binding must provide the register to control the mux and the mask
> +for the corresponding control bits.  Optionally the number of bits to
> +shift that mask if necessary.  If the shift value is missing it is the
> +same as supplying a zero shift.
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : shall be "mux-clock".
> +- #clock-cells : from common clock binding; shall be set to 0.
> +- clocks : link phandles of parent clocks
> +- reg : base address for register controlling adjustable mux
> +- bit-mask : arbitrary bitmask for programming the adjustable mux
> +
> +Optional properties:
> +- clock-output-names : From common clock binding.
> +- table : array of integer pairs defining parents & bitfield values
> +- bit-shift : number of bits to shift the bit-mask, defaults to
> +  (ffs(mask) - 1) if not present
> +- index-starts-at-one : valid input select programming starts at 1, not
> +  zero
> +- hiword-mask : lower half of the register programs the mux, upper half
> +  of the register indicates bits that were updated in the lower half
> +
> +Examples:
> +	clock: clock at 4a008100 {
> +		compatible = "mux-clock";
> +		#clock-cells = <0>;
> +		clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> +		reg = <0x4a008100 0x4>
> +		mask = <0x3>;
> +		index_one;

And here too?

[...]

> +/**
> + * of_mux_clk_setup() - Setup function for simple mux rate clock
> + */
> +void of_mux_clk_setup(struct device_node *node)
> +{
> +	struct clk *clk;
> +	const char *clk_name = node->name;
> +	void __iomem *reg;
> +	int num_parents;
> +	const char **parent_names;
> +	int i;
> +	u8 clk_mux_flags = 0;
> +	u32 mask = 0;
> +	u32 shift = 0;
> +
> +	of_property_read_string(node, "clock-output-names", &clk_name);
> +
> +	num_parents = of_clk_get_parent_count(node);
> +	if (num_parents < 1) {
> +		pr_err("%s: mux-clock %s must have parent(s)\n",
> +				__func__, node->name);
> +		return;
> +	}
> +
> +	parent_names = kzalloc((sizeof(char*) * num_parents),
> +			GFP_KERNEL);
> +
> +	for (i = 0; i < num_parents; i++)
> +		parent_names[i] = of_clk_get_parent_name(node, i);
> +
> +	reg = of_iomap(node, 0);

Would it not be a good idea to check this isn't NULL before we pass it
to clk_register_mux_table later?

> +
> +	if (of_property_read_u32(node, "bit-mask", &mask)) {
> +		pr_err("%s: missing bit-mask property for %s\n", __func__, node->name);
> +		return;
> +	}
> +
> +	if (of_property_read_u32(node, "bit-shift", &shift)) {
> +		shift = __ffs(mask);
> +		pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
> +				__func__, shift, node->name);
> +	}
> +
> +	if (of_property_read_bool(node, "index-starts-at-one"))
> +		clk_mux_flags |= CLK_MUX_INDEX_ONE;
> +
> +	if (of_property_read_bool(node, "hiword-mask"))
> +		clk_mux_flags |= CLK_MUX_HIWORD_MASK;
> +
> +	clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
> +			0, reg, (u8) shift, mask, clk_mux_flags,
> +			NULL, NULL);

Why do you need the (u8) cast on shift? Isn't that implicit?

Thanks,
Mark.

[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock

[Mike Turquette]

Quoting Mark Rutland (2013-06-26 01:40:58)
> Hi,
> 
> I have a couple of minor comments.
> 
> On Fri, Jun 21, 2013 at 07:14:14AM +0100, Mike Turquette wrote:
> > Device Tree binding for the basic clock multiplexer, plus the setup
> > function to register the clock.  Based on the existing fixed-clock
> > binding.
> > 
> > Includes minor beautification of clk-provider.h where some whitespace is
> > added and of_fixed_factor_clock_setup is relocated to maintain a
> > consistent style.
> > 
> > Tero Kristo contributed helpful bug fixes to this patch.
> > 
> > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> > ---
> > Changes since v2:
> > * added hiword-mask property to the binding
> > * changed bit-shift property from u8 to u32 in the dt binding
> > 
> > Changes since v1:
> > * pass shift value into clk_register_mux_table
> > * s/multiplexor/multiplexer/
> > * removed debug prints
> > * mask is u32, shift is u8
> > * DT property names use dashes instead of underscores
> > * DT property names are more verbose
> > * shift property is optional in binding and can be auto-generated from a
> >   full 32-bit mask
> > 
> >  .../devicetree/bindings/clock/mux-clock.txt        | 79 ++++++++++++++++++++++
> >  drivers/clk/clk-mux.c                              | 65 +++++++++++++++++-
> >  include/linux/clk-provider.h                       |  5 +-
> >  3 files changed, 147 insertions(+), 2 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
> > new file mode 100644
> > index 0000000..54cb8d1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
> > @@ -0,0 +1,79 @@
> > +Binding for simple mux clock.
> > +
> > +This binding uses the common clock binding[1].  It assumes a
> > +register-mapped multiplexer with multiple input clock signals or
> > +parents, one of which can be selected as output.  This clock does not
> > +gate or adjust the parent rate via a divider or multiplier.
> > +
> > +By default the "clocks" property lists the parents in the same order
> > +as they are programmed into the regster.  E.g:
> > +
> > +     clocks = <&foo_clock>, <&bar_clock>, <&baz_clock>;
> > +
> > +results in programming the register as follows:
> > +
> > +register value               selected parent clock
> > +0                    foo_clock
> > +1                    bar_clock
> > +2                    baz_clock
> > +
> > +Some clock controller IPs do not allow a value of zero to be programmed
> > +into the register, instead indexing begins at 1.  The optional property
> > +"index_one" modified the scheme as follows:
> 
> I assume this is meant to be "index-starts-at-one", given the code and
> changelog?
> 
> > +
> > +register value               selected clock parent
> > +1                    foo_clock
> > +2                    bar_clock
> > +3                    baz_clock
> > +
> > +Additionally an optional table of bit and parent pairs may be supplied
> > +like so:
> > +
> > +     table = <&foo_clock 0x0>, <&bar_clock, 0x2>, <&baz_clock, 0x4>;
> > +
> > +where the first value in the pair is the parent clock and the second
> > +value is the bitfield to be programmed into the register.
> > +
> > +The binding must provide the register to control the mux and the mask
> > +for the corresponding control bits.  Optionally the number of bits to
> > +shift that mask if necessary.  If the shift value is missing it is the
> > +same as supplying a zero shift.
> > +
> > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +Required properties:
> > +- compatible : shall be "mux-clock".
> > +- #clock-cells : from common clock binding; shall be set to 0.
> > +- clocks : link phandles of parent clocks
> > +- reg : base address for register controlling adjustable mux
> > +- bit-mask : arbitrary bitmask for programming the adjustable mux
> > +
> > +Optional properties:
> > +- clock-output-names : From common clock binding.
> > +- table : array of integer pairs defining parents & bitfield values
> > +- bit-shift : number of bits to shift the bit-mask, defaults to
> > +  (ffs(mask) - 1) if not present
> > +- index-starts-at-one : valid input select programming starts at 1, not
> > +  zero
> > +- hiword-mask : lower half of the register programs the mux, upper half
> > +  of the register indicates bits that were updated in the lower half
> > +
> > +Examples:
> > +     clock: clock at 4a008100 {
> > +             compatible = "mux-clock";
> > +             #clock-cells = <0>;
> > +             clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
> > +             reg = <0x4a008100 0x4>
> > +             mask = <0x3>;
> > +             index_one;
> 
> And here too?
> 
> [...]
> 
> > +/**
> > + * of_mux_clk_setup() - Setup function for simple mux rate clock
> > + */
> > +void of_mux_clk_setup(struct device_node *node)
> > +{
> > +     struct clk *clk;
> > +     const char *clk_name = node->name;
> > +     void __iomem *reg;
> > +     int num_parents;
> > +     const char **parent_names;
> > +     int i;
> > +     u8 clk_mux_flags = 0;
> > +     u32 mask = 0;
> > +     u32 shift = 0;
> > +
> > +     of_property_read_string(node, "clock-output-names", &clk_name);
> > +
> > +     num_parents = of_clk_get_parent_count(node);
> > +     if (num_parents < 1) {
> > +             pr_err("%s: mux-clock %s must have parent(s)\n",
> > +                             __func__, node->name);
> > +             return;
> > +     }
> > +
> > +     parent_names = kzalloc((sizeof(char*) * num_parents),
> > +                     GFP_KERNEL);
> > +
> > +     for (i = 0; i < num_parents; i++)
> > +             parent_names[i] = of_clk_get_parent_name(node, i);
> > +
> > +     reg = of_iomap(node, 0);
> 
> Would it not be a good idea to check this isn't NULL before we pass it
> to clk_register_mux_table later?
> 
> > +
> > +     if (of_property_read_u32(node, "bit-mask", &mask)) {
> > +             pr_err("%s: missing bit-mask property for %s\n", __func__, node->name);
> > +             return;
> > +     }
> > +
> > +     if (of_property_read_u32(node, "bit-shift", &shift)) {
> > +             shift = __ffs(mask);
> > +             pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
> > +                             __func__, shift, node->name);
> > +     }
> > +
> > +     if (of_property_read_bool(node, "index-starts-at-one"))
> > +             clk_mux_flags |= CLK_MUX_INDEX_ONE;
> > +
> > +     if (of_property_read_bool(node, "hiword-mask"))
> > +             clk_mux_flags |= CLK_MUX_HIWORD_MASK;
> > +
> > +     clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
> > +                     0, reg, (u8) shift, mask, clk_mux_flags,
> > +                     NULL, NULL);
> 
> Why do you need the (u8) cast on shift? Isn't that implicit?

All of your suggestions are good. I'll roll them into the next version.

Regards,
Mike
> 
> Thanks,
> Mark.

[PATCH v3 4/5] clk: dt: binding for basic divider clock

[Mike Turquette]

Devicetree binding for the basic clock divider, plus the setup function
to register the clock.  Based on the existing fixed-clock binding.

Tero Kristo contributed helpful bug fixes to this patch.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
Changes since v2:
* added hiword-mask property to the binding
* changed bit-shift property from u8 to u32 in the dt binding
* correctly parses divider table (thanks Tero!)

Changes since v1:
* mask is u32, shift is u8
* use bit mask instead of bitfield width DT property names use dashes
* instead of underscores
* DT property names are more verbose added minimum/maximum divider
* values to binding
* shift property is optional in binding and can be auto-generated from a
  full 32-bit mask

 .../devicetree/bindings/clock/divider-clock.txt    | 90 ++++++++++++++++++++
 drivers/clk/clk-divider.c                          | 96 +++++++++++++++++++++-
 include/linux/clk-provider.h                       |  2 +
 3 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/divider-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/divider-clock.txt b/Documentation/devicetree/bindings/clock/divider-clock.txt
new file mode 100644
index 0000000..ac80724
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/divider-clock.txt
@@ -0,0 +1,90 @@
+Binding for simple divider clock.
+
+This binding uses the common clock binding[1].  It assumes a
+register-mapped adjustable clock rate divider that does not gate and has
+only one input clock or parent.  By default the value programmed into
+the register is one less than the actual divisor value.  E.g:
+
+register value		actual divisor value
+0			1
+1			2
+2			3
+
+This assumption may be modified by the following optional properties:
+
+index-starts-at-one - valid divisor values start at 1, not the default
+of 0.  E.g:
+register value		actual divisor value
+1			1
+2			2
+3			3
+
+index-power-of-two - valid divisor values are powers of two.  E.g:
+register value		actual divisor value
+0			1
+1			2
+2			4
+
+index-allow-zero - same as index_one, but zero is divide-by-1.  E.g:
+register value		actual divisor value
+0			1
+1			1
+2			2
+
+Additionally a table of valid dividers may be supplied like so:
+
+	table = <4 0>, <8, 1>;
+
+where the first value in the pair is the divider and the second value is
+the programmed register bitfield.
+
+The binding must also provide the register to control the divider and
+the mask for the corresponding control bits.  Optionally the number of
+bits to shift that mask, if necessary.  If the shift value is missing it
+is the same as supplying a zero shift.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "divider-clock".
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link to phandle of parent clock
+- reg : base address for register controlling adjustable divider
+- bit-mask : arbitrary bitmask for programming the adjustable divider
+
+Optional properties:
+- clock-output-names : from common clock binding.
+- table : array of integer pairs defining divisors & bitfield values
+- bit-shift : number of bits to shift the bit-mask, defaults to
+  (ffs(mask) - 1) if not present
+- minimum-divider : min divisor for dividing the input clock rate, only
+  needed if the first divisor is offset from the default value
+- maximum-divider : max divisor for dividing the input clock rate, only
+  needed if the max divisor is less than (mask + 1).
+- index-starts-at-one : valid divisor programming starts at 1, not zero
+- index-power-of-two : valid divisor programming must be a power of two
+- index-allow-zero : implies index-one, and programming zero results in
+  divide-by-one
+- hiword-mask : lower half of the register programs the divider, upper
+  half of the register indicates bits that were updated in the lower
+  half
+
+Examples:
+	clock_foo: clock_foo at 4a008100 {
+		compatible = "divider-clock";
+		#clock-cells = <0>;
+		clocks = <&clock_baz>;
+		reg = <0x4a008100 0x4>
+		mask = <0x3>
+		maximum-divider = <3>
+	};
+
+	clock_bar: clock_bar at 4a008108 {
+		#clock-cells = <0>;
+		compatible = "divider-clock";
+		clocks = <&clock_foo>;
+		reg = <0x4a008108 0x4>;
+		mask = <0x1>;
+		shift = <0>;
+		table = < 4 0 >, < 8 1 >;
+	};
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index ac9cb7f..ff24ec2 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2011 Sascha Hauer, Pengutronix <s.hauer@pengutronix.de>
  * Copyright (C) 2011 Richard Zhao, Linaro <richard.zhao@linaro.org>
- * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
+ * Copyright (C) 2011-2013 Mike Turquette, Linaro Ltd <mturquette@linaro.org>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -17,6 +17,8 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/log2.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 /*
  * DOC: basic adjustable divider clock that cannot gate
@@ -340,3 +342,95 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 	return _register_divider(dev, name, parent_name, flags, reg, shift,
 			((1 << width) - 1), clk_divider_flags, table, lock);
 }
+
+#ifdef CONFIG_OF
+struct clk_div_table *of_clk_get_div_table(struct device_node *node)
+{
+	int i;
+	u32 table_size;
+	struct clk_div_table *table;
+	const __be32 *tablespec;
+	u32 val;
+
+	tablespec = of_get_property(node, "table", &table_size);
+
+	if (!tablespec)
+		return NULL;
+
+	table_size /= sizeof(struct clk_div_table);
+
+	table = kzalloc(sizeof(struct clk_div_table) * table_size, GFP_KERNEL);
+	if (!table) {
+		pr_err("%s: unable to allocate memory for %s table\n", __func__, node->name);
+		return NULL;
+	}
+
+	for (i = 0; i < table_size; i++) {
+		of_property_read_u32_index(node, "table", i * 2, &val);
+		table[i].div = val;
+		of_property_read_u32_index(node, "table", i * 2 + 1, &val);
+		table[i].val = val;
+	}
+
+	return table;
+}
+
+/**
+ * of_divider_clk_setup() - Setup function for simple div rate clock
+ */
+void of_divider_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	const char *parent_name;
+	u8 clk_divider_flags = 0;
+	u32 mask = 0;
+	u32 shift = 0;
+	struct clk_div_table *table;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	reg = of_iomap(node, 0);
+
+	if (of_property_read_u32(node, "bit-mask", &mask)) {
+		pr_err("%s: missing bit-mask property for %s\n", __func__, node->name);
+		return;
+	}
+
+	if (of_property_read_u32(node, "bit-shift", &shift)) {
+		shift = __ffs(mask);
+		pr_debug("%s: bit-shift property defaults to 0x%x for %s\n",
+				__func__, shift, node->name);
+	}
+
+	if (of_property_read_bool(node, "index-starts-at-one"))
+		clk_divider_flags |= CLK_DIVIDER_ONE_BASED;
+
+	if (of_property_read_bool(node, "index-power-of-two"))
+		clk_divider_flags |= CLK_DIVIDER_POWER_OF_TWO;
+
+	if (of_property_read_bool(node, "index-allow-zero"))
+		clk_divider_flags |= CLK_DIVIDER_ALLOW_ZERO;
+
+	if (of_property_read_bool(node, "hiword-mask"))
+		clk_divider_flags |= CLK_DIVIDER_HIWORD_MASK;
+
+	table = of_clk_get_div_table(node);
+	if (IS_ERR(table))
+		return;
+
+	clk = _register_divider(NULL, clk_name,
+			parent_name, 0,
+			reg, (u8) shift, mask,
+			clk_divider_flags, table,
+			NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+EXPORT_SYMBOL_GPL(of_divider_clk_setup);
+CLK_OF_DECLARE(divider_clk, "divider-clock", of_divider_clk_setup);
+#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 24a04b8..723ce69 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -293,6 +293,8 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		u8 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock);
 
+void of_divider_clk_setup(struct device_node *node);
+
 /**
  * struct clk_mux - multiplexer clock
  *
-- 
1.8.1.2

[PATCH v3 5/5] clk: dt: binding for basic gate clock

[Mike Turquette]

Device Tree binding for the basic clock gate, plus the setup function to
register the clock.  Based on the existing fixed-clock binding.

A different approach to this was proposed in 2012[1] and a similar
binding was proposed more recently[2] if anyone wants some extra
reading.

[1] http://article.gmane.org/gmane.linux.documentation/5679
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137878.html

Tero Kristo contributed helpful bug fixes to this patch.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
Changes since v2:
* added hiword-mask property to the binding
* changed bit-shift property from u8 to u32 in the dt binding

 .../devicetree/bindings/clock/gate-clock.txt       | 36 ++++++++++++++++++
 drivers/clk/clk-gate.c                             | 43 ++++++++++++++++++++++
 include/linux/clk-provider.h                       |  2 +
 3 files changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/gate-clock.txt b/Documentation/devicetree/bindings/clock/gate-clock.txt
new file mode 100644
index 0000000..1c0d4d5
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/gate-clock.txt
@@ -0,0 +1,36 @@
+Binding for simple gate clock.
+
+This binding uses the common clock binding[1].  It assumes a
+register-mapped clock gate controlled by a single bit that has only one
+input clock or parent.  By default setting the specified bit gates the
+clock signal and clearing the bit ungates it.
+
+The binding must provide the register to control the gate and the bit
+shift for the corresponding gate control bit. Some clocks set the bit to
+gate the clock signal, and clear it to ungate the clock signal. The
+optional "set-bit-to-disable" specifies this behavior.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "gate-clock".
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link to phandle of parent clock
+- reg : base address for register controlling adjustable gate
+- bit-shift : bit shift for programming the clock gate
+
+Optional properties:
+- clock-output-names : from common clock binding.
+- set-bit-to-disable : inverts default gate programming. Setting the bit
+  gates the clock and clearing the bit ungates the clock.
+- hiword-mask : lower half of the register controls the gate, upper half
+  of the register indicates bits that were updated in the lower half
+
+Examples:
+	clock_foo: clock_foo at 4a008100 {
+		compatible = "gate-clock";
+		#clock-cells = <0>;
+		clocks = <&clock_bar>;
+		reg = <0x4a008100 0x4>
+		bit-shift = <3>
+	};
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 790306e..cd595ec 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -15,6 +15,8 @@
 #include <linux/io.h>
 #include <linux/err.h>
 #include <linux/string.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 /**
  * DOC: basic gatable clock which can gate and ungate it's ouput
@@ -161,3 +163,44 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 
 	return clk;
 }
+
+#ifdef CONFIG_OF
+/**
+ * of_gate_clk_setup() - Setup function for simple gate rate clock
+ */
+void of_gate_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	const char *parent_name;
+	u8 clk_gate_flags = 0;
+	u32 bit_idx = 0;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	reg = of_iomap(node, 0);
+
+	if (of_property_read_u32(node, "bit-shift", &bit_idx)) {
+		pr_err("%s: missing bit-shift property for %s\n",
+				__func__, node->name);
+		return;
+	}
+
+	if (of_property_read_bool(node, "set-bit-to-disable"))
+		clk_gate_flags |= CLK_GATE_SET_TO_DISABLE;
+
+	if (of_property_read_bool(node, "hiword-mask"))
+		clk_gate_flags |= CLK_GATE_HIWORD_MASK;
+
+	clk = clk_register_gate(NULL, clk_name, parent_name, 0, reg, (u8) bit_idx,
+			clk_gate_flags, NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+EXPORT_SYMBOL_GPL(of_gate_clk_setup);
+CLK_OF_DECLARE(gate_clk, "gate-clock", of_gate_clk_setup);
+#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 723ce69..4aeaedb 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -232,6 +232,8 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 		void __iomem *reg, u8 bit_idx,
 		u8 clk_gate_flags, spinlock_t *lock);
 
+void of_gate_clk_setup(struct device_node *node);
+
 struct clk_div_table {
 	unsigned int	val;
 	unsigned int	div;
-- 
1.8.1.2

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Heiko Stübner]

Am Freitag, 21. Juni 2013, 08:14:11 schrieb Mike Turquette:
> This series introduces binding definitions for common register-mapped
> clock multiplexer, divider and gate IP blocks along with the
> corresponding setup functions for matching DT data.  The bindings are
> similar to the struct definitions but please don't hold that against the
> binding: the struct definitions closely model the hardware register
> layout.
> 
> This version fixes bugs and incorporates support for the hiword-mask
> property needed on Hisilicon and Rockchip platforms.
> 
> Tested on OMAP4460 Panda ES.

Now it works like a charm :-), so on a rk3066a based board

Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> Mike Turquette (5):
>   clk: divider: replace bitfield width with mask
>   clk: of: helper for determining number of parent clocks
>   clk: dt: binding for basic multiplexer clock
>   clk: dt: binding for basic divider clock
>   clk: dt: binding for basic gate clock
> 
>  .../devicetree/bindings/clock/divider-clock.txt    |  90 +++++++++++++++
>  .../devicetree/bindings/clock/gate-clock.txt       |  36 ++++++
>  .../devicetree/bindings/clock/mux-clock.txt        |  79 +++++++++++++
>  arch/arm/mach-imx/clk-busy.c                       |   2 +-
>  drivers/clk/clk-divider.c                          | 127
> ++++++++++++++++++--- drivers/clk/clk-gate.c                             |
>  43 +++++++
>  drivers/clk/clk-mux.c                              |  65 ++++++++++-
>  drivers/clk/clk.c                                  |   6 +
>  drivers/clk/mxs/clk-div.c                          |   2 +-
>  include/linux/clk-private.h                        |   2 +-
>  include/linux/clk-provider.h                       |  12 +-
>  11 files changed, 441 insertions(+), 23 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/clock/divider-clock.txt create mode
> 100644 Documentation/devicetree/bindings/clock/gate-clock.txt create mode
> 100644 Documentation/devicetree/bindings/clock/mux-clock.txt

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Stephen Boyd]

On 06/20/13 23:14, Mike Turquette wrote:
> This series introduces binding definitions for common register-mapped
> clock multiplexer, divider and gate IP blocks along with the
> corresponding setup functions for matching DT data.  The bindings are
> similar to the struct definitions but please don't hold that against the
> binding: the struct definitions closely model the hardware register
> layout.

I know there was some discussion about clock bindings and how they
should and should not be done at Linaro Connect Europe last week. Can
someone in that discussion reply to the mailing list with what came out
of that? I only have second hand knowledge about the discussion so it
would be good for me and others to know what was discussed. I'm
especially curious because the arm soc update etherpad[1] says "DT
describes what HW is (location, type, attributes), not how HW works
(register descriptions, bitmasks, etc)." but these proposed generic
clock bindings are describing registers and bitmasks.

[1] http://pad.linaro.org/p/LCE13_ARM_SOC_Tree_Consolidation_Update

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Mike Turquette]

Quoting Stephen Boyd (2013-07-18 14:04:44)
> On 06/20/13 23:14, Mike Turquette wrote:
> > This series introduces binding definitions for common register-mapped
> > clock multiplexer, divider and gate IP blocks along with the
> > corresponding setup functions for matching DT data.  The bindings are
> > similar to the struct definitions but please don't hold that against the
> > binding: the struct definitions closely model the hardware register
> > layout.
> 
> I know there was some discussion about clock bindings and how they
> should and should not be done at Linaro Connect Europe last week. Can
> someone in that discussion reply to the mailing list with what came out
> of that? I only have second hand knowledge about the discussion so it
> would be good for me and others to know what was discussed. I'm
> especially curious because the arm soc update etherpad[1] says "DT
> describes what HW is (location, type, attributes), not how HW works
> (register descriptions, bitmasks, etc)." but these proposed generic
> clock bindings are describing registers and bitmasks.

Hi Stephen,

I just happened across a to-do list note telling me to respond to this
email. Better late than never.

The meeting you're referring to did not focus on clock provider bindings
as much as it focused on a pair of standard clock attributes that could
be referenced by clock consumers. They are "assigned-clock-parent" and
"assigned-clock-rate". The usage scribbled on the white board[1] was
something like:

mux: mux {
	clocks = <&clock-foo>, <&clock-bar>;
};

uart: uart {
	clocks = <&mux>;
	assigned-clock-parent = <1>;
};

In the example above assigned-clock-parent uses the index of the mux,
but I prefer the following:

uart: uart {
	clocks = <&mux>;
	assigned-clock-parent = <&clock-bar>;
};

This is a way to establish initial configuration from the consumer's
perspective. Similarly something can be done for the clock rate with
assigned-clock-rate.

With all of that said this is consumer-level stuff. We'll definitely
talk about the clock provider DT bindings at the ARM Summit, which is
what you discuss above.

Regards,
Mike

[1] https://plus.google.com/u/0/107457854652287465481/posts/cvRveAupUpa

> 
> [1] http://pad.linaro.org/p/LCE13_ARM_SOC_Tree_Consolidation_Update
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Stephen Boyd]

On 08/21, Mike Turquette wrote:
> 
> I just happened across a to-do list note telling me to respond to this
> email. Better late than never.
> 
[snip]
>
> This is a way to establish initial configuration from the consumer's
> perspective. Similarly something can be done for the clock rate with
> assigned-clock-rate.

Ok. Thanks for the information. Unfortunately it isn't what I
thought it was.

> 
> With all of that said this is consumer-level stuff. We'll definitely
> talk about the clock provider DT bindings at the ARM Summit, which is
> what you discuss above.

I can't wait another 2 months to start discussing the clock
provider DT bindings. We need to discuss it on the list.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCH v3 0/5] clk: dt: bindings for mux, divider & gate clocks

[Mike Turquette]

Quoting Stephen Boyd (2013-08-22 21:32:48)
> On 08/21, Mike Turquette wrote:
> > 
> > I just happened across a to-do list note telling me to respond to this
> > email. Better late than never.
> > 
> [snip]
> >
> > This is a way to establish initial configuration from the consumer's
> > perspective. Similarly something can be done for the clock rate with
> > assigned-clock-rate.
> 
> Ok. Thanks for the information. Unfortunately it isn't what I
> thought it was.
> 
> > 
> > With all of that said this is consumer-level stuff. We'll definitely
> > talk about the clock provider DT bindings at the ARM Summit, which is
> > what you discuss above.
> 
> I can't wait another 2 months to start discussing the clock
> provider DT bindings. We need to discuss it on the list.

You asked what was discussed at a conference. I told you what was
discussed at that conference. As an aside I also told you what would be
discussed at an upcoming conference. At no point did I say the
discussion was tabled until then.

Regards,
Mike

> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation