Quoting Viresh Kumar (2013-08-05 21:45:56)

Added linaro-kernel & patches@linaro.org in cc list..

On 6 August 2013 09:27, Shaojie Sun shaojie.sun@linaro.org wrote:

...

By default gate clock could be enable and disable. but in some debug condition, must keep clock on, and never be closed. setting this flag then gate clock never be closed.

Consider rewriting it as:

By default gate clk can be enabled/disabled but during debugging we may want to disable clk-disabling feature.

This patch adds a flag for it..

...

Signed-off-by: Shaojie Sun shaojie.sun@linaro.com

drivers/clk/clk-gate.c | 24 ++++++++++++++++++++++++ include/linux/clk-provider.h | 4 ++++ 2 files changed, 28 insertions(+)

Honestly speaking, I am not sure if this patch is really required or not. And so Mike can decide on that. I will do a simple code review in that time.

I don't think this feature should be merged. Since it is purely for debug purposes, and the change is small and domain-specific, I'd rather have developers implement something like this locally during debug instead of merging it into mainline. An early return in clk_disable would suffice to do the same thing.

If many other developers find this to be extremely useful then I'll reconsider.

Thanks, Mike

...

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 790306e..cc2b00e 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -15,6 +15,7 @@ #include <linux/io.h> #include <linux/err.h> #include <linux/string.h> +#include <linux/device.h>

why?

...

/**

• DOC: basic gatable clock which can gate and ungate it's ouput

@@ -48,6 +49,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable) unsigned long flags = 0; u32 reg;

•   if (!enable && (gate->flags & CLK_GATE_ALWAYS_ON))
  

•           return;
  

•   set ^= enable;
  
    if (gate->lock)
  

@@ -159,5 +163,25 @@ struct clk *clk_register_gate(struct device *dev, const char *name, if (IS_ERR(clk)) kfree(gate);

•   if (clk_gate_flags & CLK_GATE_ALWAYS_ON) {
  

•           int ret;
  

•           if (flags & CLK_SET_PARENT_GATE)
  

•                   pr_debug("%s: %salways on, but need parent gate.\n",
  

•                                   __func__, name);
  

•           ret = clk_prepare(clk);
  

•           if (ret) {
  

•                   pr_debug("%s: %s could not be prepared.\n",
  

•                                   __func__, name);
  

•                   return clk;
  

•           }
  

•           ret = clk_enable(clk);
  

•           if (ret) {
  

•                   pr_debug("%s: %s could not be enabled.\n",
  

•                                   __func__, name);
  

•                   clk_unprepare(clk);
  

•           }
  

Call clk_prepare_enable instead.

...

•   }
  

•   return clk;
  

} diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 1ec14a7..641422c 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -214,6 +214,9 @@ void of_fixed_clk_setup(struct device_node *np);

• of this register, and mask of gate bits are in higher 16-bit of this
• register. While setting the gate bits, higher 16-bit should also be
• updated to indicate changing gate bits.

• • CLK_GATE_ALWAYS_ON - by default this clock could be enable and disable. but
• • in some debug condition, must keep this clock on, and never be closed.
• • setting this flag then this clock never be closed.

Copy some part from the commit log I just suggested..

If this flag is only for debugging we may name it that way. Maybe: CLK_GATE_DBG_ALWAYS_ON ??