| Message ID | 20170321183330.26722-5-jbrunet@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hi Jerome, Quoting Jerome Brunet (2017-03-21 11:33:27) > The patch adds clk_protect and clk_unprotect to the CCF API. These > functions allow a consumer to inform the system that the rate of clock is > critical to for its operations and it can't tolerate other consumers > changing the rate or introducing glitches while the clock is protected. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/clk/clk.c | 177 ++++++++++++++++++++++++++++++++++++++++--- > include/linux/clk-provider.h | 1 + > include/linux/clk.h | 29 +++++++ > 3 files changed, 197 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index fa77a1841e0f..69db8cc15063 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -60,6 +60,7 @@ struct clk_core { > bool orphan; > unsigned int enable_count; > unsigned int prepare_count; > + unsigned int protect_count; > unsigned long min_rate; > unsigned long max_rate; > unsigned long accuracy; > @@ -84,6 +85,7 @@ struct clk { > const char *con_id; > unsigned long min_rate; > unsigned long max_rate; > + unsigned long protect_ucount; > struct hlist_node clks_node; > }; > > @@ -160,6 +162,11 @@ static bool clk_core_is_prepared(struct clk_core *core) > return core->ops->is_prepared(core->hw); > } > > +static bool clk_core_is_protected(struct clk_core *core) > +{ > + return core->protect_count; > +} > + > static bool clk_core_is_enabled(struct clk_core *core) > { > /* > @@ -328,6 +335,11 @@ bool clk_hw_is_prepared(const struct clk_hw *hw) > return clk_core_is_prepared(hw->core); > } > > +bool clk_hw_is_protected(const struct clk_hw *hw) > +{ > + return clk_core_is_protected(hw->core); > +} > + > bool clk_hw_is_enabled(const struct clk_hw *hw) > { > return clk_core_is_enabled(hw->core); > @@ -584,6 +596,89 @@ int clk_prepare(struct clk *clk) > } > EXPORT_SYMBOL_GPL(clk_prepare); > > +static void clk_core_unprotect(struct clk_core *core) > +{ > + lockdep_assert_held(&prepare_lock); > + > + if (!core) > + return; > + > + if (WARN_ON(core->protect_count == 0)) > + return; > + > + if (--core->protect_count > 0) > + return; > + > + clk_core_unprotect(core->parent); > +} > + > +/** > + * clk_unprotect - unprotect a clock source > + * @clk: the clk being unprotected > + * > + * clk_unprotect shall be used when a consumer no longer depends on the clock > + * rate and can tolerate glitches. As with clk_unprepare and clk_enable, calls > + * to clk_unprotect must be balanced with clk_protect. > + * clk_protect may sleep Maybe: """ clk_unprotect completes a critical section during which the clock consumer cannot tolerate any change to the clock rate. If no other clock consumers have protected clocks in the parent chain, then calls to this function will allow the clocks in the parent chain to change rates freely. Unlike the clk_set_rate_range method, which allows the rate to change within a given range, protected clocks cannot have their rate changed, either directly or indirectly due to changes further up the parent chain of clocks. Calls to clk_unprotect must be balanced with calls to clk_protect. Calls to this function may sleep, and do not return error status. """ > + */ > +void clk_unprotect(struct clk *clk) > +{ > + if (!clk) > + return; > + > + clk_prepare_lock(); > + > + if (WARN_ON(clk->protect_ucount <= 0)) { > + /* > + * There is something wrong with this consumer protect count. > + * Stop here before messing with the provider > + */ > + clk_prepare_unlock(); > + return; > + } > + > + clk_core_unprotect(clk->core); > + clk->protect_ucount--; > + clk_prepare_unlock(); > +} > +EXPORT_SYMBOL_GPL(clk_unprotect); > + > +static void clk_core_protect(struct clk_core *core) > +{ > + lockdep_assert_held(&prepare_lock); > + > + if (!core) > + return; > + > + if (core->protect_count == 0) > + clk_core_protect(core->parent); > + > + core->protect_count++; > +} > + > +/** > + * clk_protect - protect a clock source > + * @clk: the clk being protected > + * > + * clk_protect can be used when a consumer depends on the clock rate and can't > + * tolerate any glitches. The consumer protecting the clock can still make > + * adjustment to clock, if it is the only one protecting the clock. Other > + * consumers can still use the clock but won't be able to adjust the rate or > + * reparent the clock while it is protected. > + * clk_protect may sleep. > + */ Maybe: """ clk_protect begins a critical section during which the clock consumer cannot tolerate any change to the clock rate. This results in all clocks up the parent chain to also be rate-protected. Unlike the clk_set_rate_range method, which allows the rate to change within a given range, protected clocks cannot have their rate changed, either directly or indirectly due to changes further up the parent chain of clocks. Calls to clk_protect should be balanced with calls to clk_unprotect. Calls to this function may sleep, and do not return error status. """ > +void clk_protect(struct clk *clk) > +{ > + if (!clk) > + return; > + > + clk_prepare_lock(); > + clk_core_protect(clk->core); > + clk->protect_ucount++; > + clk_prepare_unlock(); > +} > +EXPORT_SYMBOL_GPL(clk_protect); > + > static void clk_core_disable(struct clk_core *core) > { > lockdep_assert_held(&enable_lock); > @@ -838,7 +933,9 @@ static int clk_core_determine_round(struct clk_core *core, > { > long rate; > > - if (core->ops->determine_rate) { > + if (clk_core_is_protected(core)) { > + req->rate = core->rate; Hmm, in CCF we basically re-use round_rate/determine_rate from within calls to clk_set_rate. The point is that clk_set_rate should set the same rate that is returned from either of the other two functions. The change above breaks that subtle assumption, as a clk consumer can now call: clk_protect(clk); rate = clk_get_rate(clk); // rate is 1234; rate = clk_round_rate(clk, 5678); // rate is STILL 1234 ret = clk_set_rate(clk, 5678); rate = clk_get_rate(clk); // rate is 5678 Is my understanding correct? If so then we might consider adding some more complex knowledge to clk_round_rate. Something like: if clk_is_protected(clk) AND it is only protect by THIS clk: round the rate else: return core->rate > + } else if (core->ops->determine_rate) { > return core->ops->determine_rate(core->hw, req); > } else if (core->ops->round_rate) { > rate = core->ops->round_rate(core->hw, req->rate, > @@ -1381,7 +1478,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, > req.min_rate = min_rate; > req.max_rate = max_rate; > > - clk_core_init_rate_req(core, req); > + clk_core_init_rate_req(core, &req); Why isn't this change in patch #3? Seems like a bug introduced in patch #3 and fixed here in patch #4. > > ret = clk_core_determine_round(core, &req); > if (ret < 0) > @@ -1637,8 +1734,14 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > /* prevent racing with updates to the clock topology */ > clk_prepare_lock(); The fact that a user can protect a clk AND change its rate is very subtle. Please update the kerneldoc for clk_set_rate, clk_set_parent and any other affected apis. > > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); What happens here if two different users have both protected this same clk? Looks like we ignore the second user? In other words, what happens if clk->protect_ucount == 2? Best regards, Mike > + > ret = clk_core_set_rate_nolock(clk->core, rate); > > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -1669,12 +1772,18 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max) > > clk_prepare_lock(); > > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > if (min != clk->min_rate || max != clk->max_rate) { > clk->min_rate = min; > clk->max_rate = max; > ret = clk_core_set_rate_nolock(clk->core, clk->core->req_rate); > } > > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -1815,6 +1924,9 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent) > if ((core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) > return -EBUSY; > > + if (clk_core_is_protected(core)) > + return -EBUSY; > + > /* try finding the new parent index */ > if (parent) { > p_index = clk_fetch_parent_index(core, parent); > @@ -1878,11 +1990,24 @@ static int clk_core_set_parent_lock(struct clk_core *core, > */ > int clk_set_parent(struct clk *clk, struct clk *parent) > { > + int ret; > + > if (!clk) > return 0; > > - return clk_core_set_parent_lock(clk->core, > - parent ? parent->core : NULL); > + clk_prepare_lock(); > + > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > + ret = clk_core_set_parent(clk->core, parent ? parent->core : NULL); > + > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > + clk_prepare_unlock(); > + > + return ret; > } > EXPORT_SYMBOL_GPL(clk_set_parent); > > @@ -1893,7 +2018,10 @@ static int clk_core_set_phase(struct clk_core *core, int degrees) > if (!core) > return 0; > > - trace_clk_set_phase(clk->core, degrees); > + if (clk_core_is_protected(core)) > + return -EBUSY; > + > + trace_clk_set_phase(core, degrees); > > if (core->ops->set_phase) > ret = core->ops->set_phase(core->hw, degrees); > @@ -1936,7 +2064,15 @@ int clk_set_phase(struct clk *clk, int degrees) > degrees += 360; > > clk_prepare_lock(); > + > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > ret = clk_core_set_phase(clk->core, degrees); > + > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -2023,11 +2159,12 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > if (!c) > return; > > - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", > + seq_printf(s, "%*s%-*s %11d %12d %12d %11lu %10lu %-3d\n", > level * 3 + 1, "", > 30 - level * 3, c->name, > - c->enable_count, c->prepare_count, clk_core_get_rate(c), > - clk_core_get_accuracy(c), clk_core_get_phase(c)); > + c->enable_count, c->prepare_count, c->protect_count, > + clk_core_get_rate(c), clk_core_get_accuracy(c), > + clk_core_get_phase(c)); > } > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, > @@ -2049,8 +2186,8 @@ static int clk_summary_show(struct seq_file *s, void *data) > struct clk_core *c; > struct hlist_head **lists = (struct hlist_head **)s->private; > > - seq_puts(s, " clock enable_cnt prepare_cnt rate accuracy phase\n"); > - seq_puts(s, "----------------------------------------------------------------------------------------\n"); > + seq_puts(s, " clock enable_cnt prepare_cnt protect_cnt rate accuracy phase\n"); > + seq_puts(s, "----------------------------------------------------------------------------------------------------\n"); > > clk_prepare_lock(); > > @@ -2085,6 +2222,7 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) > seq_printf(s, "\"%s\": { ", c->name); > seq_printf(s, "\"enable_count\": %d,", c->enable_count); > seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); > + seq_printf(s, "\"protect_count\": %d,", c->protect_count); > seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); > seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); > seq_printf(s, "\"phase\": %d", clk_core_get_phase(c)); > @@ -2191,6 +2329,11 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) > if (!d) > goto err_out; > > + d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry, > + (u32 *)&core->protect_count); > + if (!d) > + goto err_out; > + > d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry, > (u32 *)&core->notifier_count); > if (!d) > @@ -2747,6 +2890,11 @@ void clk_unregister(struct clk *clk) > if (clk->core->prepare_count) > pr_warn("%s: unregistering prepared clock: %s\n", > __func__, clk->core->name); > + > + if (clk->core->protect_count) > + pr_warn("%s: unregistering protected clock: %s\n", > + __func__, clk->core->name); > + > kref_put(&clk->core->ref, __clk_release); > unlock: > clk_prepare_unlock(); > @@ -2905,6 +3053,15 @@ void __clk_put(struct clk *clk) > > clk_prepare_lock(); > > + /* Protect count not balanced: warn and sanitize */ > + if (clk->protect_ucount) { > + pr_warn("%s: releasing protected clock: %s\n", > + __func__, clk->core->name); > + > + for (; clk->protect_ucount; clk->protect_ucount--) > + clk_core_unprotect(clk->core); > + } > + > hlist_del(&clk->clks_node); > if (clk->min_rate > clk->core->req_rate || > clk->max_rate < clk->core->req_rate) > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index a428aec36ace..705a158d9b8f 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -739,6 +739,7 @@ unsigned long clk_hw_get_rate(const struct clk_hw *hw); > unsigned long __clk_get_flags(struct clk *clk); > unsigned long clk_hw_get_flags(const struct clk_hw *hw); > bool clk_hw_is_prepared(const struct clk_hw *hw); > +bool clk_hw_is_protected(const struct clk_hw *hw); > bool clk_hw_is_enabled(const struct clk_hw *hw); > bool __clk_is_enabled(struct clk *clk); > struct clk *__clk_lookup(const char *name); > diff --git a/include/linux/clk.h b/include/linux/clk.h > index e9d36b3e49de..90b72ead4411 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -265,6 +265,30 @@ struct clk *devm_clk_get(struct device *dev, const char *id); > */ > struct clk *devm_get_clk_from_child(struct device *dev, > struct device_node *np, const char *con_id); > +/** > + * clk_protect - inform the system when the clock source should be protected. > + * @clk: clock source > + * > + * This function informs the system that the consumer protecting the clock > + * depends on the rate of the clock source and can't tolerate any glitches > + * introduced by further clock rate change or re-parenting of the clock source. > + * > + * Must not be called from within atomic context. > + */ > +void clk_protect(struct clk *clk); > + > +/** > + * clk_unprotect - release the protection of the clock source. > + * @clk: clock source > + * > + * This function informs the system that the consumer previously protecting the > + * clock source can now deal with other consumer altering the clock source. > + * > + * The caller must balance the number of protect and unprotect calls. > + * > + * Must not be called from within atomic context. > + */ > +void clk_unprotect(struct clk *clk); > > /** > * clk_enable - inform the system when the clock source should be running. > @@ -460,6 +484,11 @@ static inline void clk_put(struct clk *clk) {} > > static inline void devm_clk_put(struct device *dev, struct clk *clk) {} > > + > +static inline void clk_protect(struct clk *clk) {} > + > +static inline void clk_unprotect(struct clk *clk) {} > + > static inline int clk_enable(struct clk *clk) > { > return 0; > -- > 2.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jerome, Quoting Jerome Brunet (2017-03-21 11:33:27) > The patch adds clk_protect and clk_unprotect to the CCF API. These > functions allow a consumer to inform the system that the rate of clock is > critical to for its operations and it can't tolerate other consumers > changing the rate or introducing glitches while the clock is protected. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> I haven't reviewed this yet, but changes to clk.h should also keep Russell King in Cc. Best regards, Mike > --- > drivers/clk/clk.c | 177 ++++++++++++++++++++++++++++++++++++++++--- > include/linux/clk-provider.h | 1 + > include/linux/clk.h | 29 +++++++ > 3 files changed, 197 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index fa77a1841e0f..69db8cc15063 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -60,6 +60,7 @@ struct clk_core { > bool orphan; > unsigned int enable_count; > unsigned int prepare_count; > + unsigned int protect_count; > unsigned long min_rate; > unsigned long max_rate; > unsigned long accuracy; > @@ -84,6 +85,7 @@ struct clk { > const char *con_id; > unsigned long min_rate; > unsigned long max_rate; > + unsigned long protect_ucount; > struct hlist_node clks_node; > }; > > @@ -160,6 +162,11 @@ static bool clk_core_is_prepared(struct clk_core *core) > return core->ops->is_prepared(core->hw); > } > > +static bool clk_core_is_protected(struct clk_core *core) > +{ > + return core->protect_count; > +} > + > static bool clk_core_is_enabled(struct clk_core *core) > { > /* > @@ -328,6 +335,11 @@ bool clk_hw_is_prepared(const struct clk_hw *hw) > return clk_core_is_prepared(hw->core); > } > > +bool clk_hw_is_protected(const struct clk_hw *hw) > +{ > + return clk_core_is_protected(hw->core); > +} > + > bool clk_hw_is_enabled(const struct clk_hw *hw) > { > return clk_core_is_enabled(hw->core); > @@ -584,6 +596,89 @@ int clk_prepare(struct clk *clk) > } > EXPORT_SYMBOL_GPL(clk_prepare); > > +static void clk_core_unprotect(struct clk_core *core) > +{ > + lockdep_assert_held(&prepare_lock); > + > + if (!core) > + return; > + > + if (WARN_ON(core->protect_count == 0)) > + return; > + > + if (--core->protect_count > 0) > + return; > + > + clk_core_unprotect(core->parent); > +} > + > +/** > + * clk_unprotect - unprotect a clock source > + * @clk: the clk being unprotected > + * > + * clk_unprotect shall be used when a consumer no longer depends on the clock > + * rate and can tolerate glitches. As with clk_unprepare and clk_enable, calls > + * to clk_unprotect must be balanced with clk_protect. > + * clk_protect may sleep > + */ > +void clk_unprotect(struct clk *clk) > +{ > + if (!clk) > + return; > + > + clk_prepare_lock(); > + > + if (WARN_ON(clk->protect_ucount <= 0)) { > + /* > + * There is something wrong with this consumer protect count. > + * Stop here before messing with the provider > + */ > + clk_prepare_unlock(); > + return; > + } > + > + clk_core_unprotect(clk->core); > + clk->protect_ucount--; > + clk_prepare_unlock(); > +} > +EXPORT_SYMBOL_GPL(clk_unprotect); > + > +static void clk_core_protect(struct clk_core *core) > +{ > + lockdep_assert_held(&prepare_lock); > + > + if (!core) > + return; > + > + if (core->protect_count == 0) > + clk_core_protect(core->parent); > + > + core->protect_count++; > +} > + > +/** > + * clk_protect - protect a clock source > + * @clk: the clk being protected > + * > + * clk_protect can be used when a consumer depends on the clock rate and can't > + * tolerate any glitches. The consumer protecting the clock can still make > + * adjustment to clock, if it is the only one protecting the clock. Other > + * consumers can still use the clock but won't be able to adjust the rate or > + * reparent the clock while it is protected. > + * clk_protect may sleep. > + */ > +void clk_protect(struct clk *clk) > +{ > + if (!clk) > + return; > + > + clk_prepare_lock(); > + clk_core_protect(clk->core); > + clk->protect_ucount++; > + clk_prepare_unlock(); > +} > +EXPORT_SYMBOL_GPL(clk_protect); > + > static void clk_core_disable(struct clk_core *core) > { > lockdep_assert_held(&enable_lock); > @@ -838,7 +933,9 @@ static int clk_core_determine_round(struct clk_core *core, > { > long rate; > > - if (core->ops->determine_rate) { > + if (clk_core_is_protected(core)) { > + req->rate = core->rate; > + } else if (core->ops->determine_rate) { > return core->ops->determine_rate(core->hw, req); > } else if (core->ops->round_rate) { > rate = core->ops->round_rate(core->hw, req->rate, > @@ -1381,7 +1478,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, > req.min_rate = min_rate; > req.max_rate = max_rate; > > - clk_core_init_rate_req(core, req); > + clk_core_init_rate_req(core, &req); > > ret = clk_core_determine_round(core, &req); > if (ret < 0) > @@ -1637,8 +1734,14 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > /* prevent racing with updates to the clock topology */ > clk_prepare_lock(); > > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > ret = clk_core_set_rate_nolock(clk->core, rate); > > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -1669,12 +1772,18 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max) > > clk_prepare_lock(); > > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > if (min != clk->min_rate || max != clk->max_rate) { > clk->min_rate = min; > clk->max_rate = max; > ret = clk_core_set_rate_nolock(clk->core, clk->core->req_rate); > } > > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -1815,6 +1924,9 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent) > if ((core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) > return -EBUSY; > > + if (clk_core_is_protected(core)) > + return -EBUSY; > + > /* try finding the new parent index */ > if (parent) { > p_index = clk_fetch_parent_index(core, parent); > @@ -1878,11 +1990,24 @@ static int clk_core_set_parent_lock(struct clk_core *core, > */ > int clk_set_parent(struct clk *clk, struct clk *parent) > { > + int ret; > + > if (!clk) > return 0; > > - return clk_core_set_parent_lock(clk->core, > - parent ? parent->core : NULL); > + clk_prepare_lock(); > + > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > + ret = clk_core_set_parent(clk->core, parent ? parent->core : NULL); > + > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > + clk_prepare_unlock(); > + > + return ret; > } > EXPORT_SYMBOL_GPL(clk_set_parent); > > @@ -1893,7 +2018,10 @@ static int clk_core_set_phase(struct clk_core *core, int degrees) > if (!core) > return 0; > > - trace_clk_set_phase(clk->core, degrees); > + if (clk_core_is_protected(core)) > + return -EBUSY; > + > + trace_clk_set_phase(core, degrees); > > if (core->ops->set_phase) > ret = core->ops->set_phase(core->hw, degrees); > @@ -1936,7 +2064,15 @@ int clk_set_phase(struct clk *clk, int degrees) > degrees += 360; > > clk_prepare_lock(); > + > + if (clk->protect_ucount) > + clk_core_unprotect(clk->core); > + > ret = clk_core_set_phase(clk->core, degrees); > + > + if (clk->protect_ucount) > + clk_core_protect(clk->core); > + > clk_prepare_unlock(); > > return ret; > @@ -2023,11 +2159,12 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, > if (!c) > return; > > - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", > + seq_printf(s, "%*s%-*s %11d %12d %12d %11lu %10lu %-3d\n", > level * 3 + 1, "", > 30 - level * 3, c->name, > - c->enable_count, c->prepare_count, clk_core_get_rate(c), > - clk_core_get_accuracy(c), clk_core_get_phase(c)); > + c->enable_count, c->prepare_count, c->protect_count, > + clk_core_get_rate(c), clk_core_get_accuracy(c), > + clk_core_get_phase(c)); > } > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, > @@ -2049,8 +2186,8 @@ static int clk_summary_show(struct seq_file *s, void *data) > struct clk_core *c; > struct hlist_head **lists = (struct hlist_head **)s->private; > > - seq_puts(s, " clock enable_cnt prepare_cnt rate accuracy phase\n"); > - seq_puts(s, "----------------------------------------------------------------------------------------\n"); > + seq_puts(s, " clock enable_cnt prepare_cnt protect_cnt rate accuracy phase\n"); > + seq_puts(s, "----------------------------------------------------------------------------------------------------\n"); > > clk_prepare_lock(); > > @@ -2085,6 +2222,7 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) > seq_printf(s, "\"%s\": { ", c->name); > seq_printf(s, "\"enable_count\": %d,", c->enable_count); > seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); > + seq_printf(s, "\"protect_count\": %d,", c->protect_count); > seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); > seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); > seq_printf(s, "\"phase\": %d", clk_core_get_phase(c)); > @@ -2191,6 +2329,11 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) > if (!d) > goto err_out; > > + d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry, > + (u32 *)&core->protect_count); > + if (!d) > + goto err_out; > + > d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry, > (u32 *)&core->notifier_count); > if (!d) > @@ -2747,6 +2890,11 @@ void clk_unregister(struct clk *clk) > if (clk->core->prepare_count) > pr_warn("%s: unregistering prepared clock: %s\n", > __func__, clk->core->name); > + > + if (clk->core->protect_count) > + pr_warn("%s: unregistering protected clock: %s\n", > + __func__, clk->core->name); > + > kref_put(&clk->core->ref, __clk_release); > unlock: > clk_prepare_unlock(); > @@ -2905,6 +3053,15 @@ void __clk_put(struct clk *clk) > > clk_prepare_lock(); > > + /* Protect count not balanced: warn and sanitize */ > + if (clk->protect_ucount) { > + pr_warn("%s: releasing protected clock: %s\n", > + __func__, clk->core->name); > + > + for (; clk->protect_ucount; clk->protect_ucount--) > + clk_core_unprotect(clk->core); > + } > + > hlist_del(&clk->clks_node); > if (clk->min_rate > clk->core->req_rate || > clk->max_rate < clk->core->req_rate) > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index a428aec36ace..705a158d9b8f 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -739,6 +739,7 @@ unsigned long clk_hw_get_rate(const struct clk_hw *hw); > unsigned long __clk_get_flags(struct clk *clk); > unsigned long clk_hw_get_flags(const struct clk_hw *hw); > bool clk_hw_is_prepared(const struct clk_hw *hw); > +bool clk_hw_is_protected(const struct clk_hw *hw); > bool clk_hw_is_enabled(const struct clk_hw *hw); > bool __clk_is_enabled(struct clk *clk); > struct clk *__clk_lookup(const char *name); > diff --git a/include/linux/clk.h b/include/linux/clk.h > index e9d36b3e49de..90b72ead4411 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -265,6 +265,30 @@ struct clk *devm_clk_get(struct device *dev, const char *id); > */ > struct clk *devm_get_clk_from_child(struct device *dev, > struct device_node *np, const char *con_id); > +/** > + * clk_protect - inform the system when the clock source should be protected. > + * @clk: clock source > + * > + * This function informs the system that the consumer protecting the clock > + * depends on the rate of the clock source and can't tolerate any glitches > + * introduced by further clock rate change or re-parenting of the clock source. > + * > + * Must not be called from within atomic context. > + */ > +void clk_protect(struct clk *clk); > + > +/** > + * clk_unprotect - release the protection of the clock source. > + * @clk: clock source > + * > + * This function informs the system that the consumer previously protecting the > + * clock source can now deal with other consumer altering the clock source. > + * > + * The caller must balance the number of protect and unprotect calls. > + * > + * Must not be called from within atomic context. > + */ > +void clk_unprotect(struct clk *clk); > > /** > * clk_enable - inform the system when the clock source should be running. > @@ -460,6 +484,11 @@ static inline void clk_put(struct clk *clk) {} > > static inline void devm_clk_put(struct device *dev, struct clk *clk) {} > > + > +static inline void clk_protect(struct clk *clk) {} > + > +static inline void clk_unprotect(struct clk *clk) {} > + > static inline int clk_enable(struct clk *clk) > { > return 0; > -- > 2.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-05-11 at 12:05 -0700, Michael Turquette wrote: > Hi Jerome, > > Quoting Jerome Brunet (2017-03-21 11:33:27) > > The patch adds clk_protect and clk_unprotect to the CCF API. These > > functions allow a consumer to inform the system that the rate of clock is > > critical to for its operations and it can't tolerate other consumers > > changing the rate or introducing glitches while the clock is protected. > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/clk/clk.c | 177 > > ++++++++++++++++++++++++++++++++++++++++--- > > include/linux/clk-provider.h | 1 + > > include/linux/clk.h | 29 +++++++ > > 3 files changed, 197 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index fa77a1841e0f..69db8cc15063 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -60,6 +60,7 @@ struct clk_core { > > bool orphan; > > unsigned int enable_count; > > unsigned int prepare_count; > > + unsigned int protect_count; > > unsigned long min_rate; > > unsigned long max_rate; > > unsigned long accuracy; > > @@ -84,6 +85,7 @@ struct clk { > > const char *con_id; > > unsigned long min_rate; > > unsigned long max_rate; > > + unsigned long protect_ucount; > > struct hlist_node clks_node; > > }; > > > > @@ -160,6 +162,11 @@ static bool clk_core_is_prepared(struct clk_core *core) > > return core->ops->is_prepared(core->hw); > > } > > > > +static bool clk_core_is_protected(struct clk_core *core) > > +{ > > + return core->protect_count; > > +} > > + > > static bool clk_core_is_enabled(struct clk_core *core) > > { > > /* > > @@ -328,6 +335,11 @@ bool clk_hw_is_prepared(const struct clk_hw *hw) > > return clk_core_is_prepared(hw->core); > > } > > > > +bool clk_hw_is_protected(const struct clk_hw *hw) > > +{ > > + return clk_core_is_protected(hw->core); > > +} > > + > > bool clk_hw_is_enabled(const struct clk_hw *hw) > > { > > return clk_core_is_enabled(hw->core); > > @@ -584,6 +596,89 @@ int clk_prepare(struct clk *clk) > > } > > EXPORT_SYMBOL_GPL(clk_prepare); > > > > +static void clk_core_unprotect(struct clk_core *core) > > +{ > > + lockdep_assert_held(&prepare_lock); > > + > > + if (!core) > > + return; > > + > > + if (WARN_ON(core->protect_count == 0)) > > + return; > > + > > + if (--core->protect_count > 0) > > + return; > > + > > + clk_core_unprotect(core->parent); > > +} > > + > > +/** > > + * clk_unprotect - unprotect a clock source > > + * @clk: the clk being unprotected > > + * > > + * clk_unprotect shall be used when a consumer no longer depends on the > > clock > > + * rate and can tolerate glitches. As with clk_unprepare and clk_enable, > > calls > > + * to clk_unprotect must be balanced with clk_protect. > > + * clk_protect may sleep > > Maybe: > > """ > clk_unprotect completes a critical section during which the clock > consumer cannot tolerate any change to the clock rate. If no other clock > consumers have protected clocks in the parent chain, then calls to this > function will allow the clocks in the parent chain to change rates > freely. > > Unlike the clk_set_rate_range method, which allows the rate to change > within a given range, protected clocks cannot have their rate changed, > either directly or indirectly due to changes further up the parent chain > of clocks. > > Calls to clk_unprotect must be balanced with calls to clk_protect. Calls > to this function may sleep, and do not return error status. > """ Like it ! Thx ! > > > + */ > > +void clk_unprotect(struct clk *clk) > > +{ > > + if (!clk) > > + return; > > + > > + clk_prepare_lock(); > > + > > + if (WARN_ON(clk->protect_ucount <= 0)) { > > + /* > > + * There is something wrong with this consumer protect > > count. > > + * Stop here before messing with the provider > > + */ > > + clk_prepare_unlock(); > > + return; > > + } > > + > > + clk_core_unprotect(clk->core); > > + clk->protect_ucount--; > > + clk_prepare_unlock(); > > +} > > +EXPORT_SYMBOL_GPL(clk_unprotect); > > + > > +static void clk_core_protect(struct clk_core *core) > > +{ > > + lockdep_assert_held(&prepare_lock); > > + > > + if (!core) > > + return; > > + > > + if (core->protect_count == 0) > > + clk_core_protect(core->parent); > > + > > + core->protect_count++; > > +} > > + > > +/** > > + * clk_protect - protect a clock source > > + * @clk: the clk being protected > > + * > > + * clk_protect can be used when a consumer depends on the clock rate and > > can't > > + * tolerate any glitches. The consumer protecting the clock can still make > > + * adjustment to clock, if it is the only one protecting the clock. Other > > + * consumers can still use the clock but won't be able to adjust the rate > > or > > + * reparent the clock while it is protected. > > + * clk_protect may sleep. > > + */ > > Maybe: > > """ > clk_protect begins a critical section during which the clock consumer > cannot tolerate any change to the clock rate. This results in all clocks > up the parent chain to also be rate-protected. > > Unlike the clk_set_rate_range method, which allows the rate to change > within a given range, protected clocks cannot have their rate changed, > either directly or indirectly due to changes further up the parent chain > of clocks. > > Calls to clk_protect should be balanced with calls to clk_unprotect. > Calls to this function may sleep, and do not return error status. > """ +1 > > > +void clk_protect(struct clk *clk) > > +{ > > + if (!clk) > > + return; > > + > > + clk_prepare_lock(); > > + clk_core_protect(clk->core); > > + clk->protect_ucount++; > > + clk_prepare_unlock(); > > +} > > +EXPORT_SYMBOL_GPL(clk_protect); > > + > > static void clk_core_disable(struct clk_core *core) > > { > > lockdep_assert_held(&enable_lock); > > @@ -838,7 +933,9 @@ static int clk_core_determine_round(struct clk_core > > *core, > > { > > long rate; > > > > - if (core->ops->determine_rate) { > > + if (clk_core_is_protected(core)) { > > + req->rate = core->rate; > > Hmm, in CCF we basically re-use round_rate/determine_rate from within > calls to clk_set_rate. The point is that clk_set_rate should set the > same rate that is returned from either of the other two functions. > > The change above breaks that subtle assumption, as a clk consumer can > now call: > > clk_protect(clk); > > rate = clk_get_rate(clk); // rate is 1234; > > rate = clk_round_rate(clk, 5678); // rate is STILL 1234 > > ret = clk_set_rate(clk, 5678); > > rate = clk_get_rate(clk); // rate is 5678 > > Is my understanding correct? If so then we might consider adding some > more complex knowledge to clk_round_rate. Something like: > > if clk_is_protected(clk) AND it is only protect by THIS clk: > round the rate > else: > return core->rate I may be wrong but I think it is already the way you want it. Please see below [0] > > > + } else if (core->ops->determine_rate) { > > return core->ops->determine_rate(core->hw, req); > > } else if (core->ops->round_rate) { > > rate = core->ops->round_rate(core->hw, req->rate, > > @@ -1381,7 +1478,7 @@ static struct clk_core *clk_calc_new_rates(struct > > clk_core *core, > > req.min_rate = min_rate; > > req.max_rate = max_rate; > > > > - clk_core_init_rate_req(core, req); > > + clk_core_init_rate_req(core, &req); > > Why isn't this change in patch #3? Seems like a bug introduced in patch > #3 and fixed here in patch #4. Indeed, I messed up while formatting the patches :( > > > > > ret = clk_core_determine_round(core, &req); > > if (ret < 0) > > @@ -1637,8 +1734,14 @@ int clk_set_rate(struct clk *clk, unsigned long rate) > > /* prevent racing with updates to the clock topology */ > > clk_prepare_lock(); > > The fact that a user can protect a clk AND change its rate is very > subtle. Please update the kerneldoc for clk_set_rate, clk_set_parent and > any other affected apis. Will do > > > > > + if (clk->protect_ucount) > > + clk_core_unprotect(clk->core); > > What happens here if two different users have both protected this same > clk? Looks like we ignore the second user? > > In other words, what happens if clk->protect_ucount == 2? We don't really care what is the value of the consumer count, what is important is whether it is null or not. This is linked to the point [0] above. Maybe it is not clear but here is how it (should) goes : Every time clock_rate_protect is called the consumer *and* core count is incremented. (consumer_count =< core_count) When an altering action is tried (set_rate, set_parent, ...), if the consumer count is not null, we call clk_core_unprotect which mean that the core_count is temporarily decremented by 1. It is safe to do so because we are holding the prepare_lock. When we get to + if (clk_core_is_protected(core)) { + req->rate = core->rate; Either the clock was only protected by the calling consumer, *and only once*, then the protection has been temporarily removed, test is false and the usual code will be executed, calling round/determine rate. If the test is true, it means the clock is still protected. This happens if * the clock is protected and we come from an unprotected consumer * the clock is protected by more than one consumer * the clock is protected more than one time by the same consumer - when there is multiple code path protecting the rate in a device driver, as previously discussed. That's why clk_core_unprotect decrements by 1 and not by the consumer_count. In such case, the clock is still protected, I think it make sense to return what is already set and not round/determine again. The clock was already rounded/determined when it wasn't protected. It this what you meant by : > if clk_is_protected(clk) AND it is only protect by THIS clk: ? Cheers Jerome > > > Best regards, > Mike > > > + > > ret = clk_core_set_rate_nolock(clk->core, rate); > > > > + if (clk->protect_ucount) > > + clk_core_protect(clk->core); > > + > > clk_prepare_unlock(); > > > > return ret; > > @@ -1669,12 +1772,18 @@ int clk_set_rate_range(struct clk *clk, unsigned > > long min, unsigned long max) > > > > clk_prepare_lock(); > > > > + if (clk->protect_ucount) > > + clk_core_unprotect(clk->core); > > + > > if (min != clk->min_rate || max != clk->max_rate) { > > clk->min_rate = min; > > clk->max_rate = max; > > ret = clk_core_set_rate_nolock(clk->core, clk->core- > > >req_rate); > > } > > > > + if (clk->protect_ucount) > > + clk_core_protect(clk->core); > > + > > clk_prepare_unlock(); > > > > return ret; > > @@ -1815,6 +1924,9 @@ static int clk_core_set_parent(struct clk_core *core, > > struct clk_core *parent) > > if ((core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) > > return -EBUSY; > > > > + if (clk_core_is_protected(core)) > > + return -EBUSY; > > + > > /* try finding the new parent index */ > > if (parent) { > > p_index = clk_fetch_parent_index(core, parent); > > @@ -1878,11 +1990,24 @@ static int clk_core_set_parent_lock(struct clk_core > > *core, > > */ > > int clk_set_parent(struct clk *clk, struct clk *parent) > > { > > + int ret; > > + > > if (!clk) > > return 0; > > > > - return clk_core_set_parent_lock(clk->core, > > - parent ? parent->core : NULL); > > + clk_prepare_lock(); > > + > > + if (clk->protect_ucount) > > + clk_core_unprotect(clk->core); > > + > > + ret = clk_core_set_parent(clk->core, parent ? parent->core : NULL); > > + > > + if (clk->protect_ucount) > > + clk_core_protect(clk->core); > > + > > + clk_prepare_unlock(); > > + > > + return ret; > > } > > EXPORT_SYMBOL_GPL(clk_set_parent); > > > > @@ -1893,7 +2018,10 @@ static int clk_core_set_phase(struct clk_core *core, > > int degrees) > > if (!core) > > return 0; > > > > - trace_clk_set_phase(clk->core, degrees); > > + if (clk_core_is_protected(core)) > > + return -EBUSY; > > + > > + trace_clk_set_phase(core, degrees); > > > > if (core->ops->set_phase) > > ret = core->ops->set_phase(core->hw, degrees); > > @@ -1936,7 +2064,15 @@ int clk_set_phase(struct clk *clk, int degrees) > > degrees += 360; > > > > clk_prepare_lock(); > > + > > + if (clk->protect_ucount) > > + clk_core_unprotect(clk->core); > > + > > ret = clk_core_set_phase(clk->core, degrees); > > + > > + if (clk->protect_ucount) > > + clk_core_protect(clk->core); > > + > > clk_prepare_unlock(); > > > > return ret; > > @@ -2023,11 +2159,12 @@ static void clk_summary_show_one(struct seq_file *s, > > struct clk_core *c, > > if (!c) > > return; > > > > - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", > > + seq_printf(s, "%*s%-*s %11d %12d %12d %11lu %10lu %-3d\n", > > level * 3 + 1, "", > > 30 - level * 3, c->name, > > - c->enable_count, c->prepare_count, clk_core_get_rate(c), > > - clk_core_get_accuracy(c), clk_core_get_phase(c)); > > + c->enable_count, c->prepare_count, c->protect_count, > > + clk_core_get_rate(c), clk_core_get_accuracy(c), > > + clk_core_get_phase(c)); > > } > > > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core > > *c, > > @@ -2049,8 +2186,8 @@ static int clk_summary_show(struct seq_file *s, void > > *data) > > struct clk_core *c; > > struct hlist_head **lists = (struct hlist_head **)s->private; > > > > - seq_puts(s, > > " clock enable_cnt prepare_cnt rate accu > > racy phase\n"); > > - seq_puts(s, "------------------------------------------------------- > > ---------------------------------\n"); > > + seq_puts(s, > > " clock enable_cnt prepare_cnt protect_cnt > > rate accuracy phase\n"); > > + seq_puts(s, "------------------------------------------------------- > > ---------------------------------------------\n"); > > > > clk_prepare_lock(); > > > > @@ -2085,6 +2222,7 @@ static void clk_dump_one(struct seq_file *s, struct > > clk_core *c, int level) > > seq_printf(s, "\"%s\": { ", c->name); > > seq_printf(s, "\"enable_count\": %d,", c->enable_count); > > seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); > > + seq_printf(s, "\"protect_count\": %d,", c->protect_count); > > seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); > > seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); > > seq_printf(s, "\"phase\": %d", clk_core_get_phase(c)); > > @@ -2191,6 +2329,11 @@ static int clk_debug_create_one(struct clk_core > > *core, struct dentry *pdentry) > > if (!d) > > goto err_out; > > > > + d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry, > > + (u32 *)&core->protect_count); > > + if (!d) > > + goto err_out; > > + > > d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry, > > (u32 *)&core->notifier_count); > > if (!d) > > @@ -2747,6 +2890,11 @@ void clk_unregister(struct clk *clk) > > if (clk->core->prepare_count) > > pr_warn("%s: unregistering prepared clock: %s\n", > > __func__, clk->core->name); > > + > > + if (clk->core->protect_count) > > + pr_warn("%s: unregistering protected clock: %s\n", > > + __func__, clk->core->name); > > + > > kref_put(&clk->core->ref, __clk_release); > > unlock: > > clk_prepare_unlock(); > > @@ -2905,6 +3053,15 @@ void __clk_put(struct clk *clk) > > > > clk_prepare_lock(); > > > > + /* Protect count not balanced: warn and sanitize */ > > + if (clk->protect_ucount) { > > + pr_warn("%s: releasing protected clock: %s\n", > > + __func__, clk->core->name); > > + > > + for (; clk->protect_ucount; clk->protect_ucount--) > > + clk_core_unprotect(clk->core); > > + } > > + > > hlist_del(&clk->clks_node); > > if (clk->min_rate > clk->core->req_rate || > > clk->max_rate < clk->core->req_rate) > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > index a428aec36ace..705a158d9b8f 100644 > > --- a/include/linux/clk-provider.h > > +++ b/include/linux/clk-provider.h > > @@ -739,6 +739,7 @@ unsigned long clk_hw_get_rate(const struct clk_hw *hw); > > unsigned long __clk_get_flags(struct clk *clk); > > unsigned long clk_hw_get_flags(const struct clk_hw *hw); > > bool clk_hw_is_prepared(const struct clk_hw *hw); > > +bool clk_hw_is_protected(const struct clk_hw *hw); > > bool clk_hw_is_enabled(const struct clk_hw *hw); > > bool __clk_is_enabled(struct clk *clk); > > struct clk *__clk_lookup(const char *name); > > diff --git a/include/linux/clk.h b/include/linux/clk.h > > index e9d36b3e49de..90b72ead4411 100644 > > --- a/include/linux/clk.h > > +++ b/include/linux/clk.h > > @@ -265,6 +265,30 @@ struct clk *devm_clk_get(struct device *dev, const char > > *id); > > */ > > struct clk *devm_get_clk_from_child(struct device *dev, > > struct device_node *np, const char > > *con_id); > > +/** > > + * clk_protect - inform the system when the clock source should be > > protected. > > + * @clk: clock source > > + * > > + * This function informs the system that the consumer protecting the clock > > + * depends on the rate of the clock source and can't tolerate any glitches > > + * introduced by further clock rate change or re-parenting of the clock > > source. > > + * > > + * Must not be called from within atomic context. > > + */ > > +void clk_protect(struct clk *clk); > > + > > +/** > > + * clk_unprotect - release the protection of the clock source. > > + * @clk: clock source > > + * > > + * This function informs the system that the consumer previously protecting > > the > > + * clock source can now deal with other consumer altering the clock source. > > + * > > + * The caller must balance the number of protect and unprotect calls. > > + * > > + * Must not be called from within atomic context. > > + */ > > +void clk_unprotect(struct clk *clk); > > > > /** > > * clk_enable - inform the system when the clock source should be running. > > @@ -460,6 +484,11 @@ static inline void clk_put(struct clk *clk) {} > > > > static inline void devm_clk_put(struct device *dev, struct clk *clk) {} > > > > + > > +static inline void clk_protect(struct clk *clk) {} > > + > > +static inline void clk_unprotect(struct clk *clk) {} > > + > > static inline int clk_enable(struct clk *clk) > > { > > return 0; > > -- > > 2.9.3 > > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jerome, Quoting Jerome Brunet (2017-05-12 06:08:02) > On Thu, 2017-05-11 at 12:05 -0700, Michael Turquette wrote: > > Quoting Jerome Brunet (2017-03-21 11:33:27) <snip> > > > @@ -838,7 +933,9 @@ static int clk_core_determine_round(struct clk_core > > > *core, > > > { > > > long rate; > > > > > > - if (core->ops->determine_rate) { > > > + if (clk_core_is_protected(core)) { > > > + req->rate = core->rate; > > > > Hmm, in CCF we basically re-use round_rate/determine_rate from within > > calls to clk_set_rate. The point is that clk_set_rate should set the > > same rate that is returned from either of the other two functions. > > > > The change above breaks that subtle assumption, as a clk consumer can > > now call: > > > > clk_protect(clk); > > > > rate = clk_get_rate(clk); // rate is 1234; > > > > rate = clk_round_rate(clk, 5678); // rate is STILL 1234 > > > > ret = clk_set_rate(clk, 5678); > > > > rate = clk_get_rate(clk); // rate is 5678 > > > > Is my understanding correct? If so then we might consider adding some > > more complex knowledge to clk_round_rate. Something like: > > > > if clk_is_protected(clk) AND it is only protect by THIS clk: > > round the rate > > else: > > return core->rate > > I may be wrong but I think it is already the way you want it. Please see below > [0] <snip> > > > + if (clk->protect_ucount) > > > + clk_core_unprotect(clk->core); > > > > What happens here if two different users have both protected this same > > clk? Looks like we ignore the second user? > > > > In other words, what happens if clk->protect_ucount == 2? > > We don't really care what is the value of the consumer count, what is important > is whether it is null or not. This is linked to the point [0] above. Maybe it is > not clear but here is how it (should) goes : > > Every time clock_rate_protect is called the consumer *and* core count is > incremented. (consumer_count =< core_count) > > When an altering action is tried (set_rate, set_parent, ...), if the consumer > count is not null, we call clk_core_unprotect which mean that the core_count is > temporarily decremented by 1. It is safe to do so because we are holding the > prepare_lock. > > When we get to > + if (clk_core_is_protected(core)) { > + req->rate = core->rate; > > Either the clock was only protected by the calling consumer, *and only once*, > then the protection has been temporarily removed, test is false and the usual > code will be executed, calling round/determine rate. > > If the test is true, it means the clock is still protected. This happens if > * the clock is protected and we come from an unprotected consumer > * the clock is protected by more than one consumer > * the clock is protected more than one time by the same consumer - when there is > multiple code path protecting the rate in a device driver, as previously > discussed. That's why clk_core_unprotect decrements by 1 and not by the > consumer_count. > > In such case, the clock is still protected, I think it make sense to return what > is already set and not round/determine again. The clock was already > rounded/determined when it wasn't protected. > > It this what you meant by : > > if clk_is_protected(clk) AND it is only protect by THIS clk: > ? Yes, you've answered my question and I'm happy with that. When reviewing the code before I missed the subtle point that we test for clk->protect_ucount but then only modify core->protect_count. I think a one-line comment above those conditional statements would be nice. I do have a doubt about how clk_round_rate works now. Is it correct that we do not ever decrement the protect count when a protecting consumer calls clk_round_rate? It seems to me that a clk consumer should be able to call clk_protect() and then clk_round_rate() and get a real rounded rate instead req->rate = core->rate, assuming that the core protect count is decremented to zero. In other words, can you make clk_round_rate work like clk_set_rate and clk_set_parent? Regards, Mike > > Cheers > Jerome > > > > > > > Best regards, > > Mike > > > > > + > > > ret = clk_core_set_rate_nolock(clk->core, rate); > > > > > > + if (clk->protect_ucount) > > > + clk_core_protect(clk->core); > > > + > > > clk_prepare_unlock(); > > > > > > return ret; > > > @@ -1669,12 +1772,18 @@ int clk_set_rate_range(struct clk *clk, unsigned > > > long min, unsigned long max) > > > > > > clk_prepare_lock(); > > > > > > + if (clk->protect_ucount) > > > + clk_core_unprotect(clk->core); > > > + > > > if (min != clk->min_rate || max != clk->max_rate) { > > > clk->min_rate = min; > > > clk->max_rate = max; > > > ret = clk_core_set_rate_nolock(clk->core, clk->core- > > > >req_rate); > > > } > > > > > > + if (clk->protect_ucount) > > > + clk_core_protect(clk->core); > > > + > > > clk_prepare_unlock(); > > > > > > return ret; > > > @@ -1815,6 +1924,9 @@ static int clk_core_set_parent(struct clk_core *core, > > > struct clk_core *parent) > > > if ((core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) > > > return -EBUSY; > > > > > > + if (clk_core_is_protected(core)) > > > + return -EBUSY; > > > + > > > /* try finding the new parent index */ > > > if (parent) { > > > p_index = clk_fetch_parent_index(core, parent); > > > @@ -1878,11 +1990,24 @@ static int clk_core_set_parent_lock(struct clk_core > > > *core, > > > */ > > > int clk_set_parent(struct clk *clk, struct clk *parent) > > > { > > > + int ret; > > > + > > > if (!clk) > > > return 0; > > > > > > - return clk_core_set_parent_lock(clk->core, > > > - parent ? parent->core : NULL); > > > + clk_prepare_lock(); > > > + > > > + if (clk->protect_ucount) > > > + clk_core_unprotect(clk->core); > > > + > > > + ret = clk_core_set_parent(clk->core, parent ? parent->core : NULL); > > > + > > > + if (clk->protect_ucount) > > > + clk_core_protect(clk->core); > > > + > > > + clk_prepare_unlock(); > > > + > > > + return ret; > > > } > > > EXPORT_SYMBOL_GPL(clk_set_parent); > > > > > > @@ -1893,7 +2018,10 @@ static int clk_core_set_phase(struct clk_core *core, > > > int degrees) > > > if (!core) > > > return 0; > > > > > > - trace_clk_set_phase(clk->core, degrees); > > > + if (clk_core_is_protected(core)) > > > + return -EBUSY; > > > + > > > + trace_clk_set_phase(core, degrees); > > > > > > if (core->ops->set_phase) > > > ret = core->ops->set_phase(core->hw, degrees); > > > @@ -1936,7 +2064,15 @@ int clk_set_phase(struct clk *clk, int degrees) > > > degrees += 360; > > > > > > clk_prepare_lock(); > > > + > > > + if (clk->protect_ucount) > > > + clk_core_unprotect(clk->core); > > > + > > > ret = clk_core_set_phase(clk->core, degrees); > > > + > > > + if (clk->protect_ucount) > > > + clk_core_protect(clk->core); > > > + > > > clk_prepare_unlock(); > > > > > > return ret; > > > @@ -2023,11 +2159,12 @@ static void clk_summary_show_one(struct seq_file *s, > > > struct clk_core *c, > > > if (!c) > > > return; > > > > > > - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", > > > + seq_printf(s, "%*s%-*s %11d %12d %12d %11lu %10lu %-3d\n", > > > level * 3 + 1, "", > > > 30 - level * 3, c->name, > > > - c->enable_count, c->prepare_count, clk_core_get_rate(c), > > > - clk_core_get_accuracy(c), clk_core_get_phase(c)); > > > + c->enable_count, c->prepare_count, c->protect_count, > > > + clk_core_get_rate(c), clk_core_get_accuracy(c), > > > + clk_core_get_phase(c)); > > > } > > > > > > static void clk_summary_show_subtree(struct seq_file *s, struct clk_core > > > *c, > > > @@ -2049,8 +2186,8 @@ static int clk_summary_show(struct seq_file *s, void > > > *data) > > > struct clk_core *c; > > > struct hlist_head **lists = (struct hlist_head **)s->private; > > > > > > - seq_puts(s, > > > " clock enable_cnt prepare_cnt rate accu > > > racy phase\n"); > > > - seq_puts(s, "------------------------------------------------------- > > > ---------------------------------\n"); > > > + seq_puts(s, > > > " clock enable_cnt prepare_cnt protect_cnt > > > rate accuracy phase\n"); > > > + seq_puts(s, "------------------------------------------------------- > > > ---------------------------------------------\n"); > > > > > > clk_prepare_lock(); > > > > > > @@ -2085,6 +2222,7 @@ static void clk_dump_one(struct seq_file *s, struct > > > clk_core *c, int level) > > > seq_printf(s, "\"%s\": { ", c->name); > > > seq_printf(s, "\"enable_count\": %d,", c->enable_count); > > > seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); > > > + seq_printf(s, "\"protect_count\": %d,", c->protect_count); > > > seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); > > > seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); > > > seq_printf(s, "\"phase\": %d", clk_core_get_phase(c)); > > > @@ -2191,6 +2329,11 @@ static int clk_debug_create_one(struct clk_core > > > *core, struct dentry *pdentry) > > > if (!d) > > > goto err_out; > > > > > > + d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry, > > > + (u32 *)&core->protect_count); > > > + if (!d) > > > + goto err_out; > > > + > > > d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry, > > > (u32 *)&core->notifier_count); > > > if (!d) > > > @@ -2747,6 +2890,11 @@ void clk_unregister(struct clk *clk) > > > if (clk->core->prepare_count) > > > pr_warn("%s: unregistering prepared clock: %s\n", > > > __func__, clk->core->name); > > > + > > > + if (clk->core->protect_count) > > > + pr_warn("%s: unregistering protected clock: %s\n", > > > + __func__, clk->core->name); > > > + > > > kref_put(&clk->core->ref, __clk_release); > > > unlock: > > > clk_prepare_unlock(); > > > @@ -2905,6 +3053,15 @@ void __clk_put(struct clk *clk) > > > > > > clk_prepare_lock(); > > > > > > + /* Protect count not balanced: warn and sanitize */ > > > + if (clk->protect_ucount) { > > > + pr_warn("%s: releasing protected clock: %s\n", > > > + __func__, clk->core->name); > > > + > > > + for (; clk->protect_ucount; clk->protect_ucount--) > > > + clk_core_unprotect(clk->core); > > > + } > > > + > > > hlist_del(&clk->clks_node); > > > if (clk->min_rate > clk->core->req_rate || > > > clk->max_rate < clk->core->req_rate) > > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > > > index a428aec36ace..705a158d9b8f 100644 > > > --- a/include/linux/clk-provider.h > > > +++ b/include/linux/clk-provider.h > > > @@ -739,6 +739,7 @@ unsigned long clk_hw_get_rate(const struct clk_hw *hw); > > > unsigned long __clk_get_flags(struct clk *clk); > > > unsigned long clk_hw_get_flags(const struct clk_hw *hw); > > > bool clk_hw_is_prepared(const struct clk_hw *hw); > > > +bool clk_hw_is_protected(const struct clk_hw *hw); > > > bool clk_hw_is_enabled(const struct clk_hw *hw); > > > bool __clk_is_enabled(struct clk *clk); > > > struct clk *__clk_lookup(const char *name); > > > diff --git a/include/linux/clk.h b/include/linux/clk.h > > > index e9d36b3e49de..90b72ead4411 100644 > > > --- a/include/linux/clk.h > > > +++ b/include/linux/clk.h > > > @@ -265,6 +265,30 @@ struct clk *devm_clk_get(struct device *dev, const char > > > *id); > > > */ > > > struct clk *devm_get_clk_from_child(struct device *dev, > > > struct device_node *np, const char > > > *con_id); > > > +/** > > > + * clk_protect - inform the system when the clock source should be > > > protected. > > > + * @clk: clock source > > > + * > > > + * This function informs the system that the consumer protecting the clock > > > + * depends on the rate of the clock source and can't tolerate any glitches > > > + * introduced by further clock rate change or re-parenting of the clock > > > source. > > > + * > > > + * Must not be called from within atomic context. > > > + */ > > > +void clk_protect(struct clk *clk); > > > + > > > +/** > > > + * clk_unprotect - release the protection of the clock source. > > > + * @clk: clock source > > > + * > > > + * This function informs the system that the consumer previously protecting > > > the > > > + * clock source can now deal with other consumer altering the clock source. > > > + * > > > + * The caller must balance the number of protect and unprotect calls. > > > + * > > > + * Must not be called from within atomic context. > > > + */ > > > +void clk_unprotect(struct clk *clk); > > > > > > /** > > > * clk_enable - inform the system when the clock source should be running. > > > @@ -460,6 +484,11 @@ static inline void clk_put(struct clk *clk) {} > > > > > > static inline void devm_clk_put(struct device *dev, struct clk *clk) {} > > > > > > + > > > +static inline void clk_protect(struct clk *clk) {} > > > + > > > +static inline void clk_unprotect(struct clk *clk) {} > > > + > > > static inline int clk_enable(struct clk *clk) > > > { > > > return 0; > > > -- > > > 2.9.3 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index fa77a1841e0f..69db8cc15063 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -60,6 +60,7 @@ struct clk_core { bool orphan; unsigned int enable_count; unsigned int prepare_count; + unsigned int protect_count; unsigned long min_rate; unsigned long max_rate; unsigned long accuracy; @@ -84,6 +85,7 @@ struct clk { const char *con_id; unsigned long min_rate; unsigned long max_rate; + unsigned long protect_ucount; struct hlist_node clks_node; }; @@ -160,6 +162,11 @@ static bool clk_core_is_prepared(struct clk_core *core) return core->ops->is_prepared(core->hw); } +static bool clk_core_is_protected(struct clk_core *core) +{ + return core->protect_count; +} + static bool clk_core_is_enabled(struct clk_core *core) { /* @@ -328,6 +335,11 @@ bool clk_hw_is_prepared(const struct clk_hw *hw) return clk_core_is_prepared(hw->core); } +bool clk_hw_is_protected(const struct clk_hw *hw) +{ + return clk_core_is_protected(hw->core); +} + bool clk_hw_is_enabled(const struct clk_hw *hw) { return clk_core_is_enabled(hw->core); @@ -584,6 +596,89 @@ int clk_prepare(struct clk *clk) } EXPORT_SYMBOL_GPL(clk_prepare); +static void clk_core_unprotect(struct clk_core *core) +{ + lockdep_assert_held(&prepare_lock); + + if (!core) + return; + + if (WARN_ON(core->protect_count == 0)) + return; + + if (--core->protect_count > 0) + return; + + clk_core_unprotect(core->parent); +} + +/** + * clk_unprotect - unprotect a clock source + * @clk: the clk being unprotected + * + * clk_unprotect shall be used when a consumer no longer depends on the clock + * rate and can tolerate glitches. As with clk_unprepare and clk_enable, calls + * to clk_unprotect must be balanced with clk_protect. + * clk_protect may sleep + */ +void clk_unprotect(struct clk *clk) +{ + if (!clk) + return; + + clk_prepare_lock(); + + if (WARN_ON(clk->protect_ucount <= 0)) { + /* + * There is something wrong with this consumer protect count. + * Stop here before messing with the provider + */ + clk_prepare_unlock(); + return; + } + + clk_core_unprotect(clk->core); + clk->protect_ucount--; + clk_prepare_unlock(); +} +EXPORT_SYMBOL_GPL(clk_unprotect); + +static void clk_core_protect(struct clk_core *core) +{ + lockdep_assert_held(&prepare_lock); + + if (!core) + return; + + if (core->protect_count == 0) + clk_core_protect(core->parent); + + core->protect_count++; +} + +/** + * clk_protect - protect a clock source + * @clk: the clk being protected + * + * clk_protect can be used when a consumer depends on the clock rate and can't + * tolerate any glitches. The consumer protecting the clock can still make + * adjustment to clock, if it is the only one protecting the clock. Other + * consumers can still use the clock but won't be able to adjust the rate or + * reparent the clock while it is protected. + * clk_protect may sleep. + */ +void clk_protect(struct clk *clk) +{ + if (!clk) + return; + + clk_prepare_lock(); + clk_core_protect(clk->core); + clk->protect_ucount++; + clk_prepare_unlock(); +} +EXPORT_SYMBOL_GPL(clk_protect); + static void clk_core_disable(struct clk_core *core) { lockdep_assert_held(&enable_lock); @@ -838,7 +933,9 @@ static int clk_core_determine_round(struct clk_core *core, { long rate; - if (core->ops->determine_rate) { + if (clk_core_is_protected(core)) { + req->rate = core->rate; + } else if (core->ops->determine_rate) { return core->ops->determine_rate(core->hw, req); } else if (core->ops->round_rate) { rate = core->ops->round_rate(core->hw, req->rate, @@ -1381,7 +1478,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *core, req.min_rate = min_rate; req.max_rate = max_rate; - clk_core_init_rate_req(core, req); + clk_core_init_rate_req(core, &req); ret = clk_core_determine_round(core, &req); if (ret < 0) @@ -1637,8 +1734,14 @@ int clk_set_rate(struct clk *clk, unsigned long rate) /* prevent racing with updates to the clock topology */ clk_prepare_lock(); + if (clk->protect_ucount) + clk_core_unprotect(clk->core); + ret = clk_core_set_rate_nolock(clk->core, rate); + if (clk->protect_ucount) + clk_core_protect(clk->core); + clk_prepare_unlock(); return ret; @@ -1669,12 +1772,18 @@ int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max) clk_prepare_lock(); + if (clk->protect_ucount) + clk_core_unprotect(clk->core); + if (min != clk->min_rate || max != clk->max_rate) { clk->min_rate = min; clk->max_rate = max; ret = clk_core_set_rate_nolock(clk->core, clk->core->req_rate); } + if (clk->protect_ucount) + clk_core_protect(clk->core); + clk_prepare_unlock(); return ret; @@ -1815,6 +1924,9 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent) if ((core->flags & CLK_SET_PARENT_GATE) && core->prepare_count) return -EBUSY; + if (clk_core_is_protected(core)) + return -EBUSY; + /* try finding the new parent index */ if (parent) { p_index = clk_fetch_parent_index(core, parent); @@ -1878,11 +1990,24 @@ static int clk_core_set_parent_lock(struct clk_core *core, */ int clk_set_parent(struct clk *clk, struct clk *parent) { + int ret; + if (!clk) return 0; - return clk_core_set_parent_lock(clk->core, - parent ? parent->core : NULL); + clk_prepare_lock(); + + if (clk->protect_ucount) + clk_core_unprotect(clk->core); + + ret = clk_core_set_parent(clk->core, parent ? parent->core : NULL); + + if (clk->protect_ucount) + clk_core_protect(clk->core); + + clk_prepare_unlock(); + + return ret; } EXPORT_SYMBOL_GPL(clk_set_parent); @@ -1893,7 +2018,10 @@ static int clk_core_set_phase(struct clk_core *core, int degrees) if (!core) return 0; - trace_clk_set_phase(clk->core, degrees); + if (clk_core_is_protected(core)) + return -EBUSY; + + trace_clk_set_phase(core, degrees); if (core->ops->set_phase) ret = core->ops->set_phase(core->hw, degrees); @@ -1936,7 +2064,15 @@ int clk_set_phase(struct clk *clk, int degrees) degrees += 360; clk_prepare_lock(); + + if (clk->protect_ucount) + clk_core_unprotect(clk->core); + ret = clk_core_set_phase(clk->core, degrees); + + if (clk->protect_ucount) + clk_core_protect(clk->core); + clk_prepare_unlock(); return ret; @@ -2023,11 +2159,12 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, if (!c) return; - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n", + seq_printf(s, "%*s%-*s %11d %12d %12d %11lu %10lu %-3d\n", level * 3 + 1, "", 30 - level * 3, c->name, - c->enable_count, c->prepare_count, clk_core_get_rate(c), - clk_core_get_accuracy(c), clk_core_get_phase(c)); + c->enable_count, c->prepare_count, c->protect_count, + clk_core_get_rate(c), clk_core_get_accuracy(c), + clk_core_get_phase(c)); } static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, @@ -2049,8 +2186,8 @@ static int clk_summary_show(struct seq_file *s, void *data) struct clk_core *c; struct hlist_head **lists = (struct hlist_head **)s->private; - seq_puts(s, " clock enable_cnt prepare_cnt rate accuracy phase\n"); - seq_puts(s, "----------------------------------------------------------------------------------------\n"); + seq_puts(s, " clock enable_cnt prepare_cnt protect_cnt rate accuracy phase\n"); + seq_puts(s, "----------------------------------------------------------------------------------------------------\n"); clk_prepare_lock(); @@ -2085,6 +2222,7 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) seq_printf(s, "\"%s\": { ", c->name); seq_printf(s, "\"enable_count\": %d,", c->enable_count); seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); + seq_printf(s, "\"protect_count\": %d,", c->protect_count); seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); seq_printf(s, "\"phase\": %d", clk_core_get_phase(c)); @@ -2191,6 +2329,11 @@ static int clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) if (!d) goto err_out; + d = debugfs_create_u32("clk_protect_count", S_IRUGO, core->dentry, + (u32 *)&core->protect_count); + if (!d) + goto err_out; + d = debugfs_create_u32("clk_notifier_count", S_IRUGO, core->dentry, (u32 *)&core->notifier_count); if (!d) @@ -2747,6 +2890,11 @@ void clk_unregister(struct clk *clk) if (clk->core->prepare_count) pr_warn("%s: unregistering prepared clock: %s\n", __func__, clk->core->name); + + if (clk->core->protect_count) + pr_warn("%s: unregistering protected clock: %s\n", + __func__, clk->core->name); + kref_put(&clk->core->ref, __clk_release); unlock: clk_prepare_unlock(); @@ -2905,6 +3053,15 @@ void __clk_put(struct clk *clk) clk_prepare_lock(); + /* Protect count not balanced: warn and sanitize */ + if (clk->protect_ucount) { + pr_warn("%s: releasing protected clock: %s\n", + __func__, clk->core->name); + + for (; clk->protect_ucount; clk->protect_ucount--) + clk_core_unprotect(clk->core); + } + hlist_del(&clk->clks_node); if (clk->min_rate > clk->core->req_rate || clk->max_rate < clk->core->req_rate) diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index a428aec36ace..705a158d9b8f 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -739,6 +739,7 @@ unsigned long clk_hw_get_rate(const struct clk_hw *hw); unsigned long __clk_get_flags(struct clk *clk); unsigned long clk_hw_get_flags(const struct clk_hw *hw); bool clk_hw_is_prepared(const struct clk_hw *hw); +bool clk_hw_is_protected(const struct clk_hw *hw); bool clk_hw_is_enabled(const struct clk_hw *hw); bool __clk_is_enabled(struct clk *clk); struct clk *__clk_lookup(const char *name); diff --git a/include/linux/clk.h b/include/linux/clk.h index e9d36b3e49de..90b72ead4411 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -265,6 +265,30 @@ struct clk *devm_clk_get(struct device *dev, const char *id); */ struct clk *devm_get_clk_from_child(struct device *dev, struct device_node *np, const char *con_id); +/** + * clk_protect - inform the system when the clock source should be protected. + * @clk: clock source + * + * This function informs the system that the consumer protecting the clock + * depends on the rate of the clock source and can't tolerate any glitches + * introduced by further clock rate change or re-parenting of the clock source. + * + * Must not be called from within atomic context. + */ +void clk_protect(struct clk *clk); + +/** + * clk_unprotect - release the protection of the clock source. + * @clk: clock source + * + * This function informs the system that the consumer previously protecting the + * clock source can now deal with other consumer altering the clock source. + * + * The caller must balance the number of protect and unprotect calls. + * + * Must not be called from within atomic context. + */ +void clk_unprotect(struct clk *clk); /** * clk_enable - inform the system when the clock source should be running. @@ -460,6 +484,11 @@ static inline void clk_put(struct clk *clk) {} static inline void devm_clk_put(struct device *dev, struct clk *clk) {} + +static inline void clk_protect(struct clk *clk) {} + +static inline void clk_unprotect(struct clk *clk) {} + static inline int clk_enable(struct clk *clk) { return 0;
The patch adds clk_protect and clk_unprotect to the CCF API. These functions allow a consumer to inform the system that the rate of clock is critical to for its operations and it can't tolerate other consumers changing the rate or introducing glitches while the clock is protected. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/clk/clk.c | 177 ++++++++++++++++++++++++++++++++++++++++--- include/linux/clk-provider.h | 1 + include/linux/clk.h | 29 +++++++ 3 files changed, 197 insertions(+), 10 deletions(-)