Re: [PATCH v4 5/6] clk: basic gateable and fixed-rate clks

2011-12-16 Thread Turquette, Mike
On Tue, Dec 13, 2011 at 9:15 PM, Ryan Mallon rmal...@gmail.com wrote:
 On 14/12/11 14:53, Mike Turquette wrote:

 Many platforms support simple gateable clks and fixed-rate clks that
 should not be re-implemented by every platform.

 This patch introduces a gateable clk with a common programming model of
 gate control via a write of 1 bit to a register.  Both set-to-enable and
 clear-to-enable are supported.

 Also introduced is a fixed-rate clk which has no reprogrammable aspects.

 The purpose of both types of clks is documented in drivers/clk/basic.c.

 TODO: add support for a simple divider, simple mux and a dummy clk for
 stubbing out platform support.

 Based on original patch by Jeremy Kerr and contribution by Jamie Iles.

 Signed-off-by: Mike Turquette mturque...@linaro.org
 Cc: Jeremy Kerr jeremy.k...@canonical.com
 Cc: Jamie Iles ja...@jamieiles.com

 snip

 +int clk_register_gate(struct device *dev, const char *name, unsigned long 
 flags,
 +             struct clk *fixed_parent, void __iomem *reg, u8 bit_idx,
 +             int set_to_enable)
 +{
 +     struct clk_hw_gate *gclk;
 +     struct clk *clk;
 +
 +     gclk = kmalloc(sizeof(struct clk_hw_gate), GFP_KERNEL);
 +
 +     if (!gclk) {
 +             pr_err(%s: could not allocate gated clk\n, __func__);
 +             return -ENOMEM;
 +     }
 +
 +     clk = gclk-clk;
 +
 +     /* struct clk_hw_gate assignments */
 +     gclk-fixed_parent = fixed_parent;
 +     gclk-reg = reg;
 +     gclk-bit_idx = bit_idx;
 +
 +     /* struct clk assignments */
 +     clk-name = name;
 +     clk-flags = flags;
 +
 +     if (set_to_enable)
 +             clk-ops = clk_hw_gate_set_enable_ops;
 +     else
 +             clk-ops = clk_hw_gate_set_disable_ops;


 You could avoid having two sets of operations if you stored the
 set_to_enable value in struct clk_hw_gate. It might be useful to store
 additional information in struct clk_hw_gate if you also want to extend
 to supporting non-32bit registers (readb, etc), clocks with write only
 registers, or support clocks which require more than one bit to be set
 or cleared to enable them, etc. See the basic mmio gpio driver for a
 similar case.

I haven't given the basic clks enough attention, mostly because I
haven't started using them yet in my own platform's conversion to
common struct clk :-/

I think the original reason for the extra operations is to avoid the
conditional in the enable/disable path, but if we're going to end up
adding other conditionals anyways (such as the register locking in the
iMX platform implementation) then we should probably forget about that
approach and just deal with the complexity in one set of common
enable/disable functions.

Thanks for the review,
Mike


 ~Ryan

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 5/6] clk: basic gateable and fixed-rate clks

2011-12-13 Thread Mike Turquette
Many platforms support simple gateable clks and fixed-rate clks that
should not be re-implemented by every platform.

This patch introduces a gateable clk with a common programming model of
gate control via a write of 1 bit to a register.  Both set-to-enable and
clear-to-enable are supported.

Also introduced is a fixed-rate clk which has no reprogrammable aspects.

The purpose of both types of clks is documented in drivers/clk/basic.c.

TODO: add support for a simple divider, simple mux and a dummy clk for
stubbing out platform support.

Based on original patch by Jeremy Kerr and contribution by Jamie Iles.

Signed-off-by: Mike Turquette mturque...@linaro.org
Cc: Jeremy Kerr jeremy.k...@canonical.com
Cc: Jamie Iles ja...@jamieiles.com
---
 drivers/clk/Kconfig |7 ++
 drivers/clk/Makefile|5 +-
 drivers/clk/clk-basic.c |  208 +++
 include/linux/clk.h |   35 
 4 files changed, 253 insertions(+), 2 deletions(-)
 create mode 100644 drivers/clk/clk-basic.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index adc0586..ba7eb8c 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -12,3 +12,10 @@ config HAVE_CLK_PREPARE
 config GENERIC_CLK
bool
select HAVE_CLK_PREPARE
+
+config GENERIC_CLK_BASIC
+   bool Basic clock definitions
+   depends on GENERIC_CLK
+   help
+  Allow use of basic, single-function clock types.  These
+  common definitions can be used across many platforms.
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 570d5b9..68b20a1 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -1,3 +1,4 @@
 
-obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o
-obj-$(CONFIG_GENERIC_CLK)  += clk.o
+obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o
+obj-$(CONFIG_GENERIC_CLK)  += clk.o
+obj-$(CONFIG_GENERIC_CLK_BASIC)+= clk-basic.o
diff --git a/drivers/clk/clk-basic.c b/drivers/clk/clk-basic.c
new file mode 100644
index 000..a065bc8
--- /dev/null
+++ b/drivers/clk/clk-basic.c
@@ -0,0 +1,208 @@
+/*
+ * Copyright (C) 2010-2011 Canonical Ltd jeremy.k...@canonical.com
+ * Copyright (C) 2011 Linaro Ltd mturque...@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
+ * published by the Free Software Foundation.
+ *
+ * Basic single-function clk implementations
+ */
+
+/* TODO add basic divider clk, basic mux clk and dummy clk */
+
+#include linux/clk.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/io.h
+
+/*
+ * NOTE: all of the basic clks here are just that: single-function
+ * simple clks.  One assumption in their implementation is that existing
+ * locking mechanisms (prepare_mutex and enable_spinlock) are enough to
+ * prevent race conditions during register accesses.  If this is not
+ * true for your platform then please implement your own version of
+ * these clks which take such issues into account.
+ */
+
+#define to_clk_hw_gate(ck) container_of(ck, struct clk_hw_gate, clk)
+#define to_clk_hw_fixed(ck) container_of(ck, struct clk_hw_fixed, clk)
+
+/**
+ * DOC: basic gatable clock which can gate and ungate it's ouput
+ *
+ * Traits of this clock:
+ * prepare - clk_prepare  clk_unprepare do nothing
+ * enable - clk_enable and clk_disable are functional  control gating
+ * rate - inherits rate from parent.  No clk_set_rate support
+ * parent - fixed parent.  No clk_set_parent support
+ *
+ * note: parent should not be NULL for this clock, but we check because we're
+ * paranoid
+ */
+
+static unsigned long clk_hw_gate_recalc_rate(struct clk *clk)
+{
+   if (clk-parent)
+   return clk-parent-rate;
+   else
+   return 0;
+}
+
+static struct clk *clk_hw_gate_get_parent(struct clk *clk)
+{
+   return to_clk_hw_gate(clk)-fixed_parent;
+}
+
+static void clk_hw_gate_set_bit(struct clk *clk)
+{
+   struct clk_hw_gate *gate = to_clk_hw_gate(clk);
+   u32 reg;
+
+   reg = readl(gate-reg);
+   reg |= BIT(gate-bit_idx);
+   writel(reg, gate-reg);
+}
+
+static void clk_hw_gate_clear_bit(struct clk *clk)
+{
+   struct clk_hw_gate *gate = to_clk_hw_gate(clk);
+   u32 reg;
+
+   reg = readl(gate-reg);
+   reg = ~BIT(gate-bit_idx);
+   writel(reg, gate-reg);
+}
+
+static int clk_hw_gate_enable_set(struct clk *clk)
+{
+   clk_hw_gate_set_bit(clk);
+
+   return 0;
+}
+
+static void clk_hw_gate_disable_clear(struct clk *clk)
+{
+   clk_hw_gate_clear_bit(clk);
+}
+
+struct clk_hw_ops clk_hw_gate_set_enable_ops = {
+   .enable = clk_hw_gate_enable_set,
+   .disable = clk_hw_gate_disable_clear,
+   .recalc_rate = clk_hw_gate_recalc_rate,
+   .get_parent = clk_hw_gate_get_parent,
+};
+EXPORT_SYMBOL_GPL(clk_hw_gate_set_enable_ops);
+
+static int clk_hw_gate_enable_clear(struct clk *clk)
+{
+   clk_hw_gate_clear_bit(clk);
+
+   

Re: [PATCH v4 5/6] clk: basic gateable and fixed-rate clks

2011-12-13 Thread Ryan Mallon
On 14/12/11 14:53, Mike Turquette wrote:

 Many platforms support simple gateable clks and fixed-rate clks that
 should not be re-implemented by every platform.
 
 This patch introduces a gateable clk with a common programming model of
 gate control via a write of 1 bit to a register.  Both set-to-enable and
 clear-to-enable are supported.
 
 Also introduced is a fixed-rate clk which has no reprogrammable aspects.
 
 The purpose of both types of clks is documented in drivers/clk/basic.c.
 
 TODO: add support for a simple divider, simple mux and a dummy clk for
 stubbing out platform support.
 
 Based on original patch by Jeremy Kerr and contribution by Jamie Iles.
 
 Signed-off-by: Mike Turquette mturque...@linaro.org
 Cc: Jeremy Kerr jeremy.k...@canonical.com
 Cc: Jamie Iles ja...@jamieiles.com

snip

 +int clk_register_gate(struct device *dev, const char *name, unsigned long 
 flags,
 + struct clk *fixed_parent, void __iomem *reg, u8 bit_idx,
 + int set_to_enable)
 +{
 + struct clk_hw_gate *gclk;
 + struct clk *clk;
 +
 + gclk = kmalloc(sizeof(struct clk_hw_gate), GFP_KERNEL);
 +
 + if (!gclk) {
 + pr_err(%s: could not allocate gated clk\n, __func__);
 + return -ENOMEM;
 + }
 +
 + clk = gclk-clk;
 +
 + /* struct clk_hw_gate assignments */
 + gclk-fixed_parent = fixed_parent;
 + gclk-reg = reg;
 + gclk-bit_idx = bit_idx;
 +
 + /* struct clk assignments */
 + clk-name = name;
 + clk-flags = flags;
 +
 + if (set_to_enable)
 + clk-ops = clk_hw_gate_set_enable_ops;
 + else
 + clk-ops = clk_hw_gate_set_disable_ops;


You could avoid having two sets of operations if you stored the
set_to_enable value in struct clk_hw_gate. It might be useful to store
additional information in struct clk_hw_gate if you also want to extend
to supporting non-32bit registers (readb, etc), clocks with write only
registers, or support clocks which require more than one bit to be set
or cleared to enable them, etc. See the basic mmio gpio driver for a
similar case.

~Ryan

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html