diff mbox

[RFC,2/3] clk: dt: binding for basic multiplexor clock

Message ID 1370281990-15090-3-git-send-email-mturquette@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Mike Turquette June 3, 2013, 5:53 p.m. UTC
Device Tree binding for the basic clock multiplexor, plus the setup
function to register the clock.  Based on the existing fixed-clock
binding.

Also relocate declaration of of_fixed_factor_clk_setup to keep things
tidy.

Signed-off-by: Mike Turquette <mturquette@linaro.org>
---
 .../devicetree/bindings/clock/mux-clock.txt        | 75 ++++++++++++++++++++++
 drivers/clk/clk-mux.c                              | 65 ++++++++++++++++++-
 include/linux/clk-provider.h                       |  5 +-
 3 files changed, 143 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mux-clock.txt

Comments

Heiko Stübner June 3, 2013, 7:33 p.m. UTC | #1
Hi Mike,

I think it's a multiplexEr clock in the patch title, and see below


Am Montag, 3. Juni 2013, 19:53:09 schrieb Mike Turquette:
> Device Tree binding for the basic clock multiplexor, plus the setup
> function to register the clock.  Based on the existing fixed-clock
> binding.
> 
> Also relocate declaration of of_fixed_factor_clk_setup to keep things
> tidy.
> 
> Signed-off-by: Mike Turquette <mturquette@linaro.org>

[...]

> +
> +	reg = of_iomap(node, 0);
> +	pr_err("%s: reg is 0x%p\n", __func__, reg);
> +
> +	if (of_property_read_u32(node, "mask", &mask)) {
> +		pr_err("%s: missing mask property for %s\n", __func__, node->name);
> +		return;
> +	}
> +
> +	if (of_property_read_u32(node, "shift", &shift))
> +		pr_debug("%s: missing shift property defaults to zero for %s\n",
> +				__func__, node->name);
> +
> +	if (of_property_read_bool(node, "index_one"))
> +		clk_mux_flags |= CLK_MUX_INDEX_ONE;
> +
> +	clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
> +			0, reg, 0, mask, clk_mux_flags,

                      ^- should probably be shift


Otherwise looks cool and I'm currently trying it with my Rockchip code.


Heiko
Mike Turquette June 3, 2013, 8:07 p.m. UTC | #2
Quoting Heiko Stübner (2013-06-03 12:33:19)
> Hi Mike,
> 
> I think it's a multiplexEr clock in the patch title, and see below
> 

Doh, you are right.  But "xor" is so much cooler looking than "xer"...

> 
> Am Montag, 3. Juni 2013, 19:53:09 schrieb Mike Turquette:
> > Device Tree binding for the basic clock multiplexor, plus the setup
> > function to register the clock.  Based on the existing fixed-clock
> > binding.
> > 
> > Also relocate declaration of of_fixed_factor_clk_setup to keep things
> > tidy.
> > 
> > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> 
> [...]
> 
> > +
> > +     reg = of_iomap(node, 0);
> > +     pr_err("%s: reg is 0x%p\n", __func__, reg);
> > +
> > +     if (of_property_read_u32(node, "mask", &mask)) {
> > +             pr_err("%s: missing mask property for %s\n", __func__, node->name);
> > +             return;
> > +     }
> > +
> > +     if (of_property_read_u32(node, "shift", &shift))
> > +             pr_debug("%s: missing shift property defaults to zero for %s\n",
> > +                             __func__, node->name);
> > +
> > +     if (of_property_read_bool(node, "index_one"))
> > +             clk_mux_flags |= CLK_MUX_INDEX_ONE;
> > +
> > +     clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
> > +                     0, reg, 0, mask, clk_mux_flags,
> 
>                       ^- should probably be shift
> 
> 
> Otherwise looks cool and I'm currently trying it with my Rockchip code.
> 

Right again.  My test platform seems to not shift the mask at all so
this did not cause a visible bug for me.

Thanks for the review,
Mike

