| Message ID | 20211109095013.27829-4-martin.kaistra@linutronix.de (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Add PTP support for BCM53128 switch | expand |
Martin, On Tue, Nov 09 2021 at 10:50, Martin Kaistra wrote: > * see CYCLECOUNTER_MASK() helper macro > * @mult: cycle to nanosecond multiplier > * @shift: cycle to nanosecond divisor (power of two) > + * @overflow_point: non-power of two overflow point (optional), > + * smaller than mask > */ > struct cyclecounter { > u64 (*read)(const struct cyclecounter *cc); > u64 mask; > u32 mult; > u32 shift; > + u64 overflow_point; > }; > > /** > diff --git a/kernel/time/timecounter.c b/kernel/time/timecounter.c > index e6285288d765..afd2910a9724 100644 > --- a/kernel/time/timecounter.c > +++ b/kernel/time/timecounter.c > @@ -39,6 +39,9 @@ static u64 timecounter_read_delta(struct timecounter *tc) > /* calculate the delta since the last timecounter_read_delta(): */ > cycle_delta = (cycle_now - tc->cycle_last) & tc->cc->mask; > > + if (tc->cc->overflow_point && (cycle_now - tc->cycle_last) > tc->cc->mask) > + cycle_delta -= tc->cc->mask - tc->cc->overflow_point; TBH, this took me more than one twisted braincell to grok. With support for clocks which do not wrap at power of 2 boundaries we already lose the unconditional fast path no matter what. So what's the point of having two conditions and doing this convoluted math here? In timecounter_init(): tc->ovfl = cc->ovfl ? cc->ovfl : cc->mask + 1; which makes it a common path in timecounter_read_delta(): cycle_delta = cycle_now - tc->cycle_last; if ((s64)cycle_delta) < 0) cycle_delta += tc->ovfl; which produces way better binary code. The conditional does not really matter for the timecounter use cases as that calculation is noise compared to the actual cc->read() access. Aside of that the same problem exists in timecounter_cyc2time()... After that we probably should do a treewide sweep to get rid of cc->mask to avoid confusion and subtle to understand errors when some code uses cc->mask instead of cc->ovfl. Thanks, tglx
diff --git a/include/linux/timecounter.h b/include/linux/timecounter.h index c6540ceea143..c71196a742b3 100644 --- a/include/linux/timecounter.h +++ b/include/linux/timecounter.h @@ -26,12 +26,15 @@ * see CYCLECOUNTER_MASK() helper macro * @mult: cycle to nanosecond multiplier * @shift: cycle to nanosecond divisor (power of two) + * @overflow_point: non-power of two overflow point (optional), + * smaller than mask */ struct cyclecounter { u64 (*read)(const struct cyclecounter *cc); u64 mask; u32 mult; u32 shift; + u64 overflow_point; }; /** diff --git a/kernel/time/timecounter.c b/kernel/time/timecounter.c index e6285288d765..afd2910a9724 100644 --- a/kernel/time/timecounter.c +++ b/kernel/time/timecounter.c @@ -39,6 +39,9 @@ static u64 timecounter_read_delta(struct timecounter *tc) /* calculate the delta since the last timecounter_read_delta(): */ cycle_delta = (cycle_now - tc->cycle_last) & tc->cc->mask; + if (tc->cc->overflow_point && (cycle_now - tc->cycle_last) > tc->cc->mask) + cycle_delta -= tc->cc->mask - tc->cc->overflow_point; + /* convert to nanoseconds: */ ns_offset = cyclecounter_cyc2ns(tc->cc, cycle_delta, tc->mask, &tc->frac);
Some hardware counters which are used as clocks have an overflow point which is not a power of two. In order to be able to use the cycle counter infrastructure with such hardware, add support for more generic overflow logic. Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de> --- include/linux/timecounter.h | 3 +++ kernel/time/timecounter.c | 3 +++ 2 files changed, 6 insertions(+)