On 27/11/2013 19:10, Mike Turquette wrote:
Quoting boris brezillon (2013-11-27 09:19:08)
Hi Jason,
On 27/11/2013 15:56, Jason Cooper wrote:
Boris,
Thanks for posting this series. Bear with me as I'm attempting to give
MikeT a hand.
Nice to hear.
Mike already took a look at this series, but I'm happy to get more
feedbacks.
Don't be afraid to tell me a question is stupid :-)
Your questions are far from stupid ;-).
On Wed, Nov 27, 2013 at 01:44:45PM +0100, Boris BREZILLON wrote:
This patch adds support for accuracy retrieval on fixed clocks.
It also adds a new dt property called 'clock-accuracy' to define the clock
accuracy.
This can be usefull for oscillator (RC, crystal, ...) definitions which are
always given an accuracy characteristic.
I think we need to be more explicit in the binding and the API,
especially when providing a method to recalculate the accuracy. I
presume this recalculated value would be relative against the root
clock?
Yes, indirectly.
Actually the clk accuracy depends on the whole clock chain, and is
calculated
either by comparing the real clk rate to the theorical clk rate
(accuracy = absolute_value((theorical_clk_rate - real_clk_rate)) /
theorical_clk_rate),
or by adding all the accuracies (expressed in ppm, ppb or ppt) of the
clk chain
(accuracy = current_clk_accuracy + parent_clk_accuracy).
Say you have a root clk with a +-10000 ppb accuracy, then a pll multiplying
this root clk by 40 and introducing a possible drift of +- 1000 ppb and
eventually a system clk based on this pll with a perfect div by 2 prescaler
(accuracy = 0 ppb).
If I understand correctly how accuracy propagates accross the clk tree,
you'll end up with a system clk with a +- 11000 ppb accuracy.
e.g.:
root clk = 12MHz +- 10000 ppb => 12 MHz * (1 - (10000 / 10^9)) < root
clk < 12 MHz * (1 + (10000 / 10^9))
=> 11,99988 MHz <
root clk < 12,00012 MHz
pll clk = ((root clk) * 40) +- 1000 ppb => (11,99988 MHz * 40) * (1 -
(1000 / 10^9)) < pll clk < (12,00012 MHz * 40) * (1 + (1000 / 10^9))
=>
479,994720005 MHz < pll clk < 480,005280005 MHz
system clk = ((pll clk) / 2) +- XXX ppb => 479,994720005 MHz / 2 <
system clk < 480,005280005 MHz / 2
=>
239,997360002 MHz < system clk < 240,002640002 MHz
=> system
clk accuracy = 0,002640002 / 240 = 11000 ppb
Please tell me if my assumptions are false.
There really needs to be two attributes here: the rated accuracy from
the manufacturer, and the calculated accuracy wrt another clock in the
system. We only need a binding for the manufacturer rating since the
calculated accuracy is determined at runtime.
Actually when I proposed this new functionnality I only had the theorical
(or manufacturer rated) accuracy in mind.
But providing an estimated accuracy (based on another clk) sounds
interresting if your reference clk is an extremly accurate one.
Is there a need to model clock accuracy across the clock chain?
I'm OK
modeling it in DT, and the code to do it in the clk framework isn't very
much ... but I also wonder if we're just adding complexity for no
reason.
AFAIK the most important node in the clock chain (regarding accuracy)
is the root node.
But some nodes (like PLLs) might introduce more innacuracy.
This series propose a simple way (or at least tries to keep it simple
:-)) to
express accuracy over the whole clk chain by means of the recalc_accuracy.
I'm not sure keeping the accuracy calculation (or retrieval) in the root
clk node
only will simplify the calculation (or retrieval) of a leaf clk node
accuracy (you'd
still have to walk over the clock chain to get the root clk accuracy).
My primary goal with this series is to provide a simple way (for a clock
user) to
choose the most accurate clock among several available clocks.
This is a real need on AT91 platforms which provides internal RC oscillators
with a really poor accuracy (+- 5% <=> +- 50000 ppm).
I would also prefer to see an unknown accuracy be -1.
I decided to keep 0 as a default value for unimplemented recalc_accuracy
(or unspecified fixed accuracy) to keep existing implementation coherent.
0 means the clk is perfect, and I thought it would be easier to handle a
perfect clk (even if this is not really the case) than handling an error
case.
Another aspect is the propagation of the clk accuracy accross the clk tree.
Returning -1 in the middle of the clk chain will drop the previous clk
accuracy
calculation.
Anyway, I can change this if you think this is more appropriate.
What about the absence of the property?
Instead of requiring a -1 value
can we simply detect that the property does not exist? This is nicer for
backwards compatibility with existing DTS.
I already handle the absence of the clock-accuracy property.
In this case the clock is considered perfect (accuracy = 0 ppb).
Mike, do you want me to return an error in the recalc_accuracy callback
to notifiy the absence of the clock-accuracy property ?
Regards,
Mike
There are already
clocks on the market (PPS reference clocks) with accuracies of
0.1ppb/day [1]. Obviously, these aren't system clocks. So the limit on
accuracy may be a non-issue. However, it may be worth changing the
binding property to express the units.
Wow, 0.1 ppb, this is impressive :-).
This needs more than changing the dt bindings: I currently store the
accuracy value in an unsigned long field, and expressing this in ppt
(parts per trillion) may implies storing this in an u64 field (or store a
unit field).
Signed-off-by: Boris BREZILLON <b.brezillon@xxxxxxxxxxx>
---
.../devicetree/bindings/clock/fixed-clock.txt | 3 ++
drivers/clk/clk-fixed-rate.c | 43 +++++++++++++++++---
include/linux/clk-provider.h | 4 ++
3 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.txt b/Documentation/devicetree/bindings/clock/fixed-clock.txt
index 0b1fe78..48ea0ad 100644
--- a/Documentation/devicetree/bindings/clock/fixed-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-clock.txt
@@ -10,6 +10,8 @@ Required properties:
- clock-frequency : frequency of clock in Hz. Should be a single cell.
Optional properties:
+- clock-accuracy : accuracy of clock in ppb (parts per billion).
+ Should be a single cell.
I would prefer to call this property 'clock-rated-ppb'.
Depending on what we choose to do with the accuracy field, this might be
an option.
- gpios : From common gpio binding; gpio connection to clock enable pin.
- clock-output-names : From common clock binding.
@@ -18,4 +20,5 @@ Example:
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <1000000000>;
+ clock-accuracy = <100>;
};
thx,
Jason.
[1] http://www.vectron.com/products/modules/md-010.htm
Thanks for your review, and don't hesitate to ask more questions, or to
point out
incoherencies in my approach (I'm not an expert in clk and clk accuracy
calculation,
and I might be wrong ;-)).
Best Regards,
Boris
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
