2013-05-10 15:02:54

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

This adds a metag architecture specific clk-gate and clk-mux which
extends the generic ones to use global lock2 to protect the register
fields. It is common with metag to have an RTOS running on a different
thread or core with access to different bits in the same register (which
contain clock gate/switch bits for other clocks). Access to such
registers must be serialised with a global lock such as the one provided
by the metag architecture port in <asm/global_lock.h>

RFC because despite extending the generic clocks there's still a bit of
duplicated code necessary. One alternative is to add special cases to
the generic clock components for when a global or callback function
based lock is desired instead of a spinlock, but I wasn't sure if that
sort of hack would really be appreciated in the generic drivers.

Comments?

James Hogan (2):
clk: metag/clk-gate: add metag specific clock gate
clk: metag/clk-mux: add metag specific clk-mux

.../bindings/clock/img,meta-gate-clock.txt | 28 +++
.../bindings/clock/img,meta-mux-clock.txt | 33 ++++
drivers/clk/Makefile | 1 +
drivers/clk/metag/Makefile | 3 +
drivers/clk/metag/clk-gate.c | 179 +++++++++++++++++
drivers/clk/metag/clk-mux.c | 211 +++++++++++++++++++++
6 files changed, 455 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
create mode 100644 drivers/clk/metag/Makefile
create mode 100644 drivers/clk/metag/clk-gate.c
create mode 100644 drivers/clk/metag/clk-mux.c

--
1.8.1.2


2013-05-10 15:02:56

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 2/2] clk: metag/clk-mux: add metag specific clk-mux

Add a metag architecture specific clk-mux which extends the generic one
to use global lock2 to protect the register fields. It is common with
metag to have an RTOS running on a different thread or core with access
to different bits in the same register (in this case clock switch bits
for other clocks). Access to such registers must be serialised with a
global lock such as the one provided by the metag architecture port in
<asm/global_lock.h>

Signed-off-by: James Hogan <[email protected]>
Cc: Mike Turquette <[email protected]>
---
.../bindings/clock/img,meta-mux-clock.txt | 33 ++++
drivers/clk/metag/Makefile | 1 +
drivers/clk/metag/clk-mux.c | 211 +++++++++++++++++++++
3 files changed, 245 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
create mode 100644 drivers/clk/metag/clk-mux.c

