* [PATCH] clk: gate: add CLK_GATE_ALWAYS_ON
@ 2013-08-06 3:57 Shaojie Sun
2013-08-06 4:45 ` Viresh Kumar
0 siblings, 1 reply; 3+ messages in thread
From: Shaojie Sun @ 2013-08-06 3:57 UTC (permalink / raw)
To: linux-arm-kernel
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.
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(+)
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>
/**
* 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);
+ }
+ }
+
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.
*/
struct clk_gate {
struct clk_hw hw;
@@ -225,6 +228,7 @@ struct clk_gate {
#define CLK_GATE_SET_TO_DISABLE BIT(0)
#define CLK_GATE_HIWORD_MASK BIT(1)
+#define CLK_GATE_ALWAYS_ON BIT(4)
extern const struct clk_ops clk_gate_ops;
struct clk *clk_register_gate(struct device *dev, const char *name,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH] clk: gate: add CLK_GATE_ALWAYS_ON
2013-08-06 3:57 [PATCH] clk: gate: add CLK_GATE_ALWAYS_ON Shaojie Sun
@ 2013-08-06 4:45 ` Viresh Kumar
2013-08-06 19:09 ` Mike Turquette
0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2013-08-06 4:45 UTC (permalink / raw)
To: linux-arm-kernel
Added linaro-kernel & patches at 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.
> 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 ??
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] clk: gate: add CLK_GATE_ALWAYS_ON
2013-08-06 4:45 ` Viresh Kumar
@ 2013-08-06 19:09 ` Mike Turquette
0 siblings, 0 replies; 3+ messages in thread
From: Mike Turquette @ 2013-08-06 19:09 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Viresh Kumar (2013-08-05 21:45:56)
> Added linaro-kernel & patches at 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 ??
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-08-06 19:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-06 3:57 [PATCH] clk: gate: add CLK_GATE_ALWAYS_ON Shaojie Sun
2013-08-06 4:45 ` Viresh Kumar
2013-08-06 19:09 ` Mike Turquette
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).