Re: [RFC PATCH RESEND 1/2] clk: add clk accuracy retrieval support
From: boris brezillon
Date: Sun Nov 17 2013 - 10:34:14 EST
Hello Mike,
On 16/11/2013 01:50, Mike Turquette wrote:
Quoting boris brezillon (2013-11-08 00:54:45)
Hello Mike,
On 08/11/2013 01:51, Mike Turquette wrote:
Quoting Boris BREZILLON (2013-10-13 10:17:10)
+/**
+ * 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.
I'm not sure I get your point.
I used exactly the same model as for clk rate retrieval.
Not exactly the same model. For rates we support public clk_recalc_rate
and clk_get_rate functions.
I didn't find any clk_recalc_rate function in the CCF.
Could you point me out where it is defined ?
Actually there's one flag (CLK_GET_ACCURACY_NOCACHE) which is
checked to decide wether the accuracy should be recalculated each
time the get_accuracy is called or not.
This means most of the time the clk_get_accuracy will return the cached
value unless the clk provider explicitely ask for accuracy recalculation
(e.g. a clk with dynamic accuracy according to temperature range ?).
Are you suggesting to expose 2 functions to clk users (clk_get_accuracy
and clk_recalc_accuracy) ?
Or is clk_recalc_accuracy an internal/private function used by the CCF ?
I was suggesting that in my previous email. However I've thought on it a
bit and I'm not sure there is any value to having a public
clk_recalc_accuracy right now.
The only reason to do so would be if the accuracy can change in such a
way that the clock framework is never aware of it. This would mean that
somehow the accuracy changed "behind our back".
One hypothetical example is a PLL which Linux knows about, but perhaps
is controlled by some other co-processor (not Linux). In this case the
co-processor may reprogram/relock the PLL in a way that affects it's
accuracy, and a Linux driver that knows this may want to update the CCF
book-keeping with a call to clk_recalc_accuracy.
That's all hypothetical though and it can be added in later. Looking
over your change again I think that it is sufficient for now.
This looks reasonable.
I'll propose a new patch to provide a public clk_recalc_rate function
after this
series is merged.
Can you respin this patch with fixes for the two nitpicks I outlined in
my previous mail and rebase it onto -rc1 when it drops? That should be
any day now.
I've rebased this series (with the proposed fixes) onto linux-next.
I guess there won't be a lot of changes between the current tag (
next-20131115
<https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tag/?id=next-20131115>)
and -rc1, but I'll wait till -rc1 is released ;-).
Thanks for your review.
Best Regards,
Boris
Thanks!
Mike
+
+ 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.
This is the same for clk_get_accuracies (it returns the cached
accuracy unless CLK_GET_ACCURACY_NOCACHE is defined).
And changing parent of a clk will indirectly change the clk
accuracy (clk accuracies are cumulative).
__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.
Ditto. :)
Please tell me if I misunderstood your requests.
Best Regards,
Boris
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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/