| Message ID | 20200521005321.12129-8-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support | expand |
Hi Serge, On Thu, May 21, 2020 at 2:54 AM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > Currently clocksource framework doesn't support the clocks with variable > frequency. Since MIPS GIC timer ticks rate might be unstable on some > platforms, we must make sure that it justifies the clocksource > requirements. MIPS GIC timer is incremented with the CPU cluster reference > clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate > changes synchronously. Due to this the clocksource subsystem can't rely on > the timer to measure system clocks anymore. This commit marks the MIPS GIC > based clocksource as unstable if reference clock (normally it's a CPU > reference clocks) rate changes. The clocksource will execute a watchdog > thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a > new stable one. > > Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS > GIC clocksource since normally the timer is stable. The only reason why > it gets unstable is due to the ref clock rate change, which event we > detect here in the driver by means of the clocks event notifier. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Thanks for your patch! > --- a/drivers/clocksource/mips-gic-timer.c > +++ b/drivers/clocksource/mips-gic-timer.c > @@ -24,6 +24,9 @@ > static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device); > static int gic_timer_irq; > static unsigned int gic_frequency; > +static bool __read_mostly gic_clock_unstable; > + > +static void git_clocksource_unstable(char *reason); gic_clocksource_unstable? (everywhere) Gr{oetje,eeting}s, Geert
On Thu, May 21, 2020 at 11:09:50AM +0200, Geert Uytterhoeven wrote: > Hi Serge, > > On Thu, May 21, 2020 at 2:54 AM Serge Semin > <Sergey.Semin@baikalelectronics.ru> wrote: > > Currently clocksource framework doesn't support the clocks with variable > > frequency. Since MIPS GIC timer ticks rate might be unstable on some > > platforms, we must make sure that it justifies the clocksource > > requirements. MIPS GIC timer is incremented with the CPU cluster reference > > clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate > > changes synchronously. Due to this the clocksource subsystem can't rely on > > the timer to measure system clocks anymore. This commit marks the MIPS GIC > > based clocksource as unstable if reference clock (normally it's a CPU > > reference clocks) rate changes. The clocksource will execute a watchdog > > thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a > > new stable one. > > > > Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS > > GIC clocksource since normally the timer is stable. The only reason why > > it gets unstable is due to the ref clock rate change, which event we > > detect here in the driver by means of the clocks event notifier. > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > Thanks for your patch! > > > --- a/drivers/clocksource/mips-gic-timer.c > > +++ b/drivers/clocksource/mips-gic-timer.c > > @@ -24,6 +24,9 @@ > > static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device); > > static int gic_timer_irq; > > static unsigned int gic_frequency; > > +static bool __read_mostly gic_clock_unstable; > > + > > +static void git_clocksource_unstable(char *reason); > > gic_clocksource_unstable? (everywhere) This is the most used word lately. So my hands write git everywhere by itself.) Thanks for noticing this. -Sergey > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index f2142e6bbea3..37a745f3ca91 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -572,6 +572,7 @@ config CLKSRC_VERSATILE config CLKSRC_MIPS_GIC bool depends on MIPS_GIC + select CLOCKSOURCE_WATCHDOG select TIMER_OF config CLKSRC_TANGO_XTAL diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c index ef12c12c2432..d78ad7b60cdc 100644 --- a/drivers/clocksource/mips-gic-timer.c +++ b/drivers/clocksource/mips-gic-timer.c @@ -24,6 +24,9 @@ static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device); static int gic_timer_irq; static unsigned int gic_frequency; +static bool __read_mostly gic_clock_unstable; + +static void git_clocksource_unstable(char *reason); static u64 notrace gic_read_count_2x32(void) { @@ -125,8 +128,10 @@ static int gic_clk_notifier(struct notifier_block *nb, unsigned long action, { struct clk_notifier_data *cnd = data; - if (action == POST_RATE_CHANGE) + if (action == POST_RATE_CHANGE) { + git_clocksource_unstable("ref clock rate change"); on_each_cpu(gic_update_frequency, (void *)cnd->new_rate, 1); + } return NOTIFY_OK; } @@ -172,6 +177,18 @@ static struct clocksource gic_clocksource = { .vdso_clock_mode = VDSO_CLOCKMODE_GIC, }; +static void git_clocksource_unstable(char *reason) +{ + if (gic_clock_unstable) + return; + + gic_clock_unstable = true; + + pr_info("GIC timer is unstable due to %s\n", reason); + + clocksource_mark_unstable(&gic_clocksource); +} + static int __init __gic_clocksource_init(void) { unsigned int count_width;
Currently clocksource framework doesn't support the clocks with variable frequency. Since MIPS GIC timer ticks rate might be unstable on some platforms, we must make sure that it justifies the clocksource requirements. MIPS GIC timer is incremented with the CPU cluster reference clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate changes synchronously. Due to this the clocksource subsystem can't rely on the timer to measure system clocks anymore. This commit marks the MIPS GIC based clocksource as unstable if reference clock (normally it's a CPU reference clocks) rate changes. The clocksource will execute a watchdog thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a new stable one. Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS GIC clocksource since normally the timer is stable. The only reason why it gets unstable is due to the ref clock rate change, which event we detect here in the driver by means of the clocks event notifier. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Paul Burton <paulburton@kernel.org> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Rob Herring <robh+dt@kernel.org> Cc: linux-mips@vger.kernel.org Cc: linux-rtc@vger.kernel.org Cc: devicetree@vger.kernel.org --- Changelog v3: - Mark clocksource as unstable instead of lowering its rating. --- drivers/clocksource/Kconfig | 1 + drivers/clocksource/mips-gic-timer.c | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)