lkml.org 
[lkml]   [2018]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API
    From
    Date
    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;
    > }
    >
    >
    >

    \
     
     \ /
      Last update: 2018-11-19 09:37    [W:7.320 / U:0.248 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site