Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework
From: Maxime Ripard <hidden>
Date: 2015-07-27 07:30:06
Also in:
linux-arm-kernel, lkml
On Wed, Jul 22, 2015 at 02:04:13PM +0100, Lee Jones wrote:
quoted hunk ↗ jump to hunk
These new API calls will firstly provide a mechanisms to tag a clock as critical and secondly allow any knowledgeable driver to (un)gate clocks, even if they are marked as critical. Suggested-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Signed-off-by: Lee Jones <redacted> --- drivers/clk/clk.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/clk-provider.h | 2 ++ include/linux/clk.h | 30 +++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+)diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 61c3fc5..486b1da 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c@@ -46,6 +46,21 @@ static struct clk_core *clk_core_lookup(const char *name); /*** private data structures ***/ +/** + * struct critical - Provides 'play' over critical clocks. A clock can be + * marked as critical, meaning that it should not be + * disabled. However, if a driver which is aware of the + * critical behaviour wants to control it, it can do so + * using clk_enable_critical() and clk_disable_critical(). + * + * @enabled Is clock critical? Once set, doesn't change + * @leave_on Self explanatory. Can be disabled by knowledgeable drivers + */ +struct critical { + bool enabled; + bool leave_on; +}; + struct clk_core { const char *name; const struct clk_ops *ops;@@ -75,6 +90,7 @@ struct clk_core { struct dentry *dentry; #endif struct kref ref; + struct critical critical; }; struct clk {@@ -995,6 +1011,10 @@ static void clk_core_disable(struct clk_core *clk) if (WARN_ON(clk->enable_count == 0)) return; + /* Refuse to turn off a critical clock */ + if (clk->enable_count == 1 && clk->critical.leave_on) + return; +
I think it should be handled by a separate counting. Otherwise, if you have two users that marked the clock as critical, and then one of them disable it...
quoted hunk ↗ jump to hunk
if (--clk->enable_count > 0) return;@@ -1037,6 +1057,13 @@ void clk_disable(struct clk *clk) } EXPORT_SYMBOL_GPL(clk_disable); +void clk_disable_critical(struct clk *clk) +{ + clk->core->critical.leave_on = false;
.. you just lost the fact that it was critical in the first place. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachments
- signature.asc [application/pgp-signature] 819 bytes