[PATCH v3 3/5] clk: dt: binding for basic multiplexer clock
From: mark.rutland@arm.com (Mark Rutland)
Date: 2013-06-26 08:45:57
Also in:
linux-devicetree, lkml
Hi, I have a couple of minor comments. On Fri, Jun 21, 2013 at 07:14:14AM +0100, Mike Turquette wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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.txtdiff --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.