OMAP2+ clock: revise omap2_clk_{disable,enable}()

Simplify the code in the omap2_clk_disable() and omap2_clk_enable()
functions, reducing levels of indentation.  This makes the code easier
to read.  Add some additional debugging pr_debug()s here also to help
others understand what is going on.

Revise the omap2_clk_disable() logic so that it now attempts to
disable the clock's clockdomain before recursing up the clock tree.
Simultaneously, ensure that omap2_clk_enable() is called on parent
clocks first, before enabling the clockdomain.  This ensures that a
parent clock's clockdomain is enabled before the child clock's
clockdomain.  These sequences should be the inverse of each other.

Revise the omap2_clk_enable() logic so that it now cleans up after
itself upon encountering an error.  Previously, an error enabling a
parent clock could have resulted in inconsistent usecounts on the
enclosing clockdomain.

Remove the trivial _omap2_clk_disable() and _omap2_clk_enable() static
functions, and replace it with the clkops calls that they were
executing.

For all this to work, the clockdomain omap2_clkdm_clk_enable() and
omap2_clkdm_clk_disable() code must not return an error on clockdomains
without CLKSTCTRL registers; so modify those functions to simply return 0
in that case.

While here, add some basic kerneldoc documentation on both functions,
and get rid of some old non-CodingStyle-compliant comments that have
existed since the dawn of time (at least, the OMAP clock framework's
time).

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
Cc: Rajendra Nayak <rnayak@ti.com>

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index d1f115d0edadb871b5132fd6a3bd4ba56e705e0e..a6d0b34b799091cd1f0299225d60f8c181db204a 100644 (file)
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -37,9 +37,9 @@
 
 u8 cpu_mask;
 
-/*-------------------------------------------------------------------------
- * OMAP2/3/4 specific clock functions
- *-------------------------------------------------------------------------*/
+/*
+ * OMAP2+ specific clock functions
+ */
 
 /* Private functions */
 
@@ -71,20 +71,6 @@ static void _omap2_module_wait_ready(struct clk *clk)
                             clk->name);
 }
 