> 
> Heiko
Heiko Stübner June 3, 2013, 8:15 p.m. UTC | #3
Am Montag, 3. Juni 2013, 22:07:22 schrieb Mike Turquette:
> Quoting Heiko Stübner (2013-06-03 12:33:19)
> 
> > Hi Mike,
> > 
> > I think it's a multiplexEr clock in the patch title, and see below
> 
> Doh, you are right.  But "xor" is so much cooler looking than "xer"...
> 
> > Am Montag, 3. Juni 2013, 19:53:09 schrieb Mike Turquette:
> > > Device Tree binding for the basic clock multiplexor, plus the setup
> > > function to register the clock.  Based on the existing fixed-clock
> > > binding.
> > > 
> > > Also relocate declaration of of_fixed_factor_clk_setup to keep things
> > > tidy.
> > > 
> > > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> > 
> > [...]
> > 
> > > +
> > > +     reg = of_iomap(node, 0);
> > > +     pr_err("%s: reg is 0x%p\n", __func__, reg);
> > > +
> > > +     if (of_property_read_u32(node, "mask", &mask)) {
> > > +             pr_err("%s: missing mask property for %s\n", __func__,
> > > node->name); +             return;
> > > +     }
> > > +
> > > +     if (of_property_read_u32(node, "shift", &shift))
> > > +             pr_debug("%s: missing shift property defaults to zero for
> > > %s\n", +                             __func__, node->name);
> > > +
> > > +     if (of_property_read_bool(node, "index_one"))
> > > +             clk_mux_flags |= CLK_MUX_INDEX_ONE;
> > > +
> > > +     clk = clk_register_mux_table(NULL, clk_name, parent_names,
> > > num_parents, +                     0, reg, 0, mask, clk_mux_flags,
> > > 
> >                       ^- should probably be shift
> > 
> > Otherwise looks cool and I'm currently trying it with my Rockchip code.
> 
> Right again.  My test platform seems to not shift the mask at all so
> this did not cause a visible bug for me.

I'm currently converting my rockchip stuff to it, as it would solve a lot of 
problems with the numerous clock types I'm defining just to put the shift and 
mask values somewhere.

So lets see if anything more turns up ;-)
Heiko Stübner June 3, 2013, 9:39 p.m. UTC | #4
Am Montag, 3. Juni 2013, 22:15:45 schrieb Heiko Stübner:
> Am Montag, 3. Juni 2013, 22:07:22 schrieb Mike Turquette:
> > Quoting Heiko Stübner (2013-06-03 12:33:19)
> > 
> > > Hi Mike,
> > > 
> > > I think it's a multiplexEr clock in the patch title, and see below
> > 
> > Doh, you are right.  But "xor" is so much cooler looking than "xer"...
> > 
> > > Am Montag, 3. Juni 2013, 19:53:09 schrieb Mike Turquette:
> > > > Device Tree binding for the basic clock multiplexor, plus the setup
> > > > function to register the clock.  Based on the existing fixed-clock
> > > > binding.
> > > > 
> > > > Also relocate declaration of of_fixed_factor_clk_setup to keep things
> > > > tidy.
> > > > 
> > > > Signed-off-by: Mike Turquette <mturquette@linaro.org>
> > > 
> > > [...]
> > > 
> > > > +
> > > > +     reg = of_iomap(node, 0);
> > > > +     pr_err("%s: reg is 0x%p\n", __func__, reg);
> > > > +
> > > > +     if (of_property_read_u32(node, "mask", &mask)) {
> > > > +             pr_err("%s: missing mask property for %s\n", __func__,
> > > > node->name); +             return;
> > > > +     }
> > > > +
> > > > +     if (of_property_read_u32(node, "shift", &shift))
> > > > +             pr_debug("%s: missing shift property defaults to zero
> > > > for %s\n", +                             __func__, node->name);
> > > > +
> > > > +     if (of_property_read_bool(node, "index_one"))
> > > > +             clk_mux_flags |= CLK_MUX_INDEX_ONE;
> > > > +
> > > > +     clk = clk_register_mux_table(NULL, clk_name, parent_names,
> > > > num_parents, +                     0, reg, 0, mask, clk_mux_flags,
> > > > 
> > >                       ^- should probably be shift
> > > 
> > > Otherwise looks cool and I'm currently trying it with my Rockchip code.
> > 
> > Right again.  My test platform seems to not shift the mask at all so
> > this did not cause a visible bug for me.
> 
> I'm currently converting my rockchip stuff to it, as it would solve a lot
> of problems with the numerous clock types I'm defining just to put the
> shift and mask values somewhere.
> 
> So lets see if anything more turns up ;-)

yep ... of_mux_clk_setup is quite talkative ... there are quite some (debug-?) 
pr_errs in it ;-)


Heiko
Mike Turquette June 4, 2013, 6:14 a.m. UTC | #5
On Mon, Jun 3, 2013 at 2:39 PM, Heiko Stübner <heiko@sntech.de> wrote:
> Am Montag, 3. Juni 2013, 22:15:45 schrieb Heiko Stübner:
>> Am Montag, 3. Juni 2013, 22:07:22 schrieb Mike Turquette:
>> > Quoting Heiko Stübner (2013-06-03 12:33:19)
>> >
>> > > Hi Mike,
>> > >
>> > > I think it's a multiplexEr clock in the patch title, and see below
>> >
>> > Doh, you are right.  But "xor" is so much cooler looking than "xer"...
>> >
>> > > Am Montag, 3. Juni 2013, 19:53:09 schrieb Mike Turquette:
>> > > > Device Tree binding for the basic clock multiplexor, plus the setup
>> > > > function to register the clock.  Based on the existing fixed-clock
>> > > > binding.
>> > > >
>> > > > Also relocate declaration of of_fixed_factor_clk_setup to keep things
>> > > > tidy.
>> > > >
>> > > > Signed-off-by: Mike Turquette <mturquette@linaro.org>
>> > >
>> > > [...]
>> > >
>> > > > +
>> > > > +     reg = of_iomap(node, 0);
>> > > > +     pr_err("%s: reg is 0x%p\n", __func__, reg);
>> > > > +
>> > > > +     if (of_property_read_u32(node, "mask", &mask)) {
>> > > > +             pr_err("%s: missing mask property for %s\n", __func__,
>> > > > node->name); +             return;
>> > > > +     }
>> > > > +
>> > > > +     if (of_property_read_u32(node, "shift", &shift))
>> > > > +             pr_debug("%s: missing shift property defaults to zero
>> > > > for %s\n", +                             __func__, node->name);
>> > > > +
>> > > > +     if (of_property_read_bool(node, "index_one"))
>> > > > +             clk_mux_flags |= CLK_MUX_INDEX_ONE;
>> > > > +
>> > > > +     clk = clk_register_mux_table(NULL, clk_name, parent_names,
>> > > > num_parents, +                     0, reg, 0, mask, clk_mux_flags,
>> > > >
>> > >                       ^- should probably be shift
>> > >
>> > > Otherwise looks cool and I'm currently trying it with my Rockchip code.
>> >
>> > Right again.  My test platform seems to not shift the mask at all so
>> > this did not cause a visible bug for me.
>>
>> I'm currently converting my rockchip stuff to it, as it would solve a lot
>> of problems with the numerous clock types I'm defining just to put the
>> shift and mask values somewhere.
>>
>> So lets see if anything more turns up ;-)
>
> yep ... of_mux_clk_setup is quite talkative ... there are quite some (debug-?)
> pr_errs in it ;-)
>

Yes I realized that after sending.  Hopefully not too many curse words
in the trace messages.  Will remove for v2.

Regards,
Mike

>
> Heiko
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/clock/mux-clock.txt b/Documentation/devicetree/bindings/clock/mux-clock.txt
new file mode 100644
index 0000000..eda5ba3
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mux-clock.txt
@@ -0,0 +1,75 @@ 
+Binding for simple mux clock.
+
+This binding uses the common clock binding[1].  It assumes a
+register-mapped multiplexor 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
+- 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
+- shift : number of bits to shift the mask, defaults to 0 if not present
+- index_one : valid input select programming starts at 1, not zero
+
+Examples:
+	clock: clock@4a008100 {
+		compatible = "mux-clock";
+		#clock-cells = <0>;
+		clocks = <&clock_foo>, <&clock_bar>, <&clock_baz>;
+		reg = <0x4a008100 0x4>
+		mask = <0x3>;
+		index_one;
+	};
+
+	clock: clock@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 25b1734..8bcbc7c 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
@@ -153,3 +155,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);
+	pr_err("%s: clk_name is %s\n", __func__, 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;
+	}
+	pr_err("%s: num_parents is %d\n", __func__, num_parents);
+
+	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);
+		pr_err("%s: parent_names[%d] is %s\n", __func__, i, parent_names[i]);
+	}
+
+	reg = of_iomap(node, 0);
+	pr_err("%s: reg is 0x%p\n", __func__, reg);
+
+	if (of_property_read_u32(node, "mask", &mask)) {
+		pr_err("%s: missing mask property for %s\n", __func__, node->name);
+		return;
+	}
+
+	if (of_property_read_u32(node, "shift", &shift))
+		pr_debug("%s: missing shift property defaults to zero for %s\n",
+				__func__, node->name);
+
+	if (of_property_read_bool(node, "index_one"))
+		clk_mux_flags |= CLK_MUX_INDEX_ONE;
+
+	clk = clk_register_mux_table(NULL, clk_name, parent_names, num_parents,
+			0, reg, 0, 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 f8d38f2..9c404c2 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -325,7 +325,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
@@ -346,10 +346,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
  *