Quoting Boris BREZILLON (2013-10-13 10:17:10) > The clock accuracy is expressed in ppb (parts per billion) and represents > the possible clock drift. > Say you have a clock (e.g. an oscillator) which provides a fixed clock of > 20MHz with an accuracy of +- 20Hz. This accuracy expressed in ppb is > 20Hz/20MHz = 1000 ppb (or 1 ppm). > > Clock users may need the clock accuracy information in order to choose > the best clock (the one with the best accuracy) across several available > clocks. > > This patch adds clk accuracy retrieval support for common clk framework by > means of a new function called clk_get_accuracy. > This function returns the given clock accuracy expressed in ppb. > > In order to get the clock accuracy, this implementation adds one callback > called recalc_accuracy to the clk_ops structure. > This callback is given the parent clock accuracy (if the clock is not a > root clock) and should recalculate the given clock accuracy. > > This callback is optional and may be implemented if the clock is not > a perfect clock (accuracy != 0 ppb). > > Signed-off-by: Boris BREZILLON <b.brezillon@xxxxxxxxxxx> > --- > Documentation/clk.txt | 4 ++ > drivers/clk/Kconfig | 4 ++ > drivers/clk/clk.c | 92 ++++++++++++++++++++++++++++++++++++++++-- > include/linux/clk-private.h | 1 + > include/linux/clk-provider.h | 11 +++++ > include/linux/clk.h | 17 ++++++++ > 6 files changed, 125 insertions(+), 4 deletions(-) > > diff --git a/Documentation/clk.txt b/Documentation/clk.txt > index 3aeb5c4..dc52da1 100644 > --- a/Documentation/clk.txt > +++ b/Documentation/clk.txt > @@ -77,6 +77,8 @@ the operations defined in clk.h: > int (*set_parent)(struct clk_hw *hw, u8 index); > u8 (*get_parent)(struct clk_hw *hw); > int (*set_rate)(struct clk_hw *hw, unsigned long); > + unsigned long (*recalc_accuracy)(struct clk_hw *hw, > + unsigned long parent_accuracy); > void (*init)(struct clk_hw *hw); > }; > > @@ -202,6 +204,8 @@ optional or must be evaluated on a case-by-case basis. > .set_parent | | | n | y | n | > .get_parent | | | n | y | n | > | | | | | | > +.recalc_rate | | | | | | s/recalc_rate/recalc_accuracy/ > + | | | | | | > .init | | | | | | > ----------------------------------------------------------- > [1] either one of round_rate or determine_rate is required. > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 279407a..4d12ae7 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -6,12 +6,16 @@ config CLKDEV_LOOKUP > config HAVE_CLK_PREPARE > bool > > +config HAVE_CLK_GET_ACCURACY > + bool > + This sort of thing gets messy. For platforms converted to common struct clk we select HAVE_CLK_PREPARE and we let legacy platforms select it on a case-by-case basis. For something like HAVE_CLK_GET_ACCURACY I am inclined to only add it for platforms converted to the common struct clk and not even expose it to legacy clock framework implementations. In those cases the call to clk_get_accuracy would return -EINVAL or -EPERM or something. Russell, any thoughts on that approach? > config HAVE_MACH_CLKDEV > bool > > config COMMON_CLK > bool > select HAVE_CLK_PREPARE > + select HAVE_CLK_GET_ACCURACY > select CLKDEV_LOOKUP > ---help--- > The common clock framework is a single definition of struct > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index a004769..6a8f3ef 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -104,10 +104,11 @@ static void clk_summary_show_one(struct seq_file *s, struct clk *c, int level) > if (!c) > return; > > - seq_printf(s, "%*s%-*s %-11d %-12d %-10lu", > + seq_printf(s, "%*s%-*s %-11d %-12d %-10lu %-11lu", > level * 3 + 1, "", > 30 - level * 3, c->name, > - c->enable_count, c->prepare_count, clk_get_rate(c)); > + c->enable_count, c->prepare_count, clk_get_rate(c), > + clk_get_accuracy(c)); > seq_printf(s, "\n"); > } > > @@ -129,8 +130,8 @@ static int clk_summary_show(struct seq_file *s, void *data) > { > struct clk *c; > > - seq_printf(s, " clock enable_cnt prepare_cnt rate\n"); > - seq_printf(s, "---------------------------------------------------------------------\n"); > + seq_printf(s, " clock enable_cnt prepare_cnt rate accuracy\n"); > + seq_printf(s, "---------------------------------------------------------------------------------\n"); > > clk_prepare_lock(); > > @@ -167,6 +168,7 @@ static void clk_dump_one(struct seq_file *s, struct clk *c, int level) > seq_printf(s, "\"enable_count\": %d,", c->enable_count); > seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); > seq_printf(s, "\"rate\": %lu", clk_get_rate(c)); > + seq_printf(s, "\"accuracy\": %lu", clk_get_accuracy(c)); > } > > static void clk_dump_subtree(struct seq_file *s, struct clk *c, int level) > @@ -248,6 +250,11 @@ static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry) > if (!d) > goto err_out; > > + d = debugfs_create_u32("clk_accuracy", S_IRUGO, clk->dentry, > + (u32 *)&clk->accuracy); > + if (!d) > + goto err_out; > + > d = debugfs_create_x32("clk_flags", S_IRUGO, clk->dentry, > (u32 *)&clk->flags); > if (!d) > @@ -602,6 +609,11 @@ out: > return ret; > } > > +unsigned long __clk_get_accuracy(struct clk *clk) > +{ > + return !clk ? 0 : clk->accuracy; > +} > + > unsigned long __clk_get_flags(struct clk *clk) > { > return !clk ? 0 : clk->flags; > @@ -1016,6 +1028,59 @@ static int __clk_notify(struct clk *clk, unsigned long msg, > } > > /** > + * __clk_recalc_accuracies > + * @clk: first clk in the subtree > + * @msg: notification type (see include/linux/clk.h) > + * > + * Walks the subtree of clks starting with clk and recalculates accuracies as > + * it goes. Note that if a clk does not implement the .recalc_rate callback > + * then it is assumed that the clock will take on the rate of it's parent. > + * > + * Caller must hold prepare_lock. > + */ > +static void __clk_recalc_accuracies(struct clk *clk) > +{ > + unsigned long parent_accuracy = 0; > + struct clk *child; > + > + if (clk->parent) > + parent_accuracy = clk->parent->accuracy; > + > + if (clk->ops->recalc_accuracy) > + clk->accuracy = clk->ops->recalc_accuracy(clk->hw, > + parent_accuracy); > + else > + clk->accuracy = parent_accuracy; > + > + hlist_for_each_entry(child, &clk->children, child_node) > + __clk_recalc_accuracies(child); > +} > + > +/** > + * clk_get_accuracy - return the accuracy of clk > + * @clk: the clk whose accuracy is being returned > + * > + * Simply returns the cached accuracy of the clk, unless > + * CLK_GET_ACCURACY_NOCACHE flag is set, which means a recalc_rate will be > + * issued. > + * If clk is NULL then returns 0. > + */ > +unsigned long clk_get_accuracy(struct clk *clk) > +{ > + unsigned long accuracy; > + clk_prepare_lock(); > + > + if (clk && (clk->flags & CLK_GET_ACCURACY_NOCACHE)) > + __clk_recalc_accuracies(clk); I think that there is some overlap between recalculating the accuracy here and simply getting it. You only provide clk_get_accuracy and it serves both purposes. It would be better if clk_recalc_accuracy walked the subtree of children and if clk_get_accuracy simply returned a cached value. > + > + accuracy = __clk_get_accuracy(clk); > + clk_prepare_unlock(); > + > + return accuracy; > +} > +EXPORT_SYMBOL_GPL(clk_get_accuracy); > + > +/** > * __clk_recalc_rates > * @clk: first clk in the subtree > * @msg: notification type (see include/linux/clk.h) > @@ -1545,6 +1610,7 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent) > { > clk_reparent(clk, new_parent); > clk_debug_reparent(clk, new_parent); > + __clk_recalc_accuracies(clk); Similar to the above statement. Why do this here? We do this for rates since calls to clk_get_rate will return the cached rate (unless the NOCACHE flag is set). But since a call to clk_get_accuracy will always recalculate it then there is no benefit to doing that here. > __clk_recalc_rates(clk, POST_RATE_CHANGE); > } > > @@ -1615,6 +1681,9 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > /* do the re-parent */ > ret = __clk_set_parent(clk, parent, p_index); > > + /* propagate accuracy recalculation */ > + __clk_recalc_accuracies(clk); Ditto. Regards, Mike > + > /* propagate rate recalculation accordingly */ > if (ret) > __clk_recalc_rates(clk, ABORT_RATE_CHANGE); > @@ -1724,6 +1793,21 @@ int __clk_init(struct device *dev, struct clk *clk) > hlist_add_head(&clk->child_node, &clk_orphan_list); > > /* > + * Set clk's accuracy. The preferred method is to use > + * .recalc_accuracy. For simple clocks and lazy developers the default > + * fallback is to use the parent's accuracy. If a clock doesn't have a > + * parent (or is orphaned) then accuracy is set to zero (perfect > + * clock). > + */ > + if (clk->ops->recalc_accuracy) > + clk->accuracy = clk->ops->recalc_accuracy(clk->hw, > + __clk_get_accuracy(clk->parent)); > + else if (clk->parent) > + clk->accuracy = clk->parent->accuracy; > + else > + clk->accuracy = 0; > + > + /* > * Set clk's rate. The preferred method is to use .recalc_rate. For > * simple clocks and lazy developers the default fallback is to use the > * parent's rate. If a clock doesn't have a parent (or is orphaned) > diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h > index 8138c94..accb517 100644 > --- a/include/linux/clk-private.h > +++ b/include/linux/clk-private.h > @@ -41,6 +41,7 @@ struct clk { > unsigned long flags; > unsigned int enable_count; > unsigned int prepare_count; > + unsigned long accuracy; > struct hlist_head children; > struct hlist_node child_node; > unsigned int notifier_count; > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 73bdb69..942811d 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -29,6 +29,7 @@ > #define CLK_IS_BASIC BIT(5) /* Basic clk, can't do a to_clk_foo() */ > #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */ > #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */ > +#define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */ > > struct clk_hw; > > @@ -108,6 +109,13 @@ struct clk_hw; > * which is likely helpful for most .set_rate implementation. > * Returns 0 on success, -EERROR otherwise. > * > + * @recalc_accuracy: Recalculate the accuracy of this clock. The clock accuracy > + * is expressed in ppb (parts per billion). The parent accuracy is > + * an input parameter. > + * Returns the calculated accuracy. Optional - if this op is not > + * set then clock accuracy will be initialized to parent accuracy > + * or 0 (perfect clock) if clock has no parent. > + * > * The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow > * implementations to split any work between atomic (enable) and sleepable > * (prepare) contexts. If enabling a clock requires code that might sleep, > @@ -139,6 +147,8 @@ struct clk_ops { > u8 (*get_parent)(struct clk_hw *hw); > int (*set_rate)(struct clk_hw *hw, unsigned long, > unsigned long); > + unsigned long (*recalc_accuracy)(struct clk_hw *hw, > + unsigned long parent_accuracy); > void (*init)(struct clk_hw *hw); > }; > > @@ -194,6 +204,7 @@ struct clk_hw { > struct clk_fixed_rate { > struct clk_hw hw; > unsigned long fixed_rate; > + unsigned long fixed_accuracy; > u8 flags; > }; > > diff --git a/include/linux/clk.h b/include/linux/clk.h > index 9a6d045..2fe3b54 100644 > --- a/include/linux/clk.h > +++ b/include/linux/clk.h > @@ -85,6 +85,23 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb); > #endif > > /** > + * clk_get_accuracy - obtain the clock accuracy in ppb (parts per billion) > + * for a clock source. > + * @clk: clock source > + * > + * This gets the clock source accuracy expressed in ppb. > + * A perfect clock returns 0. > + */ > +#ifdef CONFIG_HAVE_CLK_GET_ACCURACY > +unsigned long clk_get_accuracy(struct clk *clk); > +#else > +static inline unsigned long clk_get_accuracy(struct clk *clk) > +{ > + return 0; > +} > +#endif > + > +/** > * clk_prepare - prepare a clock source > * @clk: clock source > * > -- > 1.7.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html