diff --git a/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt b/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
new file mode 100644
index 0000000..7802939
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
@@ -0,0 +1,33 @@
+Binding for clock multiplexer requiring global Meta locking.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Shall be "img,meta-mux-clock".
+- #clock-cells : From common clock binding; shall be set to 0.
+- reg : Address of configuration register.
+- shift : Shift of mux value field in configuration register.
+- width : Width of mux value field in configuration register.
+- clocks : From common clock binding.
+
+Required source clocks:
+- 0..(1<<width)-1 : Input clocks to multiplex (don't have to be named).
+
+Optional properties:
+- clock-output-names : From common clock binding.
+- default-clock : Mux value to set initially.
+
+Example:
+ clock {
+ compatible = "img,meta-mux-clock";
+ #clock-cells = <0>;
+ clocks = <&xtal1>,
+ <&xtal2>;
+ reg = <0x02005908 0x4>;
+ shift = <0>;
+ width = <1>;
+ clock-output-names = "sysclk0_sw";
+ default-clock = <1>; /* default to xtal2 */
+ };
diff --git a/drivers/clk/metag/Makefile b/drivers/clk/metag/Makefile
index 8e9a6ac..c53ccb2 100644
--- a/drivers/clk/metag/Makefile
+++ b/drivers/clk/metag/Makefile
@@ -1,2 +1,3 @@
# metag clock types
obj-$(CONFIG_COMMON_CLK) += clk-gate.o
+obj-$(CONFIG_COMMON_CLK) += clk-mux.o
diff --git a/drivers/clk/metag/clk-mux.c b/drivers/clk/metag/clk-mux.c
new file mode 100644
index 0000000..5afedde
--- /dev/null
+++ b/drivers/clk/metag/clk-mux.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright (C) 2013 Imagination Technologies Ltd.
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Metag simple multiplexer clock implementation
+ * Based on simple multiplexer clock implementation, but does appropriate
+ * locking to protect registers shared between hardware threads.
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <asm/global_lock.h>
+
+/**
+ * struct clk_metag_mux - metag multiplexer clock
+ *
+ * @mux: the parent class
+ * @ops: pointer to clk_ops of parent class
+ *
+ * Clock with multiple selectable parents. Extends basic mux by using a global
+ * exclusive lock when read-modify-writing the mux field so that multiple
+ * threads/cores can use different fields in the same register.
+ */
+struct clk_metag_mux {
+ struct clk_mux mux;
+ const struct clk_ops *ops;
+};
+
+static inline struct clk_metag_mux *to_clk_metag_mux(struct clk_hw *hw)
+{
+ struct clk_mux *mux = container_of(hw, struct clk_mux, hw);
+
+ return container_of(mux, struct clk_metag_mux, mux);
+}
+
+static u8 clk_metag_mux_get_parent(struct clk_hw *hw)
+{
+ struct clk_metag_mux *mux = to_clk_metag_mux(hw);
+
+ return mux->ops->get_parent(&mux->mux.hw);
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static int clk_metag_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct clk_metag_mux *mux = to_clk_metag_mux(hw);
+ int ret;
+ unsigned long flags;
+
+ __global_lock2(flags);
+ ret = mux->ops->set_parent(&mux->mux.hw, index);
+ __global_unlock2(flags);
+
+ return ret;
+}
+
+static const struct clk_ops clk_metag_mux_ops = {
+ .get_parent = clk_metag_mux_get_parent,
+ .set_parent = clk_metag_mux_set_parent,
+};
+
+static struct clk *__init clk_register_metag_mux(struct device *dev,
+ const char *name, const char **parent_names, u8 num_parents,
+ s32 default_parent, unsigned long flags, void __iomem *reg,
+ u8 shift, u8 width, u8 clk_mux_flags)
+{
+ struct clk_metag_mux *mux;
+ struct clk *clk;
+ struct clk_init_data init;
+
+ /* allocate the mux */
+ mux = kzalloc(sizeof(struct clk_metag_mux), GFP_KERNEL);
+ if (!mux) {
+ pr_err("%s: could not allocate metag mux clk\n", __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ init.name = name;
+ init.ops = &clk_metag_mux_ops;
+ init.flags = flags | CLK_IS_BASIC;
+ init.parent_names = parent_names;
+ init.num_parents = num_parents;
+
+ /* struct clk_mux assignments */
+ mux->mux.reg = reg;
+ mux->mux.shift = shift;
+ mux->mux.mask = BIT(width) - 1;
+ mux->mux.flags = clk_mux_flags;
+ mux->mux.hw.init = &init;
+
+ /* struct clk_metag_mux assignments */
+ mux->ops = &clk_mux_ops;
+
+ /* set default value */
+ if (default_parent >= 0)
+ clk_metag_mux_set_parent(&mux->mux.hw, default_parent);
+
+ clk = clk_register(dev, &mux->mux.hw);
+
+ if (IS_ERR(clk))
+ kfree(mux);
+
+ return clk;
+}
+
+#ifdef CONFIG_OF
+/**
+ * of_metag_mux_clk_setup() - Setup function for simple fixed rate clock
+ */
+static void __init of_metag_mux_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ const char *clk_name = node->name;
+ u32 shift, width, default_clock;
+ void __iomem *reg;
+ int len, i;
+ struct property *prop;
+ const char **parent_names;
+ unsigned int num_parents;
+ u8 flags = 0;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ if (of_property_read_u32(node, "shift", &shift)) {
+ pr_err("%s(%s): could not read shift property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ if (of_property_read_u32(node, "width", &width)) {
+ pr_err("%s(%s): could not read width property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ /* count maximum number of parent clocks */
+ prop = of_find_property(node, "clocks", &len);
+ if (!prop) {
+ pr_err("%s(%s): could not find clocks property\n",
+ __func__, clk_name);
+ return;
+ }
+ /*
+ * There cannot be more parents than entries in "clocks" property (which
+ * may include additional args too. It also needs to fit in a u8.
+ */
+ num_parents = len / sizeof(u32);
+ num_parents = min(num_parents, 0xffu);
+
+ /* allocate an array of parent names */
+ parent_names = kzalloc(sizeof(const char *)*num_parents, GFP_KERNEL);
+ if (!parent_names) {
+ pr_err("%s(%s): could not allocate %u parent names\n",
+ __func__, clk_name, num_parents);
+ goto err_kfree;
+ }
+
+ /* fill in the parent names */
+ for (i = 0; i < num_parents; ++i) {
+ parent_names[i] = of_clk_get_parent_name(node, i);
+ if (!parent_names[i]) {
+ /* truncate array length if we hit the end early */
+ num_parents = i;
+ break;
+ }
+ }
+
+ /* default parent clock (mux value) */
+ if (!of_property_read_u32(node, "default-clock", &default_clock)) {
+ if (default_clock >= num_parents) {
+ pr_err("%s(%s): default-clock %u out of range (%u bits)\n",
+ __func__, clk_name, default_clock, width);
+ goto err_kfree;
+ }
+ } else {
+ default_clock = -1;
+ }
+
+ reg = of_iomap(node, 0);
+ if (!reg) {
+ pr_err("%s(%s): of_iomap failed\n",
+ __func__, clk_name);
+ goto err_kfree;
+ }
+
+ clk = clk_register_metag_mux(NULL, clk_name, parent_names, num_parents,
+ default_clock, 0, reg, shift,
+ width, flags);
+ if (IS_ERR(clk))
+ goto err_iounmap;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return;
+
+err_iounmap:
+ iounmap(reg);
+err_kfree:
+ kfree(parent_names);
+}
+CLK_OF_DECLARE(metag_mux_clk, "img,meta-mux-clock", of_metag_mux_clk_setup);
+#endif /* CONFIG_OF */
--
1.8.1.2

2013-05-10 15:03:19

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 1/2] clk: metag/clk-gate: add metag specific clock gate

Add a metag architecture specific clk-gate which extends the generic one
to use global lock2 to protect the register fields. It is common with
metag to have an RTOS running on a different thread or core with access
to different bits in the same register (in this case clock gate bits for
other clocks). Access to such registers must be serialised with a global
lock such as the one provided by the metag architecture port in
<asm/global_lock.h>

Signed-off-by: James Hogan <[email protected]>
Cc: Mike Turquette <[email protected]>
---
.../bindings/clock/img,meta-gate-clock.txt | 28 ++++
drivers/clk/Makefile | 1 +
drivers/clk/metag/Makefile | 2 +
drivers/clk/metag/clk-gate.c | 179 +++++++++++++++++++++
4 files changed, 210 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
create mode 100644 drivers/clk/metag/Makefile
create mode 100644 drivers/clk/metag/clk-gate.c

diff --git a/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt b/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
new file mode 100644
index 0000000..483097c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
@@ -0,0 +1,28 @@
+Binding for clock gate requiring global Meta locking.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Shall be "img,meta-mux-clock".
+- #clock-cells : From common clock binding; shall be set to 0.
+- reg : Address of configuration register.
+- bit : Bit number of gate switch in configuration register.
+- clocks : From common clock binding.
+
+Required source clocks:
+- 0 : Input clock that can be gated (doesn't have to be named).
+
+Optional properties:
+- clock-output-names : From common clock binding.
+
+Example:
+ clock {
+ compatible = "img,meta-gate-clock";
+ #clock-cells = <0>;
+ clocks = <&sys_clk>;
+ reg = <0x02004010 0x4>;
+ bit = <0>;
+ clock-output-names = "scb0";
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e7f7fe9..a800077 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/

+obj-$(CONFIG_METAG) += metag/
obj-$(CONFIG_X86) += x86/

# Chip specific
diff --git a/drivers/clk/metag/Makefile b/drivers/clk/metag/Makefile
new file mode 100644
index 0000000..8e9a6ac
--- /dev/null
+++ b/drivers/clk/metag/Makefile
@@ -0,0 +1,2 @@
+# metag clock types
+obj-$(CONFIG_COMMON_CLK) += clk-gate.o
diff --git a/drivers/clk/metag/clk-gate.c b/drivers/clk/metag/clk-gate.c
new file mode 100644
index 0000000..63da8d9
--- /dev/null
+++ b/drivers/clk/metag/clk-gate.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2013 Imagination Technologies Ltd.
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Metag gated clock implementation
+ * Based on gated clock implementation, but does appropriate locking to protect
+ * registers shared between hardware threads.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <asm/global_lock.h>
+
+/**
+ * struct clk_metag_gate - metag gating clock
+ *
+ * @mux: the parent class
+ * @ops: pointer to clk_ops of parent class
+ *
+ * Clock which can gate its output. Extends basic mux by using a global
+ * exclusive lock when read-modify-writing the mux field so that multiple
+ * threads/cores can use different fields in the same register.
+ */
+struct clk_metag_gate {
+ struct clk_gate gate;
+ const struct clk_ops *ops;
+};
+
+static inline struct clk_metag_gate *to_clk_metag_gate(struct clk_hw *hw)
+{
+ struct clk_gate *gate = container_of(hw, struct clk_gate, hw);
+
+ return container_of(gate, struct clk_metag_gate, gate);
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static int clk_metag_gate_enable(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+ int ret;
+ unsigned long flags;
+
+ __global_lock2(flags);
+ ret = gate->ops->enable(&gate->gate.hw);
+ __global_unlock2(flags);
+
+ return ret;
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static void clk_metag_gate_disable(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+ unsigned long flags;
+
+ __global_lock2(flags);
+ gate->ops->disable(&gate->gate.hw);
+ __global_unlock2(flags);
+}
+
+static int clk_metag_gate_is_enabled(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+
+ return gate->ops->is_enabled(&gate->gate.hw);
+}
+
+static const struct clk_ops clk_metag_gate_ops = {
+ .enable = clk_metag_gate_enable,
+ .disable = clk_metag_gate_disable,
+ .is_enabled = clk_metag_gate_is_enabled,
+};
+
+/**
+ * clk_register_metag_gate - register a Meta gate clock with the clock framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_name: name of this clock's parent
+ * @flags: framework-specific flags for this clock
+ * @reg: register address to control gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ */
+static struct clk *__init clk_register_metag_gate(struct device *dev,
+ const char *name, const char *parent_name, unsigned long flags,
+ void __iomem *reg, u8 bit_idx, u8 clk_gate_flags)
+{
+ struct clk_metag_gate *gate;
+ struct clk *clk;
+ struct clk_init_data init;
+
+ /* allocate the gate */
+ gate = kzalloc(sizeof(struct clk_metag_gate), GFP_KERNEL);
+ if (!gate) {
+ pr_err("%s: could not allocate gated clk\n", __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ init.name = name;
+ init.ops = &clk_metag_gate_ops;
+ init.flags = flags | CLK_IS_BASIC;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_gate assignments */
+ gate->gate.reg = reg;
+ gate->gate.bit_idx = bit_idx;
+ gate->gate.flags = clk_gate_flags;
+ gate->gate.hw.init = &init;
+
+ /* struct clk_metag_gate assignments */
+ gate->ops = &clk_gate_ops;
+
+ clk = clk_register(dev, &gate->gate.hw);
+
+ if (IS_ERR(clk))
+ kfree(gate);
+
+ return clk;
+}
+
+#ifdef CONFIG_OF
+/**
+ * of_metag_gate_clk_setup() - Setup function for simple fixed rate clock
+ */
+static void __init of_metag_gate_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ const char *clk_name = node->name;
+ u32 bit_idx;
+ void __iomem *reg;
+ const char *parent_name;
+ u8 flags = 0;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ if (of_property_read_u32(node, "bit", &bit_idx)) {
+ pr_err("%s(%s): could not read bit property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ parent_name = of_clk_get_parent_name(node, 0);
+ if (!parent_name) {
+ pr_err("%s(%s): could not read parent clock\n",
+ __func__, clk_name);
+ return;
+ }
+
+ reg = of_iomap(node, 0);
+ if (!reg) {
+ pr_err("%s(%s): of_iomap failed\n",
+ __func__, clk_name);
+ return;
+ }
+
+ clk = clk_register_metag_gate(NULL, clk_name, parent_name,
+ CLK_SET_RATE_PARENT, reg, bit_idx, flags);
+ if (IS_ERR(clk))
+ goto err_iounmap;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return;
+
+err_iounmap:
+ iounmap(reg);
+}
+CLK_OF_DECLARE(metag_gate_clk, "img,meta-gate-clock", of_metag_gate_clk_setup);
+#endif /* CONFIG_OF */
--
1.8.1.2

2013-05-10 15:03:17

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 2/2] clk: metag/clk-mux: add metag specific clk-mux

Add a metag architecture specific clk-mux which extends the generic one
to use global lock2 to protect the register fields. It is common with
metag to have an RTOS running on a different thread or core with access
to different bits in the same register (in this case clock switch bits
for other clocks). Access to such registers must be serialised with a
global lock such as the one provided by the metag architecture port in
<asm/global_lock.h>

Signed-off-by: James Hogan <[email protected]>
Cc: Mike Turquette <[email protected]>
---
.../bindings/clock/img,meta-mux-clock.txt | 33 ++++
drivers/clk/metag/Makefile | 1 +
drivers/clk/metag/clk-mux.c | 211 +++++++++++++++++++++
3 files changed, 245 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
create mode 100644 drivers/clk/metag/clk-mux.c

diff --git a/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt b/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
new file mode 100644
index 0000000..7802939
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
@@ -0,0 +1,33 @@
+Binding for clock multiplexer requiring global Meta locking.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Shall be "img,meta-mux-clock".
+- #clock-cells : From common clock binding; shall be set to 0.
+- reg : Address of configuration register.
+- shift : Shift of mux value field in configuration register.
+- width : Width of mux value field in configuration register.
+- clocks : From common clock binding.
+
+Required source clocks:
+- 0..(1<<width)-1 : Input clocks to multiplex (don't have to be named).
+
+Optional properties:
+- clock-output-names : From common clock binding.
+- default-clock : Mux value to set initially.
+
+Example:
+ clock {
+ compatible = "img,meta-mux-clock";
+ #clock-cells = <0>;
+ clocks = <&xtal1>,
+ <&xtal2>;
+ reg = <0x02005908 0x4>;
+ shift = <0>;
+ width = <1>;
+ clock-output-names = "sysclk0_sw";
+ default-clock = <1>; /* default to xtal2 */
+ };
diff --git a/drivers/clk/metag/Makefile b/drivers/clk/metag/Makefile
index 8e9a6ac..c53ccb2 100644
--- a/drivers/clk/metag/Makefile
+++ b/drivers/clk/metag/Makefile
@@ -1,2 +1,3 @@
# metag clock types
obj-$(CONFIG_COMMON_CLK) += clk-gate.o
+obj-$(CONFIG_COMMON_CLK) += clk-mux.o
diff --git a/drivers/clk/metag/clk-mux.c b/drivers/clk/metag/clk-mux.c
new file mode 100644
index 0000000..5afedde
--- /dev/null
+++ b/drivers/clk/metag/clk-mux.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright (C) 2013 Imagination Technologies Ltd.
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Metag simple multiplexer clock implementation
+ * Based on simple multiplexer clock implementation, but does appropriate
+ * locking to protect registers shared between hardware threads.
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <asm/global_lock.h>
+
+/**
+ * struct clk_metag_mux - metag multiplexer clock
+ *
+ * @mux: the parent class
+ * @ops: pointer to clk_ops of parent class
+ *
+ * Clock with multiple selectable parents. Extends basic mux by using a global
+ * exclusive lock when read-modify-writing the mux field so that multiple
+ * threads/cores can use different fields in the same register.
+ */
+struct clk_metag_mux {
+ struct clk_mux mux;
+ const struct clk_ops *ops;
+};
+
+static inline struct clk_metag_mux *to_clk_metag_mux(struct clk_hw *hw)
+{
+ struct clk_mux *mux = container_of(hw, struct clk_mux, hw);
+
+ return container_of(mux, struct clk_metag_mux, mux);
+}
+
+static u8 clk_metag_mux_get_parent(struct clk_hw *hw)
+{
+ struct clk_metag_mux *mux = to_clk_metag_mux(hw);
+
+ return mux->ops->get_parent(&mux->mux.hw);
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static int clk_metag_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+ struct clk_metag_mux *mux = to_clk_metag_mux(hw);
+ int ret;
+ unsigned long flags;
+
+ __global_lock2(flags);
+ ret = mux->ops->set_parent(&mux->mux.hw, index);
+ __global_unlock2(flags);
+
+ return ret;
+}
+
+static const struct clk_ops clk_metag_mux_ops = {
+ .get_parent = clk_metag_mux_get_parent,
+ .set_parent = clk_metag_mux_set_parent,
+};
+
+static struct clk *__init clk_register_metag_mux(struct device *dev,
+ const char *name, const char **parent_names, u8 num_parents,
+ s32 default_parent, unsigned long flags, void __iomem *reg,
+ u8 shift, u8 width, u8 clk_mux_flags)
+{
+ struct clk_metag_mux *mux;
+ struct clk *clk;
+ struct clk_init_data init;
+
+ /* allocate the mux */
+ mux = kzalloc(sizeof(struct clk_metag_mux), GFP_KERNEL);
+ if (!mux) {
+ pr_err("%s: could not allocate metag mux clk\n", __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ init.name = name;
+ init.ops = &clk_metag_mux_ops;
+ init.flags = flags | CLK_IS_BASIC;
+ init.parent_names = parent_names;
+ init.num_parents = num_parents;
+
+ /* struct clk_mux assignments */
+ mux->mux.reg = reg;
+ mux->mux.shift = shift;
+ mux->mux.mask = BIT(width) - 1;
+ mux->mux.flags = clk_mux_flags;
+ mux->mux.hw.init = &init;
+
+ /* struct clk_metag_mux assignments */
+ mux->ops = &clk_mux_ops;
+
+ /* set default value */
+ if (default_parent >= 0)
+ clk_metag_mux_set_parent(&mux->mux.hw, default_parent);
+
+ clk = clk_register(dev, &mux->mux.hw);
+
+ if (IS_ERR(clk))
+ kfree(mux);
+
+ return clk;
+}
+
+#ifdef CONFIG_OF
+/**
+ * of_metag_mux_clk_setup() - Setup function for simple fixed rate clock
+ */
+static void __init of_metag_mux_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ const char *clk_name = node->name;
+ u32 shift, width, default_clock;
+ void __iomem *reg;
+ int len, i;
+ struct property *prop;
+ const char **parent_names;
+ unsigned int num_parents;
+ u8 flags = 0;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ if (of_property_read_u32(node, "shift", &shift)) {
+ pr_err("%s(%s): could not read shift property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ if (of_property_read_u32(node, "width", &width)) {
+ pr_err("%s(%s): could not read width property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ /* count maximum number of parent clocks */
+ prop = of_find_property(node, "clocks", &len);
+ if (!prop) {
+ pr_err("%s(%s): could not find clocks property\n",
+ __func__, clk_name);
+ return;
+ }
+ /*
+ * There cannot be more parents than entries in "clocks" property (which
+ * may include additional args too. It also needs to fit in a u8.
+ */
+ num_parents = len / sizeof(u32);
+ num_parents = min(num_parents, 0xffu);
+
+ /* allocate an array of parent names */
+ parent_names = kzalloc(sizeof(const char *)*num_parents, GFP_KERNEL);
+ if (!parent_names) {
+ pr_err("%s(%s): could not allocate %u parent names\n",
+ __func__, clk_name, num_parents);
+ goto err_kfree;
+ }
+
+ /* fill in the parent names */
+ for (i = 0; i < num_parents; ++i) {
+ parent_names[i] = of_clk_get_parent_name(node, i);
+ if (!parent_names[i]) {
+ /* truncate array length if we hit the end early */
+ num_parents = i;
+ break;
+ }
+ }
+
+ /* default parent clock (mux value) */
+ if (!of_property_read_u32(node, "default-clock", &default_clock)) {
+ if (default_clock >= num_parents) {
+ pr_err("%s(%s): default-clock %u out of range (%u bits)\n",
+ __func__, clk_name, default_clock, width);
+ goto err_kfree;
+ }
+ } else {
+ default_clock = -1;
+ }
+
+ reg = of_iomap(node, 0);
+ if (!reg) {
+ pr_err("%s(%s): of_iomap failed\n",
+ __func__, clk_name);
+ goto err_kfree;
+ }
+
+ clk = clk_register_metag_mux(NULL, clk_name, parent_names, num_parents,
+ default_clock, 0, reg, shift,
+ width, flags);
+ if (IS_ERR(clk))
+ goto err_iounmap;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return;
+
+err_iounmap:
+ iounmap(reg);
+err_kfree:
+ kfree(parent_names);
+}
+CLK_OF_DECLARE(metag_mux_clk, "img,meta-mux-clock", of_metag_mux_clk_setup);
+#endif /* CONFIG_OF */
--
1.8.1.2

2013-05-10 15:03:21

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 1/2] clk: metag/clk-gate: add metag specific clock gate

Add a metag architecture specific clk-gate which extends the generic one
to use global lock2 to protect the register fields. It is common with
metag to have an RTOS running on a different thread or core with access
to different bits in the same register (in this case clock gate bits for
other clocks). Access to such registers must be serialised with a global
lock such as the one provided by the metag architecture port in
<asm/global_lock.h>

Signed-off-by: James Hogan <[email protected]>
Cc: Mike Turquette <[email protected]>
---
.../bindings/clock/img,meta-gate-clock.txt | 28 ++++
drivers/clk/Makefile | 1 +
drivers/clk/metag/Makefile | 2 +
drivers/clk/metag/clk-gate.c | 179 +++++++++++++++++++++
4 files changed, 210 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
create mode 100644 drivers/clk/metag/Makefile
create mode 100644 drivers/clk/metag/clk-gate.c

diff --git a/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt b/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
new file mode 100644
index 0000000..483097c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
@@ -0,0 +1,28 @@
+Binding for clock gate requiring global Meta locking.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Shall be "img,meta-mux-clock".
+- #clock-cells : From common clock binding; shall be set to 0.
+- reg : Address of configuration register.
+- bit : Bit number of gate switch in configuration register.
+- clocks : From common clock binding.
+
+Required source clocks:
+- 0 : Input clock that can be gated (doesn't have to be named).
+
+Optional properties:
+- clock-output-names : From common clock binding.
+
+Example:
+ clock {
+ compatible = "img,meta-gate-clock";
+ #clock-cells = <0>;
+ clocks = <&sys_clk>;
+ reg = <0x02004010 0x4>;
+ bit = <0>;
+ clock-output-names = "scb0";
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e7f7fe9..a800077 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/

+obj-$(CONFIG_METAG) += metag/
obj-$(CONFIG_X86) += x86/

# Chip specific
diff --git a/drivers/clk/metag/Makefile b/drivers/clk/metag/Makefile
new file mode 100644
index 0000000..8e9a6ac
--- /dev/null
+++ b/drivers/clk/metag/Makefile
@@ -0,0 +1,2 @@
+# metag clock types
+obj-$(CONFIG_COMMON_CLK) += clk-gate.o
diff --git a/drivers/clk/metag/clk-gate.c b/drivers/clk/metag/clk-gate.c
new file mode 100644
index 0000000..63da8d9
--- /dev/null
+++ b/drivers/clk/metag/clk-gate.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2013 Imagination Technologies Ltd.
+ *
+ * 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
+ * published by the Free Software Foundation.
+ *
+ * Metag gated clock implementation
+ * Based on gated clock implementation, but does appropriate locking to protect
+ * registers shared between hardware threads.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <asm/global_lock.h>
+
+/**
+ * struct clk_metag_gate - metag gating clock
+ *
+ * @mux: the parent class
+ * @ops: pointer to clk_ops of parent class
+ *
+ * Clock which can gate its output. Extends basic mux by using a global
+ * exclusive lock when read-modify-writing the mux field so that multiple
+ * threads/cores can use different fields in the same register.
+ */
+struct clk_metag_gate {
+ struct clk_gate gate;
+ const struct clk_ops *ops;
+};
+
+static inline struct clk_metag_gate *to_clk_metag_gate(struct clk_hw *hw)
+{
+ struct clk_gate *gate = container_of(hw, struct clk_gate, hw);
+
+ return container_of(gate, struct clk_metag_gate, gate);
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static int clk_metag_gate_enable(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+ int ret;
+ unsigned long flags;
+
+ __global_lock2(flags);
+ ret = gate->ops->enable(&gate->gate.hw);
+ __global_unlock2(flags);
+
+ return ret;
+}
+
+/* Acquire exclusive lock since other cores may access the same register */
+static void clk_metag_gate_disable(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+ unsigned long flags;
+
+ __global_lock2(flags);
+ gate->ops->disable(&gate->gate.hw);
+ __global_unlock2(flags);
+}
+
+static int clk_metag_gate_is_enabled(struct clk_hw *hw)
+{
+ struct clk_metag_gate *gate = to_clk_metag_gate(hw);
+
+ return gate->ops->is_enabled(&gate->gate.hw);
+}
+
+static const struct clk_ops clk_metag_gate_ops = {
+ .enable = clk_metag_gate_enable,
+ .disable = clk_metag_gate_disable,
+ .is_enabled = clk_metag_gate_is_enabled,
+};
+
+/**
+ * clk_register_metag_gate - register a Meta gate clock with the clock framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_name: name of this clock's parent
+ * @flags: framework-specific flags for this clock
+ * @reg: register address to control gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ */
+static struct clk *__init clk_register_metag_gate(struct device *dev,
+ const char *name, const char *parent_name, unsigned long flags,
+ void __iomem *reg, u8 bit_idx, u8 clk_gate_flags)
+{
+ struct clk_metag_gate *gate;
+ struct clk *clk;
+ struct clk_init_data init;
+
+ /* allocate the gate */
+ gate = kzalloc(sizeof(struct clk_metag_gate), GFP_KERNEL);
+ if (!gate) {
+ pr_err("%s: could not allocate gated clk\n", __func__);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ init.name = name;
+ init.ops = &clk_metag_gate_ops;
+ init.flags = flags | CLK_IS_BASIC;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_gate assignments */
+ gate->gate.reg = reg;
+ gate->gate.bit_idx = bit_idx;
+ gate->gate.flags = clk_gate_flags;
+ gate->gate.hw.init = &init;
+
+ /* struct clk_metag_gate assignments */
+ gate->ops = &clk_gate_ops;
+
+ clk = clk_register(dev, &gate->gate.hw);
+
+ if (IS_ERR(clk))
+ kfree(gate);
+
+ return clk;
+}
+
+#ifdef CONFIG_OF
+/**
+ * of_metag_gate_clk_setup() - Setup function for simple fixed rate clock
+ */
+static void __init of_metag_gate_clk_setup(struct device_node *node)
+{
+ struct clk *clk;
+ const char *clk_name = node->name;
+ u32 bit_idx;
+ void __iomem *reg;
+ const char *parent_name;
+ u8 flags = 0;
+
+ of_property_read_string(node, "clock-output-names", &clk_name);
+
+ if (of_property_read_u32(node, "bit", &bit_idx)) {
+ pr_err("%s(%s): could not read bit property\n",
+ __func__, clk_name);
+ return;
+ }
+
+ parent_name = of_clk_get_parent_name(node, 0);
+ if (!parent_name) {
+ pr_err("%s(%s): could not read parent clock\n",
+ __func__, clk_name);
+ return;
+ }
+
+ reg = of_iomap(node, 0);
+ if (!reg) {
+ pr_err("%s(%s): of_iomap failed\n",
+ __func__, clk_name);
+ return;
+ }
+
+ clk = clk_register_metag_gate(NULL, clk_name, parent_name,
+ CLK_SET_RATE_PARENT, reg, bit_idx, flags);
+ if (IS_ERR(clk))
+ goto err_iounmap;
+
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return;
+
+err_iounmap:
+ iounmap(reg);
+}
+CLK_OF_DECLARE(metag_gate_clk, "img,meta-gate-clock", of_metag_gate_clk_setup);
+#endif /* CONFIG_OF */
--
1.8.1.2

2013-05-10 15:04:30

by James Hogan

[permalink] [raw]
Subject: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

This adds a metag architecture specific clk-gate and clk-mux which
extends the generic ones to use global lock2 to protect the register
fields. It is common with metag to have an RTOS running on a different
thread or core with access to different bits in the same register (which
contain clock gate/switch bits for other clocks). Access to such
registers must be serialised with a global lock such as the one provided
by the metag architecture port in <asm/global_lock.h>

RFC because despite extending the generic clocks there's still a bit of
duplicated code necessary. One alternative is to add special cases to
the generic clock components for when a global or callback function
based lock is desired instead of a spinlock, but I wasn't sure if that
sort of hack would really be appreciated in the generic drivers.

Comments?

James Hogan (2):
clk: metag/clk-gate: add metag specific clock gate
clk: metag/clk-mux: add metag specific clk-mux

.../bindings/clock/img,meta-gate-clock.txt | 28 +++
.../bindings/clock/img,meta-mux-clock.txt | 33 ++++
drivers/clk/Makefile | 1 +
drivers/clk/metag/Makefile | 3 +
drivers/clk/metag/clk-gate.c | 179 +++++++++++++++++
drivers/clk/metag/clk-mux.c | 211 +++++++++++++++++++++
6 files changed, 455 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
create mode 100644 Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
create mode 100644 drivers/clk/metag/Makefile
create mode 100644 drivers/clk/metag/clk-gate.c
create mode 100644 drivers/clk/metag/clk-mux.c

--
1.8.1.2

2013-05-10 15:10:11

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

On 10/05/13 16:02, James Hogan wrote:
> This adds a metag architecture specific clk-gate and clk-mux which
> extends the generic ones to use global lock2 to protect the register
> fields. It is common with metag to have an RTOS running on a different
> thread or core with access to different bits in the same register (which
> contain clock gate/switch bits for other clocks). Access to such
> registers must be serialised with a global lock such as the one provided
> by the metag architecture port in <asm/global_lock.h>

Hmm, sorry about the duplicated emails, I had *.patch twice in my git
send-email command line :(.

2013-05-15 22:31:26

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

On 05/10/13 08:02, James Hogan wrote:
> This adds a metag architecture specific clk-gate and clk-mux which
> extends the generic ones to use global lock2 to protect the register
> fields. It is common with metag to have an RTOS running on a different
> thread or core with access to different bits in the same register (which
> contain clock gate/switch bits for other clocks). Access to such
> registers must be serialised with a global lock such as the one provided
> by the metag architecture port in <asm/global_lock.h>
>
> RFC because despite extending the generic clocks there's still a bit of
> duplicated code necessary. One alternative is to add special cases to
> the generic clock components for when a global or callback function
> based lock is desired instead of a spinlock, but I wasn't sure if that
> sort of hack would really be appreciated in the generic drivers.
>
> Comments?

Can you please Cc the devicetree mailing list when proposing new bindings?

Your patchset brings up a question I've had which is if we should be
putting the bits and register width information in devicetree at all. On
the one hand it's nice to not have anything in C code, just iterate over
nodes and register clocks. On the other hand, it's the first time I've
seen anyone put the register interface into devicetree. From what I can
tell, the regulator bindings have put at most the register base and
physical properties like enable-time, max voltage, etc., but not what
bits are needed to enable/disable a regulator. Also I thought I read
somewhere that reg properties shouldn't overlap each other, so if you
ever have two clocks living in the same register we're going to violate
that.

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

2013-05-16 09:58:13

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

On 15/05/13 23:31, Stephen Boyd wrote:
> On 05/10/13 08:02, James Hogan wrote:
>> This adds a metag architecture specific clk-gate and clk-mux which
>> extends the generic ones to use global lock2 to protect the register
>> fields. It is common with metag to have an RTOS running on a different
>> thread or core with access to different bits in the same register (which
>> contain clock gate/switch bits for other clocks). Access to such
>> registers must be serialised with a global lock such as the one provided
>> by the metag architecture port in <asm/global_lock.h>
>>
>> RFC because despite extending the generic clocks there's still a bit of
>> duplicated code necessary. One alternative is to add special cases to
>> the generic clock components for when a global or callback function
>> based lock is desired instead of a spinlock, but I wasn't sure if that
>> sort of hack would really be appreciated in the generic drivers.
>>
>> Comments?
>
> Can you please Cc the devicetree mailing list when proposing new bindings?

Erm, I think it was on Cc ([email protected] yeh?)

> Your patchset brings up a question I've had which is if we should be
> putting the bits and register width information in devicetree at all. On
> the one hand it's nice to not have anything in C code, just iterate over
> nodes and register clocks. On the other hand, it's the first time I've
> seen anyone put the register interface into devicetree. From what I can
> tell, the regulator bindings have put at most the register base and
> physical properties like enable-time, max voltage, etc., but not what
> bits are needed to enable/disable a regulator. Also I thought I read
> somewhere that reg properties shouldn't overlap each other, so if you
> ever have two clocks living in the same register we're going to violate
> that.

Oh, I wasn't aware of that limitation.

The SoC I'm working with has some registers full of clock enable bits (I
guess one could have a gate array component with up to 32 clock inputs
and outputs) and some registers full of clock mux switch bits (that
would get really messy to define as a block since each bit switches
between 2 parents, and some of the parents are other clock muxes in the
same block). Some registers contain a bunch of low power related bits
together, including clock enable bits in the same register as various
pinconf settings which is used by a separate pinctrl driver.

Cheers
James

2013-05-16 22:22:38

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

On 05/16/13 02:56, James Hogan wrote:
> On 15/05/13 23:31, Stephen Boyd wrote:
>> On 05/10/13 08:02, James Hogan wrote:
>>> This adds a metag architecture specific clk-gate and clk-mux which
>>> extends the generic ones to use global lock2 to protect the register
>>> fields. It is common with metag to have an RTOS running on a different
>>> thread or core with access to different bits in the same register (which
>>> contain clock gate/switch bits for other clocks). Access to such
>>> registers must be serialised with a global lock such as the one provided
>>> by the metag architecture port in <asm/global_lock.h>
>>>
>>> RFC because despite extending the generic clocks there's still a bit of
>>> duplicated code necessary. One alternative is to add special cases to
>>> the generic clock components for when a global or callback function
>>> based lock is desired instead of a spinlock, but I wasn't sure if that
>>> sort of hack would really be appreciated in the generic drivers.
>>>
>>> Comments?
>> Can you please Cc the devicetree mailing list when proposing new bindings?
> Erm, I think it was on Cc ([email protected] yeh?)

I added them in my reply.

>
>> Your patchset brings up a question I've had which is if we should be
>> putting the bits and register width information in devicetree at all. On
>> the one hand it's nice to not have anything in C code, just iterate over
>> nodes and register clocks. On the other hand, it's the first time I've
>> seen anyone put the register interface into devicetree. From what I can
>> tell, the regulator bindings have put at most the register base and
>> physical properties like enable-time, max voltage, etc., but not what
>> bits are needed to enable/disable a regulator. Also I thought I read
>> somewhere that reg properties shouldn't overlap each other, so if you
>> ever have two clocks living in the same register we're going to violate
>> that.
> Oh, I wasn't aware of that limitation.
>
> The SoC I'm working with has some registers full of clock enable bits (I
> guess one could have a gate array component with up to 32 clock inputs
> and outputs) and some registers full of clock mux switch bits (that
> would get really messy to define as a block since each bit switches
> between 2 parents, and some of the parents are other clock muxes in the
> same block). Some registers contain a bunch of low power related bits
> together, including clock enable bits in the same register as various
> pinconf settings which is used by a separate pinctrl driver.
>

I have similar hardware and so I would like to hear what the devicetree
knowledgeable people think about it. Hopefully Rob or Grant can shed
some light here.

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

2013-05-17 08:20:08

by James Hogan

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

On 16/05/13 23:22, Stephen Boyd wrote:
> On 05/16/13 02:56, James Hogan wrote:
>> On 15/05/13 23:31, Stephen Boyd wrote:
>>> Can you please Cc the devicetree mailing list when proposing new bindings?
>> Erm, I think it was on Cc ([email protected] yeh?)
>
> I added them in my reply.

Ah yes, sorry, my mistake.

Cheers
James

2013-05-29 17:58:38

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

Quoting Stephen Boyd (2013-05-15 15:31:11)
> On 05/10/13 08:02, James Hogan wrote:
> > This adds a metag architecture specific clk-gate and clk-mux which
> > extends the generic ones to use global lock2 to protect the register
> > fields. It is common with metag to have an RTOS running on a different
> > thread or core with access to different bits in the same register (which
> > contain clock gate/switch bits for other clocks). Access to such
> > registers must be serialised with a global lock such as the one provided
> > by the metag architecture port in <asm/global_lock.h>
> >
> > RFC because despite extending the generic clocks there's still a bit of
> > duplicated code necessary. One alternative is to add special cases to
> > the generic clock components for when a global or callback function
> > based lock is desired instead of a spinlock, but I wasn't sure if that
> > sort of hack would really be appreciated in the generic drivers.
> >
> > Comments?
>
> Can you please Cc the devicetree mailing list when proposing new bindings?
>
> Your patchset brings up a question I've had which is if we should be
> putting the bits and register width information in devicetree at all. On
> the one hand it's nice to not have anything in C code, just iterate over
> nodes and register clocks. On the other hand, it's the first time I've
> seen anyone put the register interface into devicetree. From what I can
> tell, the regulator bindings have put at most the register base and
> physical properties like enable-time, max voltage, etc., but not what
> bits are needed to enable/disable a regulator. Also I thought I read
> somewhere that reg properties shouldn't overlap each other, so if you
> ever have two clocks living in the same register we're going to violate
> that.
>

I've written bindings for the generic mux-clock, divider-clock, and I'm
working on a gate-clock (reusing some RFCs from a while back for
gate-clock). All of these specify the base address as well as the
relevant bit fields. To me this seems to intuitively describe the
hardware. I'll post these bindings soon and let the bike shedding
begin.

Regards,
Mike

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

2013-05-29 18:38:24

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH RFC 0/2] clk: add metag specific gate/mux clocks

Quoting James Hogan (2013-05-10 08:02:02)
> This adds a metag architecture specific clk-gate and clk-mux which
> extends the generic ones to use global lock2 to protect the register
> fields. It is common with metag to have an RTOS running on a different
> thread or core with access to different bits in the same register (which
> contain clock gate/switch bits for other clocks). Access to such
> registers must be serialised with a global lock such as the one provided
> by the metag architecture port in <asm/global_lock.h>
>
> RFC because despite extending the generic clocks there's still a bit of
> duplicated code necessary. One alternative is to add special cases to
> the generic clock components for when a global or callback function
> based lock is desired instead of a spinlock, but I wasn't sure if that
> sort of hack would really be appreciated in the generic drivers.
>
> Comments?
>

The general approach looks OK, but you've kind of created generic
mux-clock and gate-clock bindings in a metag-specific driver. I would
prefer to have those functions exist in the generic drivers and your
driver re-use those. I'll be posting bindings for those clocks by the
end of the week (they're on my github repo now for the curious).

Wrapping the generic clock types is an OK approach and is done by
several other drivers, so there is no problem there.

Regards,
Mike

> James Hogan (2):
> clk: metag/clk-gate: add metag specific clock gate
> clk: metag/clk-mux: add metag specific clk-mux
>
> .../bindings/clock/img,meta-gate-clock.txt | 28 +++
> .../bindings/clock/img,meta-mux-clock.txt | 33 ++++
> drivers/clk/Makefile | 1 +
> drivers/clk/metag/Makefile | 3 +
> drivers/clk/metag/clk-gate.c | 179 +++++++++++++++++
> drivers/clk/metag/clk-mux.c | 211 +++++++++++++++++++++
> 6 files changed, 455 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/img,meta-gate-clock.txt
> create mode 100644 Documentation/devicetree/bindings/clock/img,meta-mux-clock.txt
> create mode 100644 drivers/clk/metag/Makefile
> create mode 100644 drivers/clk/metag/clk-gate.c
> create mode 100644 drivers/clk/metag/clk-mux.c
>
> --
> 1.8.1.2