On Thu, 30 Apr 2015, Sascha Hauer wrote: > Some clocks are so critical to the system that they must never be turned > off unless explicitly requested. Normally unused clocks get disabled in > the clk_disable_unused initcall. Currently there are two ways for > keeping clocks enabled even if they are unused, both with their own > drawbacks: > > - The CLK_IGNORE_UNUSED flag. Clocks with this flags are not disabled > during clk_disable_unused. The problem with this is that these clocks > are skipped during the clk_disable_unused initcall, but can get > disabled due to normal clk_enable/disable at any time, not necessarily > even on operations on the critical clocks themselves but on operations > on their ancestors, siblings or children. > > - call clk_prepare_enable right after registration in the clock > driver. This works properly, but due to the increased enable counter > the clock will never get disabled, even not when a proper user > comes along who knows when the clock can safely be disabled. > > This patch solves this by introducing two new API calls: > > clk_prepare_enable_critical() will call clk_prepare_enable() on a clock > and additionally set a 'critical' flag on the clock. This call is > intended for clock providers which provide a critical clock. > > clk_disable_unprepare_critical() is intended for consumers of a critical > clock. Consumers should first call clk_prepare_enable() on a clock to > get their own prepare/enable reference and call > clk_disable_unprepare_critical() afterwards. This will clear the > 'critical' flag from the clock and decrease the prepare/enable counter. > The ownership of the clock is now transferred to the consumer. Calling > clk_disable_unprepare_critical() on a clock which doesn't have the > 'critical' flag set is just a no-op, so consumers can safely call this > on their clocks. > > To make this more versatile a clk flag could be introduced which could > be set during registration so that clk providers would not have to make > this API call on each critical clock. I haven't implemented this yet > because it turned out not to be trivial. clk_prepare_enable can only > be called when the clock is not orphaned which may not be the case > during registration time. I can implement this if this way of handling > critical clocks is considered suitable for mainline.
I like the premise of this set and I think it'll work well with my patchset. So are you two okay with me fixing up my patchset to encompass it, or did you have other plans? > Signed-off-by: Sascha Hauer <s.ha...@pengutronix.de> > --- > drivers/clk/clk.c | 46 > ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/clk-provider.h | 2 ++ > include/linux/clk.h | 10 ++++++++++ > 3 files changed, 58 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 459ce9d..d57d4fd 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -64,6 +64,7 @@ struct clk_core { > unsigned long flags; > unsigned int enable_count; > unsigned int prepare_count; > + bool enable_critical; > unsigned long accuracy; > int phase; > struct hlist_head children; > @@ -940,7 +941,13 @@ void clk_unprepare(struct clk *clk) > return; > > clk_prepare_lock(); > + > + if (clk->core->enable_critical) > + goto out; > + > clk_core_unprepare(clk->core); > + > +out: > clk_prepare_unlock(); > } > EXPORT_SYMBOL_GPL(clk_unprepare); > @@ -995,7 +1002,9 @@ int clk_prepare(struct clk *clk) > return 0; > > clk_prepare_lock(); > + > ret = clk_core_prepare(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -2250,6 +2259,43 @@ bool clk_is_match(const struct clk *p, const struct > clk *q) > } > EXPORT_SYMBOL_GPL(clk_is_match); > > +int clk_prepare_enable_critical(struct clk *clk) > +{ > + if (!clk) > + return 0; > + > + clk_prepare_lock(); > + if (clk->core->enable_critical) { > + clk_prepare_unlock(); > + return -EBUSY; > + } > + > + clk->core->enable_critical = true; > + > + clk_prepare_unlock(); > + > + return clk_prepare_enable(clk); > + > +} > + > +void clk_disable_unprepare_critical(struct clk *clk) > +{ > + if (!clk) > + return; > + > + clk_prepare_lock(); > + if (!clk->core->enable_critical) { > + clk_prepare_unlock(); > + return; > + } > + > + clk->core->enable_critical = false; > + > + clk_prepare_unlock(); > + > + clk_disable_unprepare(clk); > +} > + > /** > * __clk_init - initialize the data structures in a struct clk > * @dev: device initializing this clk, placeholder for now > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index df69531..8727c12 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -694,5 +694,7 @@ struct dentry *clk_debugfs_add_file(struct clk_hw *hw, > char *name, umode_t mode, > void *data, const struct file_operations *fops); > #endif > > +int clk_prepare_enable_critical(struct clk *clk); > + > #endif /* CONFIG_COMMON_CLK */ > #endif /* CLK_PROVIDER_H */ > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 68c16a6..b259e36 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -138,6 +138,16 @@ int clk_get_phase(struct clk *clk); > */ > bool clk_is_match(const struct clk *p, const struct clk *q); > > +/** > + * clk_disable_unprepare_critical - remove critical flag from clock > + * @clk: clock source > + * > + * This removes the critical flag from a clock and disables it. The > + * consumer should have enabled the clock by itself now if it wants > + * to keep the clock enabled. > + */ > +void clk_disable_unprepare_critical(struct clk *clk); > + > #else > > static inline long clk_get_accuracy(struct clk *clk) -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/