-/* Enables clock without considering parent dependencies or use count
- * REVISIT: Maybe change this to use clk->enable like on omap1?
- */
-static int _omap2_clk_enable(struct clk *clk)
-{
-       return clk->ops->enable(clk);
-}
-
-/* Disables clock without considering parent dependencies or use count */
-static void _omap2_clk_disable(struct clk *clk)
-{
-       clk->ops->disable(clk);
-}
-
 /* Public functions */
 
 /**
@@ -245,46 +231,106 @@ const struct clkops clkops_omap2_dflt = {
        .disable        = omap2_dflt_clk_disable,
 };
 
+/**
+ * omap2_clk_disable - disable a clock, if the system is not using it
+ * @clk: struct clk * to disable
+ *
+ * Decrements the usecount on struct clk @clk.  If there are no users
+ * left, call the clkops-specific clock disable function to disable it
+ * in hardware.  If the clock is part of a clockdomain (which they all
+ * should be), request that the clockdomain be disabled.  (It too has
+ * a usecount, and so will not be disabled in the hardware until it no
+ * longer has any users.)  If the clock has a parent clock (most of
+ * them do), then call ourselves, recursing on the parent clock.  This
+ * can cause an entire branch of the clock tree to be powered off by
+ * simply disabling one clock.  Intended to be called with the clockfw_lock
+ * spinlock held.  No return value.
+ */
 void omap2_clk_disable(struct clk *clk)
 {
-       if (clk->usecount > 0 && !(--clk->usecount)) {
-               _omap2_clk_disable(clk);
-               if (clk->parent)
-                       omap2_clk_disable(clk->parent);
-               if (clk->clkdm)
-                       omap2_clkdm_clk_disable(clk->clkdm, clk);
-
+       if (clk->usecount == 0) {
+               WARN(1, "clock: %s: omap2_clk_disable() called, but usecount "
+                    "already 0?", clk->name);
+               return;
        }
+
+       pr_debug("clock: %s: decrementing usecount\n", clk->name);
+
+       clk->usecount--;
+
+       if (clk->usecount > 0)
+               return;
+
+       pr_debug("clock: %s: disabling in hardware\n", clk->name);
+
+       clk->ops->disable(clk);
+
+       if (clk->clkdm)
+               omap2_clkdm_clk_disable(clk->clkdm, clk);
+
+       if (clk->parent)
+               omap2_clk_disable(clk->parent);
 }
 
+/**
+ * omap2_clk_enable - request that the system enable a clock
+ * @clk: struct clk * to enable
+ *
+ * Increments the usecount on struct clk @clk.  If there were no users
+ * previously, then recurse up the clock tree, enabling all of the
+ * clock's parents and all of the parent clockdomains, and finally,
+ * enabling @clk's clockdomain, and @clk itself.  Intended to be
+ * called with the clockfw_lock spinlock held.  Returns 0 upon success
+ * or a negative error code upon failure.
+ */
 int omap2_clk_enable(struct clk *clk)
 {
-       int ret = 0;
+       int ret;
 
-       if (clk->usecount++ == 0) {
-               if (clk->clkdm)
-                       omap2_clkdm_clk_enable(clk->clkdm, clk);
+       pr_debug("clock: %s: incrementing usecount\n", clk->name);
 
-               if (clk->parent) {
-                       ret = omap2_clk_enable(clk->parent);
-                       if (ret)
-                               goto err;
-               }
+       clk->usecount++;
+
+       if (clk->usecount > 1)
+               return 0;
 
-               ret = _omap2_clk_enable(clk);
+       pr_debug("clock: %s: enabling in hardware\n", clk->name);
+
+       if (clk->parent) {
+               ret = omap2_clk_enable(clk->parent);
                if (ret) {
-                       if (clk->parent)
-                               omap2_clk_disable(clk->parent);
+                       WARN(1, "clock: %s: could not enable parent %s: %d\n",
+                            clk->name, clk->parent->name, ret);
+                       goto oce_err1;
+               }
+       }
 
-                       goto err;
+       if (clk->clkdm) {
+               ret = omap2_clkdm_clk_enable(clk->clkdm, clk);
+               if (ret) {
+                       WARN(1, "clock: %s: could not enable clockdomain %s: "
+                            "%d\n", clk->name, clk->clkdm->name, ret);
+                       goto oce_err2;
                }
        }
-       return ret;
 
-err:
+       ret = clk->ops->enable(clk);
+       if (ret) {
+               WARN(1, "clock: %s: could not enable: %d\n", clk->name, ret);
+               goto oce_err3;
+       }
+
+       return 0;
+
+oce_err3:
        if (clk->clkdm)
                omap2_clkdm_clk_disable(clk->clkdm, clk);
+oce_err2:
+       if (clk->parent)
+               omap2_clk_disable(clk->parent);
+oce_err1:
        clk->usecount--;
+
        return ret;
 }
 
@@ -325,9 +371,9 @@ const struct clkops clkops_omap3_noncore_dpll_ops = {
 #endif
 
 
-/*-------------------------------------------------------------------------
- * Omap2 clock reset and init functions
- *-------------------------------------------------------------------------*/
+/*
+ * OMAP2+ clock reset and init functions
+ */
 
 #ifdef CONFIG_OMAP_RESET_CLOCKS
 void omap2_clk_disable_unused(struct clk *clk)
@@ -344,8 +390,9 @@ void omap2_clk_disable_unused(struct clk *clk)
        if (cpu_is_omap34xx()) {
                omap2_clk_enable(clk);
                omap2_clk_disable(clk);
-       } else
-               _omap2_clk_disable(clk);
+       } else {
+               clk->ops->disable(clk);
+       }
        if (clk->clkdm != NULL)
                pwrdm_clkdm_state_switch(clk->clkdm);
 }

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index b26d30a14303646f8f9eee15a8298b4072bd669e..b87ad66f083ee12ae2a47066a6746faf7ee3a747 100644 (file)
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -978,7 +978,7 @@ int omap2_clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
         * downstream clocks for debugging purposes?
         */
 
-       if (!clkdm || !clk || !clkdm->clkstctrl_reg)
+       if (!clkdm || !clk)
                return -EINVAL;
 
        if (atomic_inc_return(&clkdm->usecount) > 1)
@@ -989,6 +989,9 @@ int omap2_clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
        pr_debug("clockdomain: clkdm %s: clk %s now enabled\n", clkdm->name,
                 clk->name);
 
+       if (!clkdm->clkstctrl_reg)
+               return 0;
+
        v = omap2_clkdm_clktrctrl_read(clkdm);
 
        if ((cpu_is_omap34xx() && v == OMAP34XX_CLKSTCTRL_ENABLE_AUTO) ||
@@ -1030,7 +1033,7 @@ int omap2_clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
         * downstream clocks for debugging purposes?
         */
 
-       if (!clkdm || !clk || !clkdm->clkstctrl_reg)
+       if (!clkdm || !clk)
                return -EINVAL;
 
 #ifdef DEBUG
@@ -1048,6 +1051,9 @@ int omap2_clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
        pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
                 clk->name);
 
+       if (!clkdm->clkstctrl_reg)
+               return 0;
+
        v = omap2_clkdm_clktrctrl_read(clkdm);
 
        if ((cpu_is_omap34xx() && v == OMAP34XX_CLKSTCTRL_ENABLE_AUTO) ||