Messages in this thread | | | Subject | Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API | From | Michael Schmitz <> | Date | Mon, 19 Nov 2018 21:35:27 +1300 |
| |
Hi Finn,
this fixes the ssh timeout issues I reported for the earlier versions. Can't see a large degradation of resolution either.
The race in the previous versions may have been exacerbated in part by an incorrect assumption about the likelihood of counter wrap-around in the old atari_gettimeoffset() implementation. The comment in the code states that the likelihood is just 2%, so skips the interrupt pending bit check if the counter has run down more than 2%. It does not follow that the counter never runs down further than that threshold though. I have found the distribution of counter values observed with interrupt pending to be quite long-tailed, with a 50% threshold just beginning to catch the majority of wrap-arounds.
Your solution to stop the clock instead of allowing a jump backwards is much safer in this context.
Am 19.11.2018 um 14:10 schrieb Finn Thain: > Add a platform clocksource by adapting the existing arch_gettimeoffset > implementation. > > Normally the MFP timer C interrupt flag would be used to check for > timer counter wrap-around. Unfortunately, that flag gets cleared by the > MFP itself (due to automatic EOI mode). This means that > mfp_timer_c_handler() and atari_read_clk() must race when accounting > for counter wrap-around. > > That problem is avoided here by effectively stopping the clock when it > might otherwise jump backwards. This harms clock accuracy; the result > is not much better than the jiffies clocksource. Also, clock error is > no longer uniformly distributed. > > Signed-off-by: Finn Thain <fthain@telegraphics.com.au> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com> > --- > TODO: find a spare counter for the clocksource, rather than hanging > it off the HZ timer. > > It would be simpler to adopt the 'jiffies' clocksource here > (c.f. patch for the hp300 platform in this series). > > Changed since v1: > - Moved clk_total access to within the irq lock. > - Renamed mfp_timer_handler and mfptimer_handler. > - Avoid accessing the timer interrupt flag in atari_read_clk(). To > get monotonicity, keep track of the previous timer counter value. > --- > arch/m68k/atari/time.c | 48 +++++++++++++++++++++++++++--------------- > 1 file changed, 31 insertions(+), 17 deletions(-) > > diff --git a/arch/m68k/atari/time.c b/arch/m68k/atari/time.c > index fafa20f75ab9..914832e55ec5 100644 > --- a/arch/m68k/atari/time.c > +++ b/arch/m68k/atari/time.c > @@ -16,6 +16,7 @@ > #include <linux/init.h> > #include <linux/rtc.h> > #include <linux/bcd.h> > +#include <linux/clocksource.h> > #include <linux/delay.h> > #include <linux/export.h> > > @@ -24,12 +25,27 @@ > DEFINE_SPINLOCK(rtc_lock); > EXPORT_SYMBOL_GPL(rtc_lock); > > +static u64 atari_read_clk(struct clocksource *cs); > + > +static struct clocksource atari_clk = { > + .name = "mfp", > + .rating = 100, > + .read = atari_read_clk, > + .mask = CLOCKSOURCE_MASK(32), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS, > +}; > + > +static u32 clk_total; > +static u32 last_timer_count; > + > static irqreturn_t mfp_timer_c_handler(int irq, void *dev_id) > { > irq_handler_t timer_routine = dev_id; > unsigned long flags; > > local_irq_save(flags); > + last_timer_count = st_mfp.tim_dt_c; > + clk_total += INT_TICKS; > timer_routine(0, NULL); > local_irq_restore(flags); > > @@ -44,32 +60,30 @@ atari_sched_init(irq_handler_t timer_routine) > /* start timer C, div = 1:100 */ > st_mfp.tim_ct_cd = (st_mfp.tim_ct_cd & 15) | 0x60; > /* install interrupt service routine for MFP Timer C */ > - if (request_irq(IRQ_MFP_TIMC, mfp_timer_c_handler, 0, "timer", > + if (request_irq(IRQ_MFP_TIMC, mfp_timer_c_handler, IRQF_TIMER, "timer", > timer_routine)) > pr_err("Couldn't register timer interrupt\n"); > + > + clocksource_register_hz(&atari_clk, INT_CLK); > } > > /* ++andreas: gettimeoffset fixed to check for pending interrupt */ > > -#define TICK_SIZE 10000 > - > -/* This is always executed with interrupts disabled. */ > -u32 atari_gettimeoffset(void) > +static u64 atari_read_clk(struct clocksource *cs) > { > - u32 ticks, offset = 0; > - > - /* read MFP timer C current value */ > - ticks = st_mfp.tim_dt_c; > - /* The probability of underflow is less than 2% */ > - if (ticks > INT_TICKS - INT_TICKS / 50) > - /* Check for pending timer interrupt */ > - if (st_mfp.int_pn_b & (1 << 5)) > - offset = TICK_SIZE; > + unsigned long flags; > + u32 ticks; > > - ticks = INT_TICKS - ticks; > - ticks = ticks * 10000L / INT_TICKS; > + local_irq_save(flags); > + ticks = st_mfp.tim_dt_c; > + if (ticks > last_timer_count) /* timer wrapped since last interrupt */ > + ticks = last_timer_count; > + last_timer_count = ticks; > + ticks = INT_TICKS - ticks; > + ticks += clk_total; > + local_irq_restore(flags); > > - return (ticks + offset) * 1000; > + return ticks; > } > > >
| |