[RFC PATCH v2 00/14] m68k: Drop arch_gettimeoffset and adopt clocksource API

[RFC PATCH v2 00/14] m68k: Drop arch_gettimeoffset and adopt clocksource API

[Finn Thain]

This series removes "select ARCH_USES_GETTIMEOFFSET" from arch/m68k
and converts users of arch_gettimeoffset to the clocksource API.
Various bugs are fixed along the way.

Those platforms which do not actually implement arch_gettimeoffset
(apollo, q40, sun3, sun3x) use the "jiffies" clocksource by default.

The atari and hp300 platforms have an arch_gettimeoffset() implementation
which can't readily be converted to a clocksource. Getting a workable
clocksource on these platforms will require the insight of a platform
expert.

The difficulty with these patches is the use of the timer interrupt to
update the counter for the clock source. The timer interrupt handler races
with clocksource read method, and both of those functions race with the
timer hardware.

Hence, more testing and code review would be appreciated.

Changed since v1:

 - Dropped patches 1/13 and 2/13. These were a failed attempt to fix
5cfc8ee0bb51 and 4ad4c76b7afb. By adopting the clocksource API we can fix
this issue in mainline. By backporting this series we can fix it for -stable
(for m68k at least).

 - Dropped patch "m68k: hp300: Convert to clocksource API" and added
patch "m68k: hp300: Remove hp300_gettimeoffset".

 - Added a new patch to address an old m68k bug pointed out by Thomas
Gleixner. The bug can arise when a timer interrupt handler gets interrupted.

 - Added new patches to address old mvme16x and mvme147 bugs pointed out
by Thomas Gleixner. The bug could cause the clock to jump backwards.

 - Various other changes summarized in the relevant patches.


Finn Thain (14):
  m68k: Call timer_interrupt() with interrupts disabled
  m68k: mac: Fix VIA timer counter accesses
  m68k: mac: Clean up unused timer definitions
  m68k: apollo, q40, sun3, sun3x: Remove arch_gettimeoffset
    implementations
  m68k: Drop ARCH_USES_GETTIMEOFFSET
  m68k: amiga: Convert to clocksource API
  m68k: atari: Convert to clocksource API
  m68k: bvme6000: Convert to clocksource API
  m68k: hp300: Remove hp300_gettimeoffset()
  m68k: mac: Convert to clocksource API
  m68k: mvme147: Convert to clocksource API
  m68k: mvme147: Handle timer counter overflow
  m68k: mvme16x: Convert to clocksource API
  m68k: mvme16x: Handle timer counter overflow

 arch/m68k/Kconfig                 |   1 -
 arch/m68k/amiga/cia.c             |  10 ++
 arch/m68k/amiga/config.c          |  46 ++++++---
 arch/m68k/apollo/config.c         |   7 --
 arch/m68k/atari/ataints.c         |   4 +-
 arch/m68k/atari/config.c          |   2 -
 arch/m68k/atari/time.c            |  65 +++++++++----
 arch/m68k/bvme6000/config.c       |  70 +++++++++-----
 arch/m68k/hp300/config.c          |   1 -
 arch/m68k/hp300/time.c            |  29 ++----
 arch/m68k/hp300/time.h            |   1 -
 arch/m68k/include/asm/macints.h   |   3 -
 arch/m68k/include/asm/mvme147hw.h |   2 +-
 arch/m68k/mac/config.c            |   3 -
 arch/m68k/mac/via.c               | 150 ++++++++++++++++++++----------
 arch/m68k/mvme147/config.c        |  73 ++++++++++-----
 arch/m68k/mvme16x/config.c        |  97 +++++++++++++------
 arch/m68k/q40/config.c            |   9 --
 arch/m68k/q40/q40ints.c           |   7 +-
 arch/m68k/sun3/config.c           |   2 -
 arch/m68k/sun3/intersil.c         |   7 --
 arch/m68k/sun3/sun3ints.c         |   3 +
 arch/m68k/sun3x/config.c          |   1 -
 arch/m68k/sun3x/time.c            |  21 ++---
 arch/m68k/sun3x/time.h            |   1 -
 25 files changed, 387 insertions(+), 228 deletions(-)

-- 
2.18.1

[RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

hp300_gettimeoffset() never checks the timer interrupt flag and will
fail to notice when the timer counter gets reloaded. That means the
clock could jump backwards.

Remove this code and leave this platform on the 'jiffies' clocksource.
Note that this amounts to a regression in clock precision. However,
adopting the 'jiffies' clocksource does resolve the monotonicity issue.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
hp300_gettimeoffset() cannot be used in a clocksource conversion
unless it can be made monotonic. I can't fix this without knowing the
details of the timer implementation, such as the relationship between
the timer count and the interrupt flag.
---
 arch/m68k/hp300/time.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/arch/m68k/hp300/time.c b/arch/m68k/hp300/time.c
index d30b03ea93a2..37cccdb46def 100644
--- a/arch/m68k/hp300/time.c
+++ b/arch/m68k/hp300/time.c
@@ -31,9 +31,6 @@
 #define	CLKMSB2		0x9
 #define	CLKMSB3		0xD
 
-/* This is for machines which generate the exact clock. */
-#define USECS_PER_JIFFY (1000000/HZ)
-
 #define INTVAL ((10000 / 4) - 1)
 
 static irqreturn_t hp300_tick(int irq, void *dev_id)
@@ -53,22 +50,6 @@ static irqreturn_t hp300_tick(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-u32 hp300_gettimeoffset(void)
-{
-  /* Read current timer 1 value */
-  unsigned char lsb, msb1, msb2;
-  unsigned short ticks;
-
-  msb1 = in_8(CLOCKBASE + 5);
-  lsb = in_8(CLOCKBASE + 7);
-  msb2 = in_8(CLOCKBASE + 5);
-  if (msb1 != msb2)
-    /* A carry happened while we were reading.  Read it again */
-    lsb = in_8(CLOCKBASE + 7);
-  ticks = INTVAL - ((msb2 << 8) | lsb);
-  return ((USECS_PER_JIFFY * ticks) / INTVAL) * 1000;
-}
-
 void __init hp300_sched_init(irq_handler_t vector)
 {
   out_8(CLOCKBASE + CLKCR2, 0x1);		/* select CR1 */
-- 
2.18.1

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Kars de Jong]

Op ma 19 nov. 2018 om 02:10 schreef Finn Thain <fthain@telegraphics.com.au>:
>
> hp300_gettimeoffset() never checks the timer interrupt flag and will
> fail to notice when the timer counter gets reloaded. That means the
> clock could jump backwards.
>
> Remove this code and leave this platform on the 'jiffies' clocksource.
> Note that this amounts to a regression in clock precision. However,
> adopting the 'jiffies' clocksource does resolve the monotonicity issue.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
> hp300_gettimeoffset() cannot be used in a clocksource conversion
> unless it can be made monotonic. I can't fix this without knowing the
> details of the timer implementation, such as the relationship between
> the timer count and the interrupt flag.

I don't really like this regression...

According to NetBSD sources, there are 3 timers in the chip
(originally an MC6840 PTM). Timer 1 is used as the system timer, timer
3 runs at the same rate and is unused on Linux (on NetBSD it is used
as the statistics/profiling timer), and timer 3 is connected to timer
2 so you can make a 32-bit timer out of the two timers together (also
unused on Linux).

Timers 1 counts down at 25 MHz. The interrupt flag is set when the
counter reaches 0 after which it is automatically reloaded and starts
counting down again.

> ---
>  arch/m68k/hp300/time.c | 19 -------------------
>  1 file changed, 19 deletions(-)

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Tue, 20 Nov 2018, Kars de Jong wrote:

> Op ma 19 nov. 2018 om 02:10 schreef Finn Thain <fthain@telegraphics.com.au>:
> >
> > hp300_gettimeoffset() never checks the timer interrupt flag and will
> > fail to notice when the timer counter gets reloaded. That means the
> > clock could jump backwards.
> >
> > Remove this code and leave this platform on the 'jiffies' clocksource.
> > Note that this amounts to a regression in clock precision. However,
> > adopting the 'jiffies' clocksource does resolve the monotonicity issue.
> >
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > ---
> > hp300_gettimeoffset() cannot be used in a clocksource conversion
> > unless it can be made monotonic. I can't fix this without knowing the
> > details of the timer implementation, such as the relationship between
> > the timer count and the interrupt flag.
> 
> I don't really like this regression...
> 

Me neither...

I'll see if I can write a conversion patch based on the information you've 
provided. Can you test it?

> According to NetBSD sources, there are 3 timers in the chip (originally 
> an MC6840 PTM).

Thanks for the tip. I will examine the datasheet for the 6840.

I'll also take another look at the NetBSD code.

> Timer 1 is used as the system timer, timer 3 runs at the same rate and 
> is unused on Linux (on NetBSD it is used as the statistics/profiling 
> timer), and timer 3 is connected to timer 2 so you can make a 32-bit 
> timer out of the two timers together (also unused on Linux).
> 
> Timers 1 counts down at 25 MHz.

You mean, 250 kHz, right? The code in mainline programs the timer for 2500 
cycles, hoping to get 10 ms. That is, 250 cycles per ms.

> The interrupt flag is set when the counter reaches 0 after which it is 
> automatically reloaded and starts counting down again.
> 

Thanks.

On atari, the 68901 counts down to 0x01 and raises an interrupt. On mac, 
the 6522 counts down to 0xFFFF then raises an interrupt. No idea about 
amiga (Geert?) -- this has to be handled correctly to get a monotonic 
clocksource. I'll fix this in v3 (where the information is available).

-- 

> > ---
> >  arch/m68k/hp300/time.c | 19 -------------------
> >  1 file changed, 19 deletions(-)
> 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Geert Uytterhoeven]

Hi Finn,

On Wed, Nov 21, 2018 at 12:13 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On atari, the 68901 counts down to 0x01 and raises an interrupt. On mac,
> the 6522 counts down to 0xFFFF then raises an interrupt. No idea about
> amiga (Geert?) -- this has to be handled correctly to get a monotonic
> clocksource. I'll fix this in v3 (where the information is available).

The docs state that the CIA generates on interrupt on underflow, so I
guess that's the same behavior as the 6522 VIA.

Unfortunately the 24-bit ("TOD") counters in the two CIAs run from HSYNC
resp. VSYNC, which depends on the video mode, and thus can't be used
as a monotonic clock source.

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

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:

> Hi Finn,
> 
> On Wed, Nov 21, 2018 at 12:13 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> > On atari, the 68901 counts down to 0x01 and raises an interrupt. On 
> > mac, the 6522 counts down to 0xFFFF then raises an interrupt. No idea 
> > about amiga (Geert?) -- this has to be handled correctly to get a 
> > monotonic clocksource. I'll fix this in v3 (where the information is 
> > available).
> 
> The docs state that the CIA generates on interrupt on underflow, so I 
> guess that's the same behavior as the 6522 VIA.
> 

Difficult to say. The sequence varies from one implementation to another. 
Let's ignore the MSB and LSB and pretend it's one register:

MC68901: N, N-1, N-2, ..., 2, 1, N, N-1, N-2, ...
MC6840:  N, N-1, N-2, ..., 2, 1, 0, N, N-1, N-2, ...
SY6522:  N, N-1, N-2, ..., 2, 1, 0, 0xFFFF, N, N-1, N-2, ...

Now the question is, when the timer asserts its interrupt, and the count 
register is fetched immediately, what value does it have?

For the MC68901, you get 1. For the SY6522, you get 0xFFFF. For MC6840, as 
far as I can tell, you'd get 0.

I'll add some code to my github repo to find out what happens with CIA.

> Unfortunately the 24-bit ("TOD") counters in the two CIAs run from HSYNC 
> resp. VSYNC, which depends on the video mode, and thus can't be used as 
> a monotonic clock source.
> 

Is that because of video mode changes? Could the clocksource be 
unregistered before the mode change and then re-registered at a different 
frequency afterwards?

-- 

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Geert Uytterhoeven]

Hi Finn,

On Wed, Nov 21, 2018 at 9:41 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:
> > On Wed, Nov 21, 2018 at 12:13 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> > > On atari, the 68901 counts down to 0x01 and raises an interrupt. On
> > > mac, the 6522 counts down to 0xFFFF then raises an interrupt. No idea
> > > about amiga (Geert?) -- this has to be handled correctly to get a
> > > monotonic clocksource. I'll fix this in v3 (where the information is
> > > available).
> >
> > The docs state that the CIA generates on interrupt on underflow, so I
> > guess that's the same behavior as the 6522 VIA.
>
> Difficult to say. The sequence varies from one implementation to another.
> Let's ignore the MSB and LSB and pretend it's one register:
>
> MC68901: N, N-1, N-2, ..., 2, 1, N, N-1, N-2, ...
> MC6840:  N, N-1, N-2, ..., 2, 1, 0, N, N-1, N-2, ...
> SY6522:  N, N-1, N-2, ..., 2, 1, 0, 0xFFFF, N, N-1, N-2, ...
>
> Now the question is, when the timer asserts its interrupt, and the count
> register is fetched immediately, what value does it have?
>
> For the MC68901, you get 1. For the SY6522, you get 0xFFFF. For MC6840, as
> far as I can tell, you'd get 0.

I assume 0xFFFF. The 8520 CIA is almost identical to the 6526 CIA, as used in
the C64, which is a direct descendant of the 6522 VIA.

> > Unfortunately the 24-bit ("TOD") counters in the two CIAs run from HSYNC
> > resp. VSYNC, which depends on the video mode, and thus can't be used as
> > a monotonic clock source.
>
> Is that because of video mode changes? Could the clocksource be
> unregistered before the mode change and then re-registered at a different
> frequency afterwards?

On older Amigas (most plain 68000, unless expanded), video modes are fixed.
Some have a jumper to select the power supply tick instead of the VSYNC
signal, which is more accurate, but still runs at a low 50 or 60 Hz.
On anything more modern (A3000 and up), video modes are programmable.
In addition, blanking the screen according to VESA will disable sync signals,
thus stopping the clock, I assume.

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

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:

> The 8520 CIA is almost identical to the 6526 CIA, as used in the C64...
> 

The 6526 CIA datasheet says, "In continuous mode, the timer will count 
from the latched value to zero, generate and interrupt, reload the latched 
value and repeat the procedure continuously."

This suggests that either 0 or N (the latched value) would result from a 
read from the counter immediately following an interrupt. Who can say 
which? Just have to try it. The answer should allow us to avoid the risk 
of a clocksource that jumps forwards and backwards.

-- 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Geert Uytterhoeven]

Hi Finn,

On Wed, Nov 21, 2018 at 10:47 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:
> > The 8520 CIA is almost identical to the 6526 CIA, as used in the C64...
>
> The 6526 CIA datasheet says, "In continuous mode, the timer will count
> from the latched value to zero, generate and interrupt, reload the latched
> value and repeat the procedure continuously."

I stand corrected...

> This suggests that either 0 or N (the latched value) would result from a
> read from the counter immediately following an interrupt. Who can say
> which? Just have to try it. The answer should allow us to avoid the risk
> of a clocksource that jumps forwards and backwards.

The code in amiga_gettimeoffset() does:

        ticks = hi << 8 | lo;

        if (ticks > jiffy_ticks / 2)
                /* check for pending interrupt */
                if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
                        offset = 10000;

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

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:

> > This suggests that either 0 or N (the latched value) would result from 
> > a read from the counter immediately following an interrupt. Who can 
> > say which? Just have to try it. The answer should allow us to avoid 
> > the risk of a clocksource that jumps forwards and backwards.
> 
> The code in amiga_gettimeoffset() does:
> 
>         ticks = hi << 8 | lo;
> 
>         if (ticks > jiffy_ticks / 2)
>                 /* check for pending interrupt */
>                 if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
>                         offset = 10000;
> 

That _suggests_ that there's no interrupt when ticks == 0.

But look what happens next:

>         ticks = jiffy_ticks - ticks;
> 
>         ticks = (10000 * ticks) / jiffy_ticks;
> 
>         return (ticks + offset) * 1000;

If (hi << 8 | lo) == 0, and you set offset = 10000, then the return value 
would be maximal.

Let's immediately call this function again. This time (hi << 8 | lo) == N. 
Let's add the offset again. I'm afraid the clock just jumped backwards.

So the logic you quoted has a rationale which is unrelated to the 
question.

-- 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Wed, 21 Nov 2018, I wrote:

> On Wed, 21 Nov 2018, Geert Uytterhoeven wrote:
> 
> > > This suggests that either 0 or N (the latched value) would result 
> > > from a read from the counter immediately following an interrupt. Who 
> > > can say which? Just have to try it. The answer should allow us to 
> > > avoid the risk of a clocksource that jumps forwards and backwards.
> > 
> > The code in amiga_gettimeoffset() does:
> > 
> >         ticks = hi << 8 | lo;
> > 
> >         if (ticks > jiffy_ticks / 2)
> >                 /* check for pending interrupt */
> >                 if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
> >                         offset = 10000;
> > 
> 
> That _suggests_ that there's no interrupt when ticks == 0.
> 
> But look what happens next:
> 
> >         ticks = jiffy_ticks - ticks;
> > 
> >         ticks = (10000 * ticks) / jiffy_ticks;
> > 
> >         return (ticks + offset) * 1000;
> 
> If (hi << 8 | lo) == 0, and you set offset = 10000, then the return 
> value would be maximal.
> 
> Let's immediately call this function again. This time (hi << 8 | lo) == 
> N. Let's add the offset again. I'm afraid the clock just jumped 
> backwards.
> 
> So the logic you quoted has a rationale which is unrelated to the 
> question.
> 

I've learned that emulators like MAME [1] and VICE [2] have used a 
reverse-engineered description of the CIA called "A Software Model of the 
CIA6526" by Wolfgang Lorenz and an accompanying test suite [3].

That document says, "When the counter has reached zero, it is reloaded 
from the latch as soon as there is another clock waiting in the pipeline. 
In phase 2 mode, this is always the case. This explains why you are 
reading zeros in cascaded mode only (2-2-2-1-1-1-0-0-2) but not in phase 2 
mode (2-1-2)."

I think this is a good argument that a zero count will never be observed 
by reading the counter register. Hence, it seems that this conditional in 
the v3 patch,

	if (msb > 0)

is redundant and should be removed.

It could be reverted to,

	if (ticks > jiffy_ticks / 2)

which is intended to reduce the number of calls to cia_set_irq() but 
assumes low interrupt latency (below 5 ms).

Maybe the timer interrupt has a sufficiently high priority and latency is 
low? Maybe cia_set_irq() is really expensive?

I don't know the platform well enough so I'm inclined to revert. We can 
benchmark gettimeofday syscalls on elgar but is that hardware 
representative of other relevant models?

[1] 
https://github.com/mamedev/mame/commit/e2ed0490520f538c346c8bdaa9084bcbc43427cb

[2]
http://vice-emu.sourceforge.net/vice_9.html

[3]
https://www.commodore.ca/manuals/funet/cbm/documents/chipdata/cia6526.zip

-- 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Michael Schmitz]

Hi Finn,

Am 25.11.2018 um 14:15 schrieb Finn Thain:
> Maybe the timer interrupt has a sufficiently high priority and latency is
> low? Maybe cia_set_irq() is really expensive?
>
> I don't know the platform well enough so I'm inclined to revert. We can
> benchmark gettimeofday syscalls on elgar but is that hardware
> representative of other relevant models?

I suppose the CIA is on the main board, so running with the slower clock 
speed that you'd see with a vanilla Amiga without 060 accelerator board. 
Ought to be representative enough?

Adrian has a few other Amigas with different hardware base, so we might 
be able to get test coverage on more than one model.

Cheers,

	Michael


> [1]
> https://github.com/mamedev/mame/commit/e2ed0490520f538c346c8bdaa9084bcbc43427cb
>
> [2]
> http://vice-emu.sourceforge.net/vice_9.html
>
> [3]
> https://www.commodore.ca/manuals/funet/cbm/documents/chipdata/cia6526.zip
>

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Finn Thain]

On Sun, 25 Nov 2018, Michael Schmitz wrote:

> > We can benchmark gettimeofday syscalls on elgar but is that hardware 
> > representative of other relevant models?
> 
> I suppose the CIA is on the main board, so running with the slower clock 
> speed that you'd see with a vanilla Amiga without 060 accelerator board. 
> Ought to be representative enough?
> 

Not really. An accelerated CPU clock exaggerates the slowdown factor, 
given the fixed 0.7 MHz CIA clock.

The "(ticks > jiffy_ticks / 2)" conditional won't make anything worse even 
if interrupt latency is already too high, so I'm inclined towards the more 
prudent option. This patch series is already complicated enough.

In anycase, I'd prefer not to speculate about interrupt priorities or 
latencies when the list probably has people who know the answers.

-- 

Re: [RFC PATCH v2 09/14] m68k: hp300: Remove hp300_gettimeoffset()

[Kars de Jong]

Op wo 21 nov. 2018 om 00:13 schreef Finn Thain <fthain@telegraphics.com.au>:
>
> On Tue, 20 Nov 2018, Kars de Jong wrote:
>
> > Op ma 19 nov. 2018 om 02:10 schreef Finn Thain <fthain@telegraphics.com.au>:
> > >
> > > hp300_gettimeoffset() never checks the timer interrupt flag and will
> > > fail to notice when the timer counter gets reloaded. That means the
> > > clock could jump backwards.
> > >
> > > Remove this code and leave this platform on the 'jiffies' clocksource.
> > > Note that this amounts to a regression in clock precision. However,
> > > adopting the 'jiffies' clocksource does resolve the monotonicity issue.
> > >
> > > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > > ---
> > > hp300_gettimeoffset() cannot be used in a clocksource conversion
> > > unless it can be made monotonic. I can't fix this without knowing the
> > > details of the timer implementation, such as the relationship between
> > > the timer count and the interrupt flag.
> >
> > I don't really like this regression...
> >
>
> Me neither...
>
> I'll see if I can write a conversion patch based on the information you've
> provided. Can you test it?

I can try... It's been a while since I booted the machine to Linux
though (NFS support only).
MAME is also starting to support it, but not quite there yet :-)

> > According to NetBSD sources, there are 3 timers in the chip (originally
> > an MC6840 PTM).
>
> Thanks for the tip. I will examine the datasheet for the 6840.
>
> I'll also take another look at the NetBSD code.
>
> > Timer 1 is used as the system timer, timer 3 runs at the same rate and
> > is unused on Linux (on NetBSD it is used as the statistics/profiling
> > timer), and timer 3 is connected to timer 2 so you can make a 32-bit
> > timer out of the two timers together (also unused on Linux).
> >
> > Timers 1 counts down at 25 MHz.
>
> You mean, 250 kHz, right? The code in mainline programs the timer for 2500
> cycles, hoping to get 10 ms. That is, 250 cycles per ms.

Eh, yes, that makes a lot more sense.

> > The interrupt flag is set when the counter reaches 0 after which it is
> > automatically reloaded and starts counting down again.
> >
>
> Thanks.
>
> On atari, the 68901 counts down to 0x01 and raises an interrupt. On mac,
> the 6522 counts down to 0xFFFF then raises an interrupt. No idea about
> amiga (Geert?) -- this has to be handled correctly to get a monotonic
> clocksource. I'll fix this in v3 (where the information is available).

Cool!

Kars.

[RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[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>
---
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;
 }
 
 
-- 
2.18.1

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Michael Schmitz]

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;
>  }
>
>
>

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Linus Walleij]

On Mon, Nov 19, 2018 at 2:22 AM Finn Thain <fthain@telegraphics.com.au> wrote:

> 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>
> ---
> TODO: find a spare counter for the clocksource, rather than hanging
> it off the HZ timer.

Yes you already see the same as I see: this chip MK68901 has
no less than four timers. I bet the kernel is just using one of them,
out of habit.

By just setting another timer as free-running we get a classic
and clean Linux clocksource for the Atari.

This is however a very good start in untangling the mess
(as is the whole patch series).

As with the Amiga, this chip also has an RTC clock that should
go to the RTC subsystem, naturally.

Yours,
Linus Walleij

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Geert Uytterhoeven]

Hi Linus,

On Tue, Nov 20, 2018 at 9:10 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> As with the Amiga, this chip also has an RTC clock that should
> go to the RTC subsystem, naturally.

Please note the Amiga CIA is an 8520, not 6526, hence it has a 24-bit TOD
instead of a BCD TOD, so it's not suitable for use as an RTC.

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

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Finn Thain]

On Tue, 20 Nov 2018, Linus Walleij wrote:

> 
> Yes you already see the same as I see: this chip MK68901 has no less 
> than four timers. I bet the kernel is just using one of them, out of 
> habit.
> 
> By just setting another timer as free-running we get a classic and clean 
> Linux clocksource for the Atari.
> 

These are all 8-bit timers. Whereas the smallest clocksource mask I can 
find with grep is 24-bits.

You can divide the oscillator down to 12288 Hz giving a maximum period of 
20 ms. My concern would be that clocksource counter wrap could still go 
undetected given a little interrupt latency.

> This is however a very good start in untangling the mess (as is the 
> whole patch series).
> 

It should be exciting to see what happens when some of these changes get 
tested 8-) I've only seen results for Mac and Atari so far.

> As with the Amiga, this chip also has an RTC clock that should go to the 
> RTC subsystem, naturally.
> 

I think some Atari's have an MC146818, which is drivers/rtc/rtc-cmos.c,
arch/alpha/kernel/rtc.c etc.

-- 

> Yours,
> Linus Walleij
> 

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Linus Walleij]

On Tue, Nov 20, 2018 at 10:30 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On Tue, 20 Nov 2018, Linus Walleij wrote:

> > As with the Amiga, this chip also has an RTC clock that should go to the
> > RTC subsystem, naturally.
>
> I think some Atari's have an MC146818, which is drivers/rtc/rtc-cmos.c,
> arch/alpha/kernel/rtc.c etc.

Indeed, some systems have more than one RTC, often one in their
SoC silicon and one battery backed elsewhere (like a dedicated
chip on I2C or inside a PMIC).

We usually just register more of them. The RTC subsystem can
handle any amount. It's up to userspace to figure out which RTC
to actually use, which is not so good, we should probably
have some heuristic like a quality indicator on them.

Yours,
Linus Walleij

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Andreas Schwab]

On Nov 20 2018, Linus Walleij <linus.walleij@linaro.org> wrote:

> Yes you already see the same as I see: this chip MK68901 has
> no less than four timers. I bet the kernel is just using one of them,
> out of habit.

Note that not all timers can be used freely.  Some of them are hardwired
to generate the clock for the serial interfaces.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Michael Schmitz]

Am 20.11.2018 um 23:02 schrieb Andreas Schwab:
> On Nov 20 2018, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> Yes you already see the same as I see: this chip MK68901 has
>> no less than four timers. I bet the kernel is just using one of them,
>> out of habit.
>
> Note that not all timers can be used freely.  Some of them are hardwired
> to generate the clock for the serial interfaces.

Timer A is used by the DMA sound driver - no workaround possible there.

Timer B is used by the framebuffer driver, but it's used only once to 
reprogram the screen base address at driver init. This one could 
potentially be used after framebuffer init to improve the clocksource 
accuracy.

Timer D is already used to generate timer interrupts used to poll the 
ROM port network card / USB adapters. This timer is initialized early in 
the boot process, which prevents using the MFP UART as serial console 
(something that I hadn't properly considered before). I'll send a patch 
for that. I'll also consider using timer B or timer C interrupts instead 
to poll ROM port hardware.

There are no serial drivers anymore that could use the MFP UART 
hardware, so that point is somewhat moot at present.

Cheers,

	Michael

> Andreas.
>

Re: [RFC PATCH v2 07/14] m68k: atari: Convert to clocksource API

[Finn Thain]

On Sat, 24 Nov 2018, Michael Schmitz wrote:

> 
> Am 20.11.2018 um 23:02 schrieb Andreas Schwab:
> > On Nov 20 2018, Linus Walleij <linus.walleij@linaro.org> wrote:
> > 
> > > Yes you already see the same as I see: this chip MK68901 has no less 
> > > than four timers. I bet the kernel is just using one of them, out of 
> > > habit.
> > 
> > Note that not all timers can be used freely.  Some of them are 
> > hardwired to generate the clock for the serial interfaces.
> 
> Timer A is used by the DMA sound driver - no workaround possible there.
> 
> Timer B is used by the framebuffer driver, but it's used only once to 
> reprogram the screen base address at driver init. This one could 
> potentially be used after framebuffer init to improve the clocksource 
> accuracy.
> 

I don't think it would make a good clocksource as MFP timers are all 8 
bits wide. It could be used as a clock event device in the course of a 
GENERIC_CLOCKEVENTS conversion (discussed elsewhere in this thread), 
though the longest timed interrupt inverval would be only 21 ms.

-- 

> Timer D is already used to generate timer interrupts used to poll the 
> ROM port network card / USB adapters. This timer is initialized early in 
> the boot process, which prevents using the MFP UART as serial console 
> (something that I hadn't properly considered before). I'll send a patch 
> for that. I'll also consider using timer B or timer C interrupts instead 
> to poll ROM port hardware.
> 
> There are no serial drivers anymore that could use the MFP UART 
> hardware, so that point is somewhat moot at present.
> 
> Cheers,
> 
> 	Michael
> 
> > Andreas.
> > 
> 

[RFC PATCH v2 08/14] m68k: bvme6000: Convert to clocksource API

[Finn Thain]

Add a platform clocksource by adapting the existing arch_gettimeoffset
implementation.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
Changed since v1:
 - Moved clk_total access to within the irq lock.
---
 arch/m68k/bvme6000/config.c | 52 +++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/arch/m68k/bvme6000/config.c b/arch/m68k/bvme6000/config.c
index c27c104ac7e7..9691d741e9dc 100644
--- a/arch/m68k/bvme6000/config.c
+++ b/arch/m68k/bvme6000/config.c
@@ -18,6 +18,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/tty.h>
+#include <linux/clocksource.h>
 #include <linux/console.h>
 #include <linux/linkage.h>
 #include <linux/init.h>
@@ -147,6 +148,21 @@ irqreturn_t bvme6000_abort_int (int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static u64 bvme6000_read_clk(struct clocksource *cs);
+
+static struct clocksource bvme6000_clk = {
+	.name   = "rtc",
+	.rating = 250,
+	.read   = bvme6000_read_clk,
+	.mask   = CLOCKSOURCE_MASK(32),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static u32 clk_total;
+
+#define RTC_TIMER_CLOCK_FREQ 8000000
+#define RTC_TIMER_CYCLES     (RTC_TIMER_CLOCK_FREQ / HZ)
+#define RTC_TIMER_COUNT      ((RTC_TIMER_CYCLES / 2) - 1)
 
 static irqreturn_t bvme6000_timer_int (int irq, void *dev_id)
 {
@@ -158,6 +174,7 @@ static irqreturn_t bvme6000_timer_int (int irq, void *dev_id)
     local_irq_save(flags);
     msr = rtc->msr & 0xc0;
     rtc->msr = msr | 0x20;		/* Ack the interrupt */
+    clk_total += RTC_TIMER_CYCLES;
     timer_routine(0, NULL);
     local_irq_restore(flags);
 
@@ -180,13 +197,13 @@ void bvme6000_sched_init (irq_handler_t timer_routine)
 
     rtc->msr = 0;	/* Ensure timer registers accessible */
 
-    if (request_irq(BVME_IRQ_RTC, bvme6000_timer_int, 0,
-				"timer", timer_routine))
+    if (request_irq(BVME_IRQ_RTC, bvme6000_timer_int, IRQF_TIMER, "timer",
+                    timer_routine))
 	panic ("Couldn't register timer int");
 
     rtc->t1cr_omr = 0x04;	/* Mode 2, ext clk */
-    rtc->t1msb = 39999 >> 8;
-    rtc->t1lsb = 39999 & 0xff;
+    rtc->t1msb = RTC_TIMER_COUNT >> 8;
+    rtc->t1lsb = RTC_TIMER_COUNT & 0xff;
     rtc->irr_icr1 &= 0xef;	/* Route timer 1 to INTR pin */
     rtc->msr = 0x40;		/* Access int.cntrl, etc */
     rtc->pfr_icr0 = 0x80;	/* Just timer 1 ints enabled */
@@ -198,14 +215,14 @@ void bvme6000_sched_init (irq_handler_t timer_routine)
 
     rtc->msr = msr;
 
+    clocksource_register_hz(&bvme6000_clk, RTC_TIMER_CLOCK_FREQ);
+
     if (request_irq(BVME_IRQ_ABORT, bvme6000_abort_int, 0,
 				"abort", bvme6000_abort_int))
 	panic ("Couldn't register abort int");
 }
 
 
-/* This is always executed with interrupts disabled.  */
-
 /*
  * NOTE:  Don't accept any readings within 5us of rollover, as
  * the T1INT bit may be a little slow getting set.  There is also
@@ -213,14 +230,18 @@ void bvme6000_sched_init (irq_handler_t timer_routine)
  * results...
  */
 
-u32 bvme6000_gettimeoffset(void)
+static u64 bvme6000_read_clk(struct clocksource *cs)
 {
+    unsigned long flags;
     volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
     volatile PitRegsPtr pit = (PitRegsPtr)BVME_PIT_BASE;
-    unsigned char msr = rtc->msr & 0xc0;
+    unsigned char msr;
     unsigned char t1int, t1op;
     u32 v = 800000, ov;
 
+    local_irq_save(flags);
+
+    msr = rtc->msr & 0xc0;
     rtc->msr = 0;	/* Ensure timer registers accessible */
 
     do {
@@ -233,17 +254,20 @@ u32 bvme6000_gettimeoffset(void)
     } while (t1int != (rtc->msr & 0x20) ||
 		t1op != (pit->pcdr & 0x04) ||
 			abs(ov-v) > 80 ||
-				v > 39960);
+				v > RTC_TIMER_COUNT - (RTC_TIMER_COUNT / 100));
 
-    v = 39999 - v;
+    v = RTC_TIMER_COUNT - v;
     if (!t1op)				/* If in second half cycle.. */
-	v += 40000;
-    v /= 8;				/* Convert ticks to microseconds */
+	v += RTC_TIMER_CYCLES / 2;
     if (t1int)
-	v += 10000;			/* Int pending, + 10ms */
+	v += RTC_TIMER_CYCLES;
     rtc->msr = msr;
 
-    return v * 1000;
+    v += clk_total;
+
+    local_irq_restore(flags);
+
+    return v;
 }
 
 /*
-- 
2.18.1

[RFC PATCH v2 11/14] m68k: mvme147: Convert to clocksource API

[Finn Thain]

Add a platform clocksource by adapting the existing arch_gettimeoffset
implementation.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
Changed since v1:
 - Moved clk_total access to within the irq lock.
 - Use type u32 for tick counter.
---
 arch/m68k/include/asm/mvme147hw.h |  1 -
 arch/m68k/mvme147/config.c        | 38 ++++++++++++++++++++++++++-----
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h
index 9c7ff67c5ffd..7c3dd513128e 100644
--- a/arch/m68k/include/asm/mvme147hw.h
+++ b/arch/m68k/include/asm/mvme147hw.h
@@ -66,7 +66,6 @@ struct pcc_regs {
 #define PCC_INT_ENAB		0x08
 
 #define PCC_TIMER_INT_CLR	0x80
-#define PCC_TIMER_PRELOAD	63936l
 
 #define PCC_LEVEL_ABORT		0x07
 #define PCC_LEVEL_SERIAL	0x04
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c
index 4ef4faa5ed8b..82b53b5ca82b 100644
--- a/arch/m68k/mvme147/config.c
+++ b/arch/m68k/mvme147/config.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/tty.h>
+#include <linux/clocksource.h>
 #include <linux/console.h>
 #include <linux/linkage.h>
 #include <linux/init.h>
@@ -92,6 +93,21 @@ void __init config_mvme147(void)
 		vme_brdtype = VME_TYPE_MVME147;
 }
 
+static u64 mvme147_read_clk(struct clocksource *cs);
+
+static struct clocksource mvme147_clk = {
+	.name   = "pcc",
+	.rating = 250,
+	.read   = mvme147_read_clk,
+	.mask   = CLOCKSOURCE_MASK(32),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static u32 clk_total;
+
+#define PCC_TIMER_CLOCK_FREQ 160000
+#define PCC_TIMER_CYCLES     (PCC_TIMER_CLOCK_FREQ / HZ)
+#define PCC_TIMER_PRELOAD    (0x10000 - PCC_TIMER_CYCLES)
 
 /* Using pcc tick timer 1 */
 
@@ -103,6 +119,7 @@ static irqreturn_t mvme147_timer_int (int irq, void *dev_id)
 	local_irq_save(flags);
 	m147_pcc->t1_int_cntrl = PCC_TIMER_INT_CLR;
 	m147_pcc->t1_int_cntrl = PCC_INT_ENAB|PCC_LEVEL_TIMER1;
+	clk_total += PCC_TIMER_CYCLES;
 	timer_routine(0, NULL);
 	local_irq_restore(flags);
 
@@ -112,32 +129,41 @@ static irqreturn_t mvme147_timer_int (int irq, void *dev_id)
 
 void mvme147_sched_init (irq_handler_t timer_routine)
 {
-	if (request_irq(PCC_IRQ_TIMER1, mvme147_timer_int, 0, "timer 1",
-			timer_routine))
+	if (request_irq(PCC_IRQ_TIMER1, mvme147_timer_int, IRQF_TIMER,
+			"timer 1", timer_routine))
 		pr_err("Couldn't register timer interrupt\n");
 
 	/* Init the clock with a value */
-	/* our clock goes off every 6.25us */
+	/* The clock counter increments until 0xFFFF then reloads */
 	m147_pcc->t1_preload = PCC_TIMER_PRELOAD;
 	m147_pcc->t1_cntrl = 0x0;	/* clear timer */
 	m147_pcc->t1_cntrl = 0x3;	/* start timer */
 	m147_pcc->t1_int_cntrl = PCC_TIMER_INT_CLR;  /* clear pending ints */
 	m147_pcc->t1_int_cntrl = PCC_INT_ENAB|PCC_LEVEL_TIMER1;
+
+	clocksource_register_hz(&mvme147_clk, PCC_TIMER_CLOCK_FREQ);
 }
 
-/* This is always executed with interrupts disabled.  */
 /* XXX There are race hazards in this code XXX */
-u32 mvme147_gettimeoffset(void)
+static u64 mvme147_read_clk(struct clocksource *cs)
 {
+	unsigned long flags;
 	volatile unsigned short *cp = (volatile unsigned short *)0xfffe1012;
 	unsigned short n;
+	u32 ticks;
+
+	local_irq_save(flags);
 
 	n = *cp;
 	while (n != *cp)
 		n = *cp;
 
 	n -= PCC_TIMER_PRELOAD;
-	return ((unsigned long)n * 25 / 4) * 1000;
+	ticks = clk_total + n;
+
+	local_irq_restore(flags);
+
+	return ticks;
 }
 
 static int bcd2int (unsigned char b)
-- 
2.18.1

[RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

[Finn Thain]

Add a platform clocksource by adapting the existing arch_gettimeoffset
implementation.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Stan Johnson <userm57@yahoo.com>
---
Changed since v1:
 - Moved clk_total access to within the irq lock.
 - Use type u32 for tick counter.
---
 arch/m68k/mac/via.c | 46 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index d1dbf9017300..de59a5cb4250 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -23,6 +23,7 @@
  *
  */
 
+#include <linux/clocksource.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
@@ -573,16 +574,40 @@ EXPORT_SYMBOL(via2_scsi_drq_pending);
 /* timer and clock source */
 
 #define VIA_CLOCK_FREQ     783360                /* VIA "phase 2" clock in Hz */
-#define VIA_TIMER_INTERVAL (1000000 / HZ)        /* microseconds per jiffy */
 #define VIA_TIMER_CYCLES   (VIA_CLOCK_FREQ / HZ) /* clock cycles per jiffy */
 
 #define VIA_TC             (VIA_TIMER_CYCLES - 2) /* including 0 and -1 */
 #define VIA_TC_LOW         (VIA_TC & 0xFF)
 #define VIA_TC_HIGH        (VIA_TC >> 8)
 
+static u64 mac_read_clk(struct clocksource *cs);
+
+static struct clocksource mac_clk = {
+	.name   = "via1",
+	.rating = 250,
+	.read   = mac_read_clk,
+	.mask   = CLOCKSOURCE_MASK(32),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static u32 clk_total;
+static u32 clk_offset;
+
+static irqreturn_t via_timer_handler(int irq, void *dev_id)
+{
+	irq_handler_t timer_routine = dev_id;
+
+	clk_total += VIA_TIMER_CYCLES;
+	clk_offset = 0;
+	timer_routine(0, NULL);
+
+	return IRQ_HANDLED;
+}
+
 void __init via_init_clock(irq_handler_t timer_routine)
 {
-	if (request_irq(IRQ_MAC_TIMER_1, timer_routine, 0, "timer", NULL)) {
+	if (request_irq(IRQ_MAC_TIMER_1, via_timer_handler, IRQF_TIMER, "timer",
+			timer_routine)) {
 		pr_err("Couldn't register %s interrupt\n", "timer");
 		return;
 	}
@@ -592,13 +617,16 @@ void __init via_init_clock(irq_handler_t timer_routine)
 	via1[vT1CL] = VIA_TC_LOW;
 	via1[vT1CH] = VIA_TC_HIGH;
 	via1[vACR] |= 0x40;
+
+	clocksource_register_hz(&mac_clk, VIA_CLOCK_FREQ);
 }
 
-u32 mac_gettimeoffset(void)
+static u64 mac_read_clk(struct clocksource *cs)
 {
 	unsigned long flags;
 	u8 count_high;
-	u16 count, offset = 0;
+	u16 count;
+	u32 ticks;
 
 	/*
 	 * Timer counter wrap-around is detected with the timer interrupt flag
@@ -617,11 +645,11 @@ u32 mac_gettimeoffset(void)
 			/* spin */;
 	}
 	if (via1[vIFR] & VIA_TIMER_1_INT)
-		offset = VIA_TIMER_CYCLES;
-	local_irq_restore(flags);
-
+		clk_offset = VIA_TIMER_CYCLES;
 	count = count_high << 8;
-	count = VIA_TIMER_CYCLES - count + offset;
+	ticks = VIA_TIMER_CYCLES - count;
+	ticks += clk_offset + clk_total;
+	local_irq_restore(flags);
 
-	return ((count * VIA_TIMER_INTERVAL) / VIA_TIMER_CYCLES) * 1000;
+	return ticks;
 }
-- 
2.18.1

Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

[Linus Walleij]

On Mon, Nov 19, 2018 at 2:22 AM Finn Thain <fthain@telegraphics.com.au> wrote:

> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Stan Johnson <userm57@yahoo.com>

As noted for the Amiga CIA (which is pretty much a sibling to this
MOS 6522 VIA) this chip also has two counters and could use one
as clocksource and the other as clock event.

Again I bet this is just using one timer out of habit.

It will be an easy feat to make a clean clocksource+clock event
for this hardware as well once this refactoring is complete.

Yours,
Linus Walleij

Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

[Finn Thain]

On Tue, 20 Nov 2018, Linus Walleij wrote:

> On Mon, Nov 19, 2018 at 2:22 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> 
> > Add a platform clocksource by adapting the existing arch_gettimeoffset
> > implementation.
> >
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Tested-by: Stan Johnson <userm57@yahoo.com>
> 
> As noted for the Amiga CIA (which is pretty much a sibling to this MOS 
> 6522 VIA) this chip also has two counters and could use one as 
> clocksource and the other as clock event.
> 
> Again I bet this is just using one timer out of habit.
> 
> It will be an easy feat to make a clean clocksource+clock event for this 
> hardware as well once this refactoring is complete.
> 

Right. Both timer 1 and timer 2 have a timed interrupt mode, and either 
could probably serve as a clock event device or a clocksource. Some Mac 
models have a second VIA with two more timers, but not all.

(As clock event devices, the longest interval you can program is about 83 
ms. As 783360 Hz, 16-bit clocksources, they wrap about 12 times every 
second.)

If you use one timer as a clock event device and the other as a 
clocksource, there are no timers left to run the existing 
timer_interrupt() handler. So this arrangement would require 
GENERIC_CLOCKEVENTS=y, right?

Then, wouldn't all relevant platforms have to support 
GENERIC_CLOCKEVENTS=y, if a single binary was to support all of those 
platforms?

-- 

> Yours,
> Linus Walleij
> 

Re: [RFC PATCH v2 10/14] m68k: mac: Convert to clocksource API

[Linus Walleij]

On Tue, Nov 20, 2018 at 10:00 AM Finn Thain <fthain@telegraphics.com.au> wrote:

> If you use one timer as a clock event device and the other as a
> clocksource, there are no timers left to run the existing
> timer_interrupt() handler. So this arrangement would require
> GENERIC_CLOCKEVENTS=y, right?

I think so, yes.

> Then, wouldn't all relevant platforms have to support
> GENERIC_CLOCKEVENTS=y, if a single binary was to support all of those
> platforms?

Good point. I wonder of there is a path forward where we
could have peaceful GENERIC_CLOCKEVENTS and
!GENERIC_CLOCKEVENTS co-existence.

Yours,
Linus Walleij

[RFC PATCH v2 04/14] m68k: apollo, q40, sun3, sun3x: Remove arch_gettimeoffset implementations

[Finn Thain]

These dummy implementations are no better than
default_arch_gettimeoffset() so remove them.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/apollo/config.c | 7 -------
 arch/m68k/q40/config.c    | 9 ---------
 arch/m68k/sun3/config.c   | 2 --
 arch/m68k/sun3/intersil.c | 7 -------
 arch/m68k/sun3x/config.c  | 1 -
 arch/m68k/sun3x/time.c    | 5 -----
 arch/m68k/sun3x/time.h    | 1 -
 7 files changed, 32 deletions(-)

diff --git a/arch/m68k/apollo/config.c b/arch/m68k/apollo/config.c
index aef8d42e078d..7d168e6dfb01 100644
--- a/arch/m68k/apollo/config.c
+++ b/arch/m68k/apollo/config.c
@@ -29,7 +29,6 @@ u_long apollo_model;
 
 extern void dn_sched_init(irq_handler_t handler);
 extern void dn_init_IRQ(void);
-extern u32 dn_gettimeoffset(void);
 extern int dn_dummy_hwclk(int, struct rtc_time *);
 extern void dn_dummy_reset(void);
 #ifdef CONFIG_HEARTBEAT
@@ -152,7 +151,6 @@ void __init config_apollo(void)
 
 	mach_sched_init=dn_sched_init; /* */
 	mach_init_IRQ=dn_init_IRQ;
-	arch_gettimeoffset   = dn_gettimeoffset;
 	mach_max_dma_address = 0xffffffff;
 	mach_hwclk           = dn_dummy_hwclk; /* */
 	mach_reset	     = dn_dummy_reset;  /* */
@@ -205,11 +203,6 @@ void dn_sched_init(irq_handler_t timer_routine)
 		pr_err("Couldn't register timer interrupt\n");
 }
 
-u32 dn_gettimeoffset(void)
-{
-	return 0xdeadbeef;
-}
-
 int dn_dummy_hwclk(int op, struct rtc_time *t) {
 
 
diff --git a/arch/m68k/q40/config.c b/arch/m68k/q40/config.c
index 96810d91da2b..e63eb5f06999 100644
--- a/arch/m68k/q40/config.c
+++ b/arch/m68k/q40/config.c
@@ -40,7 +40,6 @@ extern void q40_init_IRQ(void);
 static void q40_get_model(char *model);
 extern void q40_sched_init(irq_handler_t handler);
 
-static u32 q40_gettimeoffset(void);
 static int q40_hwclk(int, struct rtc_time *);
 static unsigned int q40_get_ss(void);
 static int q40_get_rtc_pll(struct rtc_pll_info *pll);
@@ -169,7 +168,6 @@ void __init config_q40(void)
 	mach_sched_init = q40_sched_init;
 
 	mach_init_IRQ = q40_init_IRQ;
-	arch_gettimeoffset = q40_gettimeoffset;
 	mach_hwclk = q40_hwclk;
 	mach_get_ss = q40_get_ss;
 	mach_get_rtc_pll = q40_get_rtc_pll;
@@ -201,13 +199,6 @@ int __init q40_parse_bootinfo(const struct bi_record *rec)
 	return 1;
 }
 
-
-static u32 q40_gettimeoffset(void)
-{
-	return 5000 * (ql_ticks != 0) * 1000;
-}
-
-
 /*
  * Looks like op is non-zero for setting the clock, and zero for
  * reading the clock.
diff --git a/arch/m68k/sun3/config.c b/arch/m68k/sun3/config.c
index 79a2bb857906..867e68d92c71 100644
--- a/arch/m68k/sun3/config.c
+++ b/arch/m68k/sun3/config.c
@@ -37,7 +37,6 @@
 
 char sun3_reserved_pmeg[SUN3_PMEGS_NUM];
 
-extern u32 sun3_gettimeoffset(void);
 static void sun3_sched_init(irq_handler_t handler);
 extern void sun3_get_model (char* model);
 extern int sun3_hwclk(int set, struct rtc_time *t);
@@ -138,7 +137,6 @@ void __init config_sun3(void)
         mach_sched_init      =  sun3_sched_init;
         mach_init_IRQ        =  sun3_init_IRQ;
         mach_reset           =  sun3_reboot;
-	arch_gettimeoffset   =  sun3_gettimeoffset;
 	mach_get_model	     =  sun3_get_model;
 	mach_hwclk           =  sun3_hwclk;
 	mach_halt	     =  sun3_halt;
diff --git a/arch/m68k/sun3/intersil.c b/arch/m68k/sun3/intersil.c
index d911070af02a..8fc74864de81 100644
--- a/arch/m68k/sun3/intersil.c
+++ b/arch/m68k/sun3/intersil.c
@@ -22,13 +22,6 @@
 #define STOP_VAL (INTERSIL_STOP | INTERSIL_INT_ENABLE | INTERSIL_24H_MODE)
 #define START_VAL (INTERSIL_RUN | INTERSIL_INT_ENABLE | INTERSIL_24H_MODE)
 
-/* does this need to be implemented? */
-u32 sun3_gettimeoffset(void)
-{
-  return 1000;
-}
-
-
 /* get/set hwclock */
 
 int sun3_hwclk(int set, struct rtc_time *t)
diff --git a/arch/m68k/sun3x/config.c b/arch/m68k/sun3x/config.c
index 33d3a1c6fba0..03ce7f9facfe 100644
--- a/arch/m68k/sun3x/config.c
+++ b/arch/m68k/sun3x/config.c
@@ -49,7 +49,6 @@ void __init config_sun3x(void)
 	mach_sched_init      = sun3x_sched_init;
 	mach_init_IRQ        = sun3_init_IRQ;
 
-	arch_gettimeoffset   = sun3x_gettimeoffset;
 	mach_reset           = sun3x_reboot;
 
 	mach_hwclk           = sun3x_hwclk;
diff --git a/arch/m68k/sun3x/time.c b/arch/m68k/sun3x/time.c
index 3c8a86d08508..9163294b0fb6 100644
--- a/arch/m68k/sun3x/time.c
+++ b/arch/m68k/sun3x/time.c
@@ -73,11 +73,6 @@ int sun3x_hwclk(int set, struct rtc_time *t)
 
 	return 0;
 }
-/* Not much we can do here */
-u32 sun3x_gettimeoffset(void)
-{
-    return 0L;
-}
 
 #if 0
 static irqreturn_t sun3x_timer_tick(int irq, void *dev_id)
diff --git a/arch/m68k/sun3x/time.h b/arch/m68k/sun3x/time.h
index 496f406412ad..86ce78bb3c28 100644
--- a/arch/m68k/sun3x/time.h
+++ b/arch/m68k/sun3x/time.h
@@ -3,7 +3,6 @@
 #define SUN3X_TIME_H
 
 extern int sun3x_hwclk(int set, struct rtc_time *t);
-u32 sun3x_gettimeoffset(void);
 void sun3x_sched_init(irq_handler_t vector);
 
 struct mostek_dt {
-- 
2.18.1

[RFC PATCH v2 01/14] m68k: Call timer_interrupt() with interrupts disabled

[Finn Thain]

Some platforms execute their timer handler with the interrupt priority
level below 6. That means the handler could be interrupted by another
driver and this could lead to re-entry of the timer core.

Avoid this by use of local_irq_save/restore for timer interrupt dispatch.
This provides mutual exclusion around the timer interrupt flag access
which is needed later in this series for the clocksource coversion.

Reported-by: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/alpine.DEB.2.21.1811131407120.2697@nanos.tec.linutronix.de
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
I've only checked 680x0 MMU platforms for this issue.
---
 arch/m68k/amiga/cia.c       | 10 ++++++++++
 arch/m68k/atari/ataints.c   |  4 ++--
 arch/m68k/atari/time.c      | 15 ++++++++++++++-
 arch/m68k/bvme6000/config.c | 18 +++++++++---------
 arch/m68k/hp300/time.c      | 10 ++++++++--
 arch/m68k/mac/via.c         | 17 +++++++++++++++++
 arch/m68k/mvme147/config.c  | 18 ++++++++++--------
 arch/m68k/mvme16x/config.c  | 19 ++++++++++---------
 arch/m68k/q40/q40ints.c     |  7 ++++++-
 arch/m68k/sun3/sun3ints.c   |  3 +++
 arch/m68k/sun3x/time.c      | 16 ++++++++++------
 11 files changed, 99 insertions(+), 38 deletions(-)

diff --git a/arch/m68k/amiga/cia.c b/arch/m68k/amiga/cia.c
index 2081b8cd5591..b65665160711 100644
--- a/arch/m68k/amiga/cia.c
+++ b/arch/m68k/amiga/cia.c
@@ -88,10 +88,20 @@ static irqreturn_t cia_handler(int irq, void *dev_id)
 	struct ciabase *base = dev_id;
 	int mach_irq;
 	unsigned char ints;
+	unsigned long flags;
 
+	/* Interrupts get disabled while the timer irq flag is cleared and
+	 * the timer interrupt serviced.
+	 */
+	local_irq_save(flags);
 	mach_irq = base->cia_irq;
 	ints = cia_set_irq(base, CIA_ICR_ALL);
 	amiga_custom.intreq = base->int_mask;
+	if (ints & 1) {
+		generic_handle_irq(mach_irq);
+		mach_irq++, ints >>= 1;
+	}
+	local_irq_restore(flags);
 	for (; ints; mach_irq++, ints >>= 1) {
 		if (ints & 1)
 			generic_handle_irq(mach_irq);
diff --git a/arch/m68k/atari/ataints.c b/arch/m68k/atari/ataints.c
index 3d2b63bedf05..56f02ea2c248 100644
--- a/arch/m68k/atari/ataints.c
+++ b/arch/m68k/atari/ataints.c
@@ -142,7 +142,7 @@ struct mfptimerbase {
 	.name		= "MFP Timer D"
 };
 
-static irqreturn_t mfptimer_handler(int irq, void *dev_id)
+static irqreturn_t mfp_timer_d_handler(int irq, void *dev_id)
 {
 	struct mfptimerbase *base = dev_id;
 	int mach_irq;
@@ -344,7 +344,7 @@ void __init atari_init_IRQ(void)
 	st_mfp.tim_ct_cd = (st_mfp.tim_ct_cd & 0xf0) | 0x6;
 
 	/* request timer D dispatch handler */
-	if (request_irq(IRQ_MFP_TIMD, mfptimer_handler, IRQF_SHARED,
+	if (request_irq(IRQ_MFP_TIMD, mfp_timer_d_handler, IRQF_SHARED,
 			stmfp_base.name, &stmfp_base))
 		pr_err("Couldn't register %s interrupt\n", stmfp_base.name);
 
diff --git a/arch/m68k/atari/time.c b/arch/m68k/atari/time.c
index 9cca64286464..fafa20f75ab9 100644
--- a/arch/m68k/atari/time.c
+++ b/arch/m68k/atari/time.c
@@ -24,6 +24,18 @@
 DEFINE_SPINLOCK(rtc_lock);
 EXPORT_SYMBOL_GPL(rtc_lock);
 
+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);
+	timer_routine(0, NULL);
+	local_irq_restore(flags);
+
+	return IRQ_HANDLED;
+}
+
 void __init
 atari_sched_init(irq_handler_t timer_routine)
 {
@@ -32,7 +44,8 @@ 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, timer_routine, 0, "timer", timer_routine))
+    if (request_irq(IRQ_MFP_TIMC, mfp_timer_c_handler, 0, "timer",
+                    timer_routine))
 	pr_err("Couldn't register timer interrupt\n");
 }
 
diff --git a/arch/m68k/bvme6000/config.c b/arch/m68k/bvme6000/config.c
index 143ee9fa3893..d1de3cb1f8fe 100644
--- a/arch/m68k/bvme6000/config.c
+++ b/arch/m68k/bvme6000/config.c
@@ -44,11 +44,6 @@ extern int bvme6000_hwclk (int, struct rtc_time *);
 extern void bvme6000_reset (void);
 void bvme6000_set_vectors (void);
 
-/* Save tick handler routine pointer, will point to xtime_update() in
- * kernel/timer/timekeeping.c, called via bvme6000_process_int() */
-
-static irq_handler_t tick_handler;
-
 
 int __init bvme6000_parse_bootinfo(const struct bi_record *bi)
 {
@@ -157,12 +152,18 @@ irqreturn_t bvme6000_abort_int (int irq, void *dev_id)
 
 static irqreturn_t bvme6000_timer_int (int irq, void *dev_id)
 {
+    irq_handler_t timer_routine = dev_id;
+    unsigned long flags;
     volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
-    unsigned char msr = rtc->msr & 0xc0;
+    unsigned char msr;
 
+    local_irq_save(flags);
+    msr = rtc->msr & 0xc0;
     rtc->msr = msr | 0x20;		/* Ack the interrupt */
+    timer_routine(0, NULL);
+    local_irq_restore(flags);
 
-    return tick_handler(irq, dev_id);
+    return IRQ_HANDLED;
 }
 
 /*
@@ -181,9 +182,8 @@ void bvme6000_sched_init (irq_handler_t timer_routine)
 
     rtc->msr = 0;	/* Ensure timer registers accessible */
 
-    tick_handler = timer_routine;
     if (request_irq(BVME_IRQ_RTC, bvme6000_timer_int, 0,
-				"timer", bvme6000_timer_int))
+				"timer", timer_routine))
 	panic ("Couldn't register timer int");
 
     rtc->t1cr_omr = 0x04;	/* Mode 2, ext clk */
diff --git a/arch/m68k/hp300/time.c b/arch/m68k/hp300/time.c
index 289d928a46cb..d30b03ea93a2 100644
--- a/arch/m68k/hp300/time.c
+++ b/arch/m68k/hp300/time.c
@@ -38,13 +38,19 @@
 
 static irqreturn_t hp300_tick(int irq, void *dev_id)
 {
+	irq_handler_t timer_routine = dev_id;
+	unsigned long flags;
 	unsigned long tmp;
-	irq_handler_t vector = dev_id;
+
+	local_irq_save(flags);
 	in_8(CLOCKBASE + CLKSR);
 	asm volatile ("movpw %1@(5),%0" : "=d" (tmp) : "a" (CLOCKBASE));
+	timer_routine(0, NULL);
+	local_irq_restore(flags);
+
 	/* Turn off the network and SCSI leds */
 	blinken_leds(0, 0xe0);
-	return vector(irq, NULL);
+	return IRQ_HANDLED;
 }
 
 u32 hp300_gettimeoffset(void)
diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index e4facff0c1f3..2ab85b6eb4fe 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -436,6 +436,8 @@ void via_nubus_irq_shutdown(int irq)
  * via6522.c :-), disable/pending masks added.
  */
 
+#define VIA_TIMER_1_INT BIT(6)
+
 void via1_irq(struct irq_desc *desc)
 {
 	int irq_num;
@@ -445,6 +447,21 @@ void via1_irq(struct irq_desc *desc)
 	if (!events)
 		return;
 
+	irq_num = IRQ_MAC_TIMER_1;
+	irq_bit = VIA_TIMER_1_INT;
+	if (events & irq_bit) {
+		unsigned long flags;
+
+		local_irq_save(flags);
+		via1[vIFR] = irq_bit;
+		generic_handle_irq(IRQ_MAC_TIMER_1);
+		local_irq_restore(flags);
+
+		events &= ~irq_bit;
+		if (!events)
+			return;
+	}
+
 	irq_num = VIA1_SOURCE_BASE;
 	irq_bit = 1;
 	do {
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c
index adea549d240e..93c68d2b8e0e 100644
--- a/arch/m68k/mvme147/config.c
+++ b/arch/m68k/mvme147/config.c
@@ -45,11 +45,6 @@ extern void mvme147_reset (void);
 
 static int bcd2int (unsigned char b);
 
-/* Save tick handler routine pointer, will point to xtime_update() in
- * kernel/time/timekeeping.c, called via mvme147_process_int() */
-
-irq_handler_t tick_handler;
-
 
 int __init mvme147_parse_bootinfo(const struct bi_record *bi)
 {
@@ -104,16 +99,23 @@ void __init config_mvme147(void)
 
 static irqreturn_t mvme147_timer_int (int irq, void *dev_id)
 {
+	irq_handler_t timer_routine = dev_id;
+	unsigned long flags;
+
+	local_irq_save(flags);
 	m147_pcc->t1_int_cntrl = PCC_TIMER_INT_CLR;
 	m147_pcc->t1_int_cntrl = PCC_INT_ENAB|PCC_LEVEL_TIMER1;
-	return tick_handler(irq, dev_id);
+	timer_routine(0, NULL);
+	local_irq_restore(flags);
+
+	return IRQ_HANDLED;
 }
 
 
 void mvme147_sched_init (irq_handler_t timer_routine)
 {
-	tick_handler = timer_routine;
-	if (request_irq(PCC_IRQ_TIMER1, mvme147_timer_int, 0, "timer 1", NULL))
+	if (request_irq(PCC_IRQ_TIMER1, mvme147_timer_int, 0, "timer 1",
+			timer_routine))
 		pr_err("Couldn't register timer interrupt\n");
 
 	/* Init the clock with a value */
diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c
index 6ee36a5b528d..4e6229856396 100644
--- a/arch/m68k/mvme16x/config.c
+++ b/arch/m68k/mvme16x/config.c
@@ -50,11 +50,6 @@ extern void mvme16x_reset (void);
 
 int bcd2int (unsigned char b);
 
-/* Save tick handler routine pointer, will point to xtime_update() in
- * kernel/time/timekeeping.c, called via mvme16x_process_int() */
-
-static irq_handler_t tick_handler;
-
 
 unsigned short mvme16x_config;
 EXPORT_SYMBOL(mvme16x_config);
@@ -352,8 +347,15 @@ static irqreturn_t mvme16x_abort_int (int irq, void *dev_id)
 
 static irqreturn_t mvme16x_timer_int (int irq, void *dev_id)
 {
-    *(volatile unsigned char *)0xfff4201b |= 8;
-    return tick_handler(irq, dev_id);
+	irq_handler_t timer_routine = dev_id;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	*(volatile unsigned char *)0xfff4201b |= 8;
+	timer_routine(0, NULL);
+	local_irq_restore(flags);
+
+	return IRQ_HANDLED;
 }
 
 void mvme16x_sched_init (irq_handler_t timer_routine)
@@ -361,14 +363,13 @@ void mvme16x_sched_init (irq_handler_t timer_routine)
     uint16_t brdno = be16_to_cpu(mvme_bdid.brdno);
     int irq;
 
-    tick_handler = timer_routine;
     /* Using PCCchip2 or MC2 chip tick timer 1 */
     *(volatile unsigned long *)0xfff42008 = 0;
     *(volatile unsigned long *)0xfff42004 = 10000;	/* 10ms */
     *(volatile unsigned char *)0xfff42017 |= 3;
     *(volatile unsigned char *)0xfff4201b = 0x16;
     if (request_irq(MVME16x_IRQ_TIMER, mvme16x_timer_int, 0,
-				"timer", mvme16x_timer_int))
+                    "timer", timer_routine))
 	panic ("Couldn't register timer int");
 
     if (brdno == 0x0162 || brdno == 0x172)
diff --git a/arch/m68k/q40/q40ints.c b/arch/m68k/q40/q40ints.c
index 3e7603202977..ea339c2d47d1 100644
--- a/arch/m68k/q40/q40ints.c
+++ b/arch/m68k/q40/q40ints.c
@@ -139,8 +139,13 @@ static irqreturn_t q40_timer_int (int irq, void * dev)
 		*DAC_RIGHT=sval;
 	}
 
-	if (!ql_ticks)
+	if (!ql_ticks) {
+		unsigned long flags;
+
+		local_irq_save(flags);
 		q40_timer_routine(irq, dev);
+		local_irq_restore(flags);
+	}
 	return IRQ_HANDLED;
 }
 
diff --git a/arch/m68k/sun3/sun3ints.c b/arch/m68k/sun3/sun3ints.c
index 6bbca30c9188..a5824abb4a39 100644
--- a/arch/m68k/sun3/sun3ints.c
+++ b/arch/m68k/sun3/sun3ints.c
@@ -61,8 +61,10 @@ static irqreturn_t sun3_int7(int irq, void *dev_id)
 
 static irqreturn_t sun3_int5(int irq, void *dev_id)
 {
+	unsigned long flags;
 	unsigned int cnt;
 
+	local_irq_save(flags);
 #ifdef CONFIG_SUN3
 	intersil_clear();
 #endif
@@ -76,6 +78,7 @@ static irqreturn_t sun3_int5(int irq, void *dev_id)
 	cnt = kstat_irqs_cpu(irq, 0);
 	if (!(cnt % 20))
 		sun3_leds(led_pattern[cnt % 160 / 20]);
+	local_irq_restore(flags);
 	return IRQ_HANDLED;
 }
 
diff --git a/arch/m68k/sun3x/time.c b/arch/m68k/sun3x/time.c
index 047e2bcee3d7..3c8a86d08508 100644
--- a/arch/m68k/sun3x/time.c
+++ b/arch/m68k/sun3x/time.c
@@ -80,15 +80,19 @@ u32 sun3x_gettimeoffset(void)
 }
 
 #if 0
-static void sun3x_timer_tick(int irq, void *dev_id, struct pt_regs *regs)
+static irqreturn_t sun3x_timer_tick(int irq, void *dev_id)
 {
-    void (*vector)(int, void *, struct pt_regs *) = dev_id;
+	irq_handler_t timer_routine = dev_id;
+	unsigned long flags;
 
-    /* Clear the pending interrupt - pulse the enable line low */
-    disable_irq(5);
-    enable_irq(5);
+	local_irq_save(flags);
+	/* Clear the pending interrupt - pulse the enable line low */
+	disable_irq(5);
+	enable_irq(5);
+	timer_routine(0, NULL);
+	local_irq_restore(flags);
 
-    vector(irq, NULL, regs);
+	return IRQ_HANDLED;
 }
 #endif
 
-- 
2.18.1

[RFC PATCH v2 03/14] m68k: mac: Clean up unused timer definitions

[Finn Thain]

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/include/asm/macints.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/m68k/include/asm/macints.h b/arch/m68k/include/asm/macints.h
index cddb2d3ea49b..4da172bd048c 100644
--- a/arch/m68k/include/asm/macints.h
+++ b/arch/m68k/include/asm/macints.h
@@ -121,7 +121,4 @@
 #define SLOT2IRQ(x)	  (x + 47)
 #define IRQ2SLOT(x)	  (x - 47)
 
-#define INT_CLK   24576	    /* CLK while int_clk =2.456MHz and divide = 100 */
-#define INT_TICKS 246	    /* to make sched_time = 99.902... HZ */
-
 #endif /* asm/macints.h */
-- 
2.18.1

Re: [RFC PATCH v2 03/14] m68k: mac: Clean up unused timer definitions

[Finn Thain]

Geert,

Please consider cherry-picking this patch. It isn't particularly relevant 
to the API conversion so I'd better not to include it in v3, when I submit 
v3.

-- 

On Mon, 19 Nov 2018, Finn Thain wrote:

> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
>  arch/m68k/include/asm/macints.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/m68k/include/asm/macints.h b/arch/m68k/include/asm/macints.h
> index cddb2d3ea49b..4da172bd048c 100644
> --- a/arch/m68k/include/asm/macints.h
> +++ b/arch/m68k/include/asm/macints.h
> @@ -121,7 +121,4 @@
>  #define SLOT2IRQ(x)	  (x + 47)
>  #define IRQ2SLOT(x)	  (x - 47)
>  
> -#define INT_CLK   24576	    /* CLK while int_clk =2.456MHz and divide = 100 */
> -#define INT_TICKS 246	    /* to make sched_time = 99.902... HZ */
> -
>  #endif /* asm/macints.h */
> 

[RFC PATCH v2 12/14] m68k: mvme147: Handle timer counter overflow

[Finn Thain]

Reading the timer counter races with timer overflow (and the
corresponding interrupt). This is resolved by reading the overflow
register and taking this value into account. The interrupt handler
must clear the overflow register when it eventually executes.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
TODO: find a spare counter for the clocksource, rather than hanging
it off the HZ timer.
---
 arch/m68k/include/asm/mvme147hw.h |  1 +
 arch/m68k/mvme147/config.c        | 23 +++++++++++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/m68k/include/asm/mvme147hw.h b/arch/m68k/include/asm/mvme147hw.h
index 7c3dd513128e..257b29184af9 100644
--- a/arch/m68k/include/asm/mvme147hw.h
+++ b/arch/m68k/include/asm/mvme147hw.h
@@ -66,6 +66,7 @@ struct pcc_regs {
 #define PCC_INT_ENAB		0x08
 
 #define PCC_TIMER_INT_CLR	0x80
+#define PCC_TIMER_CLR_OVF	0x04
 
 #define PCC_LEVEL_ABORT		0x07
 #define PCC_LEVEL_SERIAL	0x04
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c
index 82b53b5ca82b..545a1fe0e119 100644
--- a/arch/m68k/mvme147/config.c
+++ b/arch/m68k/mvme147/config.c
@@ -118,7 +118,7 @@ static irqreturn_t mvme147_timer_int (int irq, void *dev_id)
 
 	local_irq_save(flags);
 	m147_pcc->t1_int_cntrl = PCC_TIMER_INT_CLR;
-	m147_pcc->t1_int_cntrl = PCC_INT_ENAB|PCC_LEVEL_TIMER1;
+	m147_pcc->t1_cntrl = PCC_TIMER_CLR_OVF;
 	clk_total += PCC_TIMER_CYCLES;
 	timer_routine(0, NULL);
 	local_irq_restore(flags);
@@ -144,23 +144,22 @@ void mvme147_sched_init (irq_handler_t timer_routine)
 	clocksource_register_hz(&mvme147_clk, PCC_TIMER_CLOCK_FREQ);
 }
 
-/* XXX There are race hazards in this code XXX */
 static u64 mvme147_read_clk(struct clocksource *cs)
 {
 	unsigned long flags;
-	volatile unsigned short *cp = (volatile unsigned short *)0xfffe1012;
-	unsigned short n;
+	u8 overflow, tmp;
+	u16 count;
 	u32 ticks;
 
 	local_irq_save(flags);
-
-	n = *cp;
-	while (n != *cp)
-		n = *cp;
-
-	n -= PCC_TIMER_PRELOAD;
-	ticks = clk_total + n;
-
+	tmp = m147_pcc->t1_cntrl >> 4;
+	count = m147_pcc->t1_count;
+	overflow = m147_pcc->t1_cntrl >> 4;
+	if (overflow != tmp)
+		count = m147_pcc->t1_count;
+	count -= PCC_TIMER_PRELOAD;
+	ticks = count + overflow * PCC_TIMER_CYCLES;
+	ticks += clk_total;
 	local_irq_restore(flags);
 
 	return ticks;
-- 
2.18.1

[RFC PATCH v2 13/14] m68k: mvme16x: Convert to clocksource API

[Finn Thain]

Add a platform clocksource by adapting the existing arch_gettimeoffset
implementation.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
Changed since v1:
 - Moved clk_total access to within the irq lock.
---
 arch/m68k/mvme16x/config.c | 39 +++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c
index 8bafa6a37593..2c109ee2a1a5 100644
--- a/arch/m68k/mvme16x/config.c
+++ b/arch/m68k/mvme16x/config.c
@@ -19,6 +19,7 @@
 #include <linux/mm.h>
 #include <linux/seq_file.h>
 #include <linux/tty.h>
+#include <linux/clocksource.h>
 #include <linux/console.h>
 #include <linux/linkage.h>
 #include <linux/init.h>
@@ -343,6 +344,21 @@ static irqreturn_t mvme16x_abort_int (int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static u64 mvme16x_read_clk(struct clocksource *cs);
+
+static struct clocksource mvme16x_clk = {
+	.name   = "pcc",
+	.rating = 250,
+	.read   = mvme16x_read_clk,
+	.mask   = CLOCKSOURCE_MASK(32),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+static u32 clk_total;
+
+#define PCC_TIMER_CLOCK_FREQ 1000000
+#define PCC_TIMER_CYCLES     (PCC_TIMER_CLOCK_FREQ / HZ)
+
 static irqreturn_t mvme16x_timer_int (int irq, void *dev_id)
 {
 	irq_handler_t timer_routine = dev_id;
@@ -350,6 +366,7 @@ static irqreturn_t mvme16x_timer_int (int irq, void *dev_id)
 
 	local_irq_save(flags);
 	*(volatile unsigned char *)0xfff4201b |= 8;
+	clk_total += PCC_TIMER_CYCLES;
 	timer_routine(0, NULL);
 	local_irq_restore(flags);
 
@@ -363,13 +380,15 @@ void mvme16x_sched_init (irq_handler_t timer_routine)
 
     /* Using PCCchip2 or MC2 chip tick timer 1 */
     *(volatile unsigned long *)0xfff42008 = 0;
-    *(volatile unsigned long *)0xfff42004 = 10000;	/* 10ms */
+    *(volatile unsigned long *)0xfff42004 = PCC_TIMER_CYCLES;
     *(volatile unsigned char *)0xfff42017 |= 3;
     *(volatile unsigned char *)0xfff4201b = 0x16;
-    if (request_irq(MVME16x_IRQ_TIMER, mvme16x_timer_int, 0,
-                    "timer", timer_routine))
+    if (request_irq(MVME16x_IRQ_TIMER, mvme16x_timer_int, IRQF_TIMER, "timer",
+                    timer_routine))
 	panic ("Couldn't register timer int");
 
+    clocksource_register_hz(&mvme16x_clk, PCC_TIMER_CLOCK_FREQ);
+
     if (brdno == 0x0162 || brdno == 0x172)
 	irq = MVME162_IRQ_ABORT;
     else
@@ -379,11 +398,17 @@ void mvme16x_sched_init (irq_handler_t timer_routine)
 	panic ("Couldn't register abort int");
 }
 
-
-/* This is always executed with interrupts disabled.  */
-u32 mvme16x_gettimeoffset(void)
+static u64 mvme16x_read_clk(struct clocksource *cs)
 {
-    return (*(volatile u32 *)0xfff42008) * 1000;
+	unsigned long flags;
+	u32 ticks;
+
+	local_irq_save(flags);
+	ticks = *(volatile u32 *)0xfff42008;
+	ticks += clk_total;
+	local_irq_restore(flags);
+
+	return ticks;
 }
 
 int bcd2int (unsigned char b)
-- 
2.18.1

[RFC PATCH v2 02/14] m68k: mac: Fix VIA timer counter accesses

[Finn Thain]

This resolves some bugs that affect VIA timer counter accesses.
Avoid lost interrupts caused by reading the counter low byte register.
Make allowance for the fact that the counter will be decremented to
0xFFFF before being reloaded.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
Changed since v1:
 - Test the timer interrupt flag unconditionally.
 - Drop some extraneous clean up.
 - Don't try to recover from lost timer interrupts. Don't lose them
in the first place. That means giving up on the timer counter low byte.
The extra precision is probably not worth the extra complexity and
I couldn't make it work anyway.
---
 arch/m68k/mac/via.c | 105 +++++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 49 deletions(-)

diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
index 2ab85b6eb4fe..d1dbf9017300 100644
--- a/arch/m68k/mac/via.c
+++ b/arch/m68k/mac/via.c
@@ -54,16 +54,6 @@ static __u8 rbv_clear;
 
 static int gIER,gIFR,gBufA,gBufB;
 
-/*
- * Timer defs.
- */
-
-#define TICK_SIZE		10000
-#define MAC_CLOCK_TICK		(783300/HZ)		/* ticks per HZ */
-#define MAC_CLOCK_LOW		(MAC_CLOCK_TICK&0xFF)
-#define MAC_CLOCK_HIGH		(MAC_CLOCK_TICK>>8)
-
-
 /*
  * On Macs with a genuine VIA chip there is no way to mask an individual slot
  * interrupt. This limitation also seems to apply to VIA clone logic cores in
@@ -267,22 +257,6 @@ void __init via_init(void)
 	}
 }
 
-/*
- * Start the 100 Hz clock
- */
-
-void __init via_init_clock(irq_handler_t func)
-{
-	via1[vACR] |= 0x40;
-	via1[vT1LL] = MAC_CLOCK_LOW;
-	via1[vT1LH] = MAC_CLOCK_HIGH;
-	via1[vT1CL] = MAC_CLOCK_LOW;
-	via1[vT1CH] = MAC_CLOCK_HIGH;
-
-	if (request_irq(IRQ_MAC_TIMER_1, func, 0, "timer", func))
-		pr_err("Couldn't register %s interrupt\n", "timer");
-}
-
 /*
  * Debugging dump, used in various places to see what's going on.
  */
@@ -310,29 +284,6 @@ void via_debug_dump(void)
 	}
 }
 
-/*
- * This is always executed with interrupts disabled.
- *
- * TBI: get time offset between scheduling timer ticks
- */
-
-u32 mac_gettimeoffset(void)
-{
-	unsigned long ticks, offset = 0;
-
-	/* read VIA1 timer 2 current value */
-	ticks = via1[vT1CL] | (via1[vT1CH] << 8);
-	/* The probability of underflow is less than 2% */
-	if (ticks > MAC_CLOCK_TICK - MAC_CLOCK_TICK / 50)
-		/* Check for pending timer interrupt in VIA1 IFR */
-		if (via1[vIFR] & 0x40) offset = TICK_SIZE;
-
-	ticks = MAC_CLOCK_TICK - ticks;
-	ticks = ticks * 10000L / MAC_CLOCK_TICK;
-
-	return (ticks + offset) * 1000;
-}
-
 /*
  * Flush the L2 cache on Macs that have it by flipping
  * the system into 24-bit mode for an instant.
@@ -618,3 +569,59 @@ int via2_scsi_drq_pending(void)
 	return via2[gIFR] & (1 << IRQ_IDX(IRQ_MAC_SCSIDRQ));
 }
 EXPORT_SYMBOL(via2_scsi_drq_pending);
+
+/* timer and clock source */
+
+#define VIA_CLOCK_FREQ     783360                /* VIA "phase 2" clock in Hz */
+#define VIA_TIMER_INTERVAL (1000000 / HZ)        /* microseconds per jiffy */
+#define VIA_TIMER_CYCLES   (VIA_CLOCK_FREQ / HZ) /* clock cycles per jiffy */
+
+#define VIA_TC             (VIA_TIMER_CYCLES - 2) /* including 0 and -1 */
+#define VIA_TC_LOW         (VIA_TC & 0xFF)
+#define VIA_TC_HIGH        (VIA_TC >> 8)
+
+void __init via_init_clock(irq_handler_t timer_routine)
+{
+	if (request_irq(IRQ_MAC_TIMER_1, timer_routine, 0, "timer", NULL)) {
+		pr_err("Couldn't register %s interrupt\n", "timer");
+		return;
+	}
+
+	via1[vT1LL] = VIA_TC_LOW;
+	via1[vT1LH] = VIA_TC_HIGH;
+	via1[vT1CL] = VIA_TC_LOW;
+	via1[vT1CH] = VIA_TC_HIGH;
+	via1[vACR] |= 0x40;
+}
+
+u32 mac_gettimeoffset(void)
+{
+	unsigned long flags;
+	u8 count_high;
+	u16 count, offset = 0;
+
+	/*
+	 * Timer counter wrap-around is detected with the timer interrupt flag
+	 * but reading the counter low byte (vT1CL) would reset the flag.
+	 * Also, accessing both counter registers is essentially a data race.
+	 * These problems are avoided by ignoring the low byte. Clock accuracy
+	 * is 256 times worse (error can reach 0.327 ms) but CPU overhead is
+	 * reduced by avoiding slow VIA register accesses.
+	 */
+
+	local_irq_save(flags);
+	count_high = via1[vT1CH];
+	if (count_high == 0xFF) {
+		count_high = 0;
+		while (via1[vT1CH] == 0xFF)
+			/* spin */;
+	}
+	if (via1[vIFR] & VIA_TIMER_1_INT)
+		offset = VIA_TIMER_CYCLES;
+	local_irq_restore(flags);
+
+	count = count_high << 8;
+	count = VIA_TIMER_CYCLES - count + offset;
+
+	return ((count * VIA_TIMER_INTERVAL) / VIA_TIMER_CYCLES) * 1000;
+}
-- 
2.18.1

[RFC PATCH v2 06/14] m68k: amiga: Convert to clocksource API

[Finn Thain]

Add a platform clocksource by adapting the existing arch_gettimeoffset
implementation.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
Changed since v1:
 - Moved clk_total access to within the irq lock.
---
 arch/m68k/amiga/config.c | 43 ++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/m68k/amiga/config.c b/arch/m68k/amiga/config.c
index d4976c1aa0cc..c498f8419c87 100644
--- a/arch/m68k/amiga/config.c
+++ b/arch/m68k/amiga/config.c
@@ -17,6 +17,7 @@
 #include <linux/mm.h>
 #include <linux/seq_file.h>
 #include <linux/tty.h>
+#include <linux/clocksource.h>
 #include <linux/console.h>
 #include <linux/rtc.h>
 #include <linux/init.h>
@@ -461,7 +462,28 @@ void __init config_amiga(void)
 		*(unsigned char *)ZTWO_VADDR(0xde0002) |= 0x80;
 }
 
+static u64 amiga_read_clk(struct clocksource *cs);
+
+static struct clocksource amiga_clk = {
+	.name   = "ciab",
+	.rating = 250,
+	.read   = amiga_read_clk,
+	.mask   = CLOCKSOURCE_MASK(32),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
 static unsigned short jiffy_ticks;
+static u32 clk_total;
+
+static irqreturn_t ciab_timer_handler(int irq, void *dev_id)
+{
+	irq_handler_t timer_routine = dev_id;
+
+	clk_total += jiffy_ticks;
+	timer_routine(0, NULL);
+
+	return IRQ_HANDLED;
+}
 
 static void __init amiga_sched_init(irq_handler_t timer_routine)
 {
@@ -481,20 +503,23 @@ static void __init amiga_sched_init(irq_handler_t timer_routine)
 	 * Please don't change this to use ciaa, as it interferes with the
 	 * SCSI code. We'll have to take a look at this later
 	 */
-	if (request_irq(IRQ_AMIGA_CIAB_TA, timer_routine, 0, "timer", NULL))
+	if (request_irq(IRQ_AMIGA_CIAB_TA, ciab_timer_handler, IRQF_TIMER,
+			"timer", timer_routine))
 		pr_err("Couldn't register timer interrupt\n");
 	/* start timer */
 	ciab.cra |= 0x11;
-}
 
-#define TICK_SIZE 10000
+	clocksource_register_hz(&amiga_clk, amiga_eclock);
+}
 
-/* This is always executed with interrupts disabled.  */
-static u32 amiga_gettimeoffset(void)
+static u64 amiga_read_clk(struct clocksource *cs)
 {
+	unsigned long flags;
 	unsigned short hi, lo, hi2;
 	u32 ticks, offset = 0;
 
+	local_irq_save(flags);
+
 	/* read CIA B timer A current value */
 	hi  = ciab.tahi;
 	lo  = ciab.talo;
@@ -510,12 +535,14 @@ static u32 amiga_gettimeoffset(void)
 	if (ticks > jiffy_ticks / 2)
 		/* check for pending interrupt */
 		if (cia_set_irq(&ciab_base, 0) & CIA_ICR_TA)
-			offset = 10000;
+			offset = jiffy_ticks;
 
 	ticks = jiffy_ticks - ticks;
-	ticks = (10000 * ticks) / jiffy_ticks;
+	ticks += offset + clk_total;
+
+	local_irq_restore(flags);
 
-	return (ticks + offset) * 1000;
+	return ticks;
 }
 
 static void amiga_reset(void)  __noreturn;
-- 
2.18.1

Re: [RFC PATCH v2 06/14] m68k: amiga: Convert to clocksource API

[Linus Walleij]

On Mon, Nov 19, 2018 at 2:22 AM Finn Thain <fthain@telegraphics.com.au> wrote:

> Add a platform clocksource by adapting the existing arch_gettimeoffset
> implementation.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Changed since v1:
>  - Moved clk_total access to within the irq lock.

Came to think of it, Geert can probably answer to the use cases
for the CIAs in Linux: the Amiga CIA has two counters.

It would make sense to use one as a free-runing clocksource and
the other one as clock event. Then Linux is extremely happy
without any complex workarounds trying to use just one timer
for both jobs.

Is there some specific reason why we can't use both counters
like this, except for legacy? (I am thinking it would be an improvement
on top of Finn's series once they go in.)

Yours,
Linus Walleij

[RFC PATCH v2 05/14] m68k: Drop ARCH_USES_GETTIMEOFFSET

[Finn Thain]

The functions that implement arch_gettimeoffset are re-used by
new clocksource drivers in subsequent patches.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/m68k/Kconfig           | 1 -
 arch/m68k/amiga/config.c    | 3 ---
 arch/m68k/atari/config.c    | 2 --
 arch/m68k/bvme6000/config.c | 2 --
 arch/m68k/hp300/config.c    | 1 -
 arch/m68k/hp300/time.h      | 1 -
 arch/m68k/mac/config.c      | 3 ---
 arch/m68k/mvme147/config.c  | 2 --
 arch/m68k/mvme16x/config.c  | 2 --
 9 files changed, 17 deletions(-)

diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
index 070553791e97..cc62660a5760 100644
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -19,7 +19,6 @@ config M68K
 	select GENERIC_STRNCPY_FROM_USER if MMU
 	select GENERIC_STRNLEN_USER if MMU
 	select ARCH_WANT_IPC_PARSE_VERSION
-	select ARCH_USES_GETTIMEOFFSET if MMU && !COLDFIRE
 	select HAVE_FUTEX_CMPXCHG if MMU && FUTEX
 	select HAVE_MOD_ARCH_SPECIFIC
 	select MODULES_USE_ELF_REL
diff --git a/arch/m68k/amiga/config.c b/arch/m68k/amiga/config.c
index 65f63a457130..d4976c1aa0cc 100644
--- a/arch/m68k/amiga/config.c
+++ b/arch/m68k/amiga/config.c
@@ -95,8 +95,6 @@ static char amiga_model_name[13] = "Amiga ";
 static void amiga_sched_init(irq_handler_t handler);
 static void amiga_get_model(char *model);
 static void amiga_get_hardware_list(struct seq_file *m);
-/* amiga specific timer functions */
-static u32 amiga_gettimeoffset(void);
 extern void amiga_mksound(unsigned int count, unsigned int ticks);
 static void amiga_reset(void);
 extern void amiga_init_sound(void);
@@ -386,7 +384,6 @@ void __init config_amiga(void)
 	mach_init_IRQ        = amiga_init_IRQ;
 	mach_get_model       = amiga_get_model;
 	mach_get_hardware_list = amiga_get_hardware_list;
-	arch_gettimeoffset   = amiga_gettimeoffset;
 
 	/*
 	 * default MAX_DMA=0xffffffff on all machines. If we don't do so, the SCSI
diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c
index bd96702a1ad0..89e65be2655f 100644
--- a/arch/m68k/atari/config.c
+++ b/arch/m68k/atari/config.c
@@ -78,7 +78,6 @@ static void atari_heartbeat(int on);
 
 /* atari specific timer functions (in time.c) */
 extern void atari_sched_init(irq_handler_t);
-extern u32 atari_gettimeoffset(void);
 extern int atari_mste_hwclk (int, struct rtc_time *);
 extern int atari_tt_hwclk (int, struct rtc_time *);
 
@@ -205,7 +204,6 @@ void __init config_atari(void)
 	mach_init_IRQ        = atari_init_IRQ;
 	mach_get_model	 = atari_get_model;
 	mach_get_hardware_list = atari_get_hardware_list;
-	arch_gettimeoffset   = atari_gettimeoffset;
 	mach_reset           = atari_reset;
 	mach_max_dma_address = 0xffffff;
 #if IS_ENABLED(CONFIG_INPUT_M68K_BEEP)
diff --git a/arch/m68k/bvme6000/config.c b/arch/m68k/bvme6000/config.c
index d1de3cb1f8fe..c27c104ac7e7 100644
--- a/arch/m68k/bvme6000/config.c
+++ b/arch/m68k/bvme6000/config.c
@@ -39,7 +39,6 @@
 
 static void bvme6000_get_model(char *model);
 extern void bvme6000_sched_init(irq_handler_t handler);
-extern u32 bvme6000_gettimeoffset(void);
 extern int bvme6000_hwclk (int, struct rtc_time *);
 extern void bvme6000_reset (void);
 void bvme6000_set_vectors (void);
@@ -105,7 +104,6 @@ void __init config_bvme6000(void)
     mach_max_dma_address = 0xffffffff;
     mach_sched_init      = bvme6000_sched_init;
     mach_init_IRQ        = bvme6000_init_IRQ;
-    arch_gettimeoffset   = bvme6000_gettimeoffset;
     mach_hwclk           = bvme6000_hwclk;
     mach_reset		 = bvme6000_reset;
     mach_get_model       = bvme6000_get_model;
diff --git a/arch/m68k/hp300/config.c b/arch/m68k/hp300/config.c
index a19bcd23f80b..a161d44fd20b 100644
--- a/arch/m68k/hp300/config.c
+++ b/arch/m68k/hp300/config.c
@@ -254,7 +254,6 @@ void __init config_hp300(void)
 	mach_sched_init      = hp300_sched_init;
 	mach_init_IRQ        = hp300_init_IRQ;
 	mach_get_model       = hp300_get_model;
-	arch_gettimeoffset   = hp300_gettimeoffset;
 	mach_hwclk	     = hp300_hwclk;
 	mach_get_ss	     = hp300_get_ss;
 	mach_reset           = hp300_reset;
diff --git a/arch/m68k/hp300/time.h b/arch/m68k/hp300/time.h
index f5583ec4033d..1d77b55cc72a 100644
--- a/arch/m68k/hp300/time.h
+++ b/arch/m68k/hp300/time.h
@@ -1,2 +1 @@
 extern void hp300_sched_init(irq_handler_t vector);
-extern u32 hp300_gettimeoffset(void);
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index cd9317d53276..11be08f4f750 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -54,8 +54,6 @@ struct mac_booter_data mac_bi_data;
 /* The phys. video addr. - might be bogus on some machines */
 static unsigned long mac_orig_videoaddr;
 
-/* Mac specific timer functions */
-extern u32 mac_gettimeoffset(void);
 extern int mac_hwclk(int, struct rtc_time *);
 extern void iop_preinit(void);
 extern void iop_init(void);
@@ -155,7 +153,6 @@ void __init config_mac(void)
 	mach_sched_init = mac_sched_init;
 	mach_init_IRQ = mac_init_IRQ;
 	mach_get_model = mac_get_model;
-	arch_gettimeoffset = mac_gettimeoffset;
 	mach_hwclk = mac_hwclk;
 	mach_reset = mac_reset;
 	mach_halt = mac_poweroff;
diff --git a/arch/m68k/mvme147/config.c b/arch/m68k/mvme147/config.c
index 93c68d2b8e0e..4ef4faa5ed8b 100644
--- a/arch/m68k/mvme147/config.c
+++ b/arch/m68k/mvme147/config.c
@@ -38,7 +38,6 @@
 
 static void mvme147_get_model(char *model);
 extern void mvme147_sched_init(irq_handler_t handler);
-extern u32 mvme147_gettimeoffset(void);
 extern int mvme147_hwclk (int, struct rtc_time *);
 extern void mvme147_reset (void);
 
@@ -84,7 +83,6 @@ void __init config_mvme147(void)
 	mach_max_dma_address	= 0x01000000;
 	mach_sched_init		= mvme147_sched_init;
 	mach_init_IRQ		= mvme147_init_IRQ;
-	arch_gettimeoffset	= mvme147_gettimeoffset;
 	mach_hwclk		= mvme147_hwclk;
 	mach_reset		= mvme147_reset;
 	mach_get_model		= mvme147_get_model;
diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c
index 4e6229856396..8bafa6a37593 100644
--- a/arch/m68k/mvme16x/config.c
+++ b/arch/m68k/mvme16x/config.c
@@ -44,7 +44,6 @@ static MK48T08ptr_t volatile rtc = (MK48T08ptr_t)MVME_RTC_BASE;
 
 static void mvme16x_get_model(char *model);
 extern void mvme16x_sched_init(irq_handler_t handler);
-extern u32 mvme16x_gettimeoffset(void);
 extern int mvme16x_hwclk (int, struct rtc_time *);
 extern void mvme16x_reset (void);
 
@@ -272,7 +271,6 @@ void __init config_mvme16x(void)
     mach_max_dma_address = 0xffffffff;
     mach_sched_init      = mvme16x_sched_init;
     mach_init_IRQ        = mvme16x_init_IRQ;
-    arch_gettimeoffset   = mvme16x_gettimeoffset;
     mach_hwclk           = mvme16x_hwclk;
     mach_reset		 = mvme16x_reset;
     mach_get_model       = mvme16x_get_model;
-- 
2.18.1

[RFC PATCH v2 14/14] m68k: mvme16x: Handle timer counter overflow

[Finn Thain]

Reading the timer counter races with timer overflow (and the
corresponding interrupt). This is resolved by reading the overflow
register and taking this value into account. The interrupt handler
must clear the overflow register when it eventually executes.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
TODO: find a spare counter for the clocksource, rather than hanging
it off the HZ timer.
---
 arch/m68k/mvme16x/config.c | 45 +++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/arch/m68k/mvme16x/config.c b/arch/m68k/mvme16x/config.c
index 2c109ee2a1a5..9bc2da69f80c 100644
--- a/arch/m68k/mvme16x/config.c
+++ b/arch/m68k/mvme16x/config.c
@@ -115,11 +115,11 @@ static void __init mvme16x_init_IRQ (void)
 	m68k_setup_user_interrupt(VEC_USER, 192);
 }
 
-#define pcc2chip	((volatile u_char *)0xfff42000)
-#define PccSCCMICR	0x1d
-#define PccSCCTICR	0x1e
-#define PccSCCRICR	0x1f
-#define PccTPIACKR	0x25
+#define PCC2CHIP   (0xfff42000)
+#define PCCSCCMICR (PCC2CHIP + 0x1d)
+#define PCCSCCTICR (PCC2CHIP + 0x1e)
+#define PCCSCCRICR (PCC2CHIP + 0x1f)
+#define PCCTPIACKR (PCC2CHIP + 0x25)
 
 #ifdef CONFIG_EARLY_PRINTK
 
@@ -227,10 +227,10 @@ void mvme16x_cons_write(struct console *co, const char *str, unsigned count)
 	base_addr[CyIER] = CyTxMpty;
 
 	while (1) {
-		if (pcc2chip[PccSCCTICR] & 0x20)
+		if (in_8(PCCSCCTICR) & 0x20)
 		{
 			/* We have a Tx int. Acknowledge it */
-			sink = pcc2chip[PccTPIACKR];
+			sink = in_8(PCCTPIACKR);
 			if ((base_addr[CyLICR] >> 2) == port) {
 				if (i == count) {
 					/* Last char of string is now output */
@@ -359,13 +359,26 @@ static u32 clk_total;
 #define PCC_TIMER_CLOCK_FREQ 1000000
 #define PCC_TIMER_CYCLES     (PCC_TIMER_CLOCK_FREQ / HZ)
 
+#define PCCTCMP1             (PCC2CHIP + 0x04)
+#define PCCTCNT1             (PCC2CHIP + 0x08)
+#define PCCTOVR1             (PCC2CHIP + 0x17)
+#define PCCTIC1              (PCC2CHIP + 0x1b)
+
+#define PCCTOVR1_TIC_EN      0x01
+#define PCCTOVR1_COC_EN      0x02
+#define PCCTOVR1_OVR_CLR     0x04
+
+#define PCCTIC1_INT_CLR      0x08
+#define PCCTIC1_INT_EN       0x10
+
 static irqreturn_t mvme16x_timer_int (int irq, void *dev_id)
 {
 	irq_handler_t timer_routine = dev_id;
 	unsigned long flags;
 
 	local_irq_save(flags);
-	*(volatile unsigned char *)0xfff4201b |= 8;
+	out_8(PCCTIC1, in_8(PCCTIC1) | PCCTIC1_INT_CLR);
+	out_8(PCCTOVR1, PCCTOVR1_OVR_CLR);
 	clk_total += PCC_TIMER_CYCLES;
 	timer_routine(0, NULL);
 	local_irq_restore(flags);
@@ -379,10 +392,10 @@ void mvme16x_sched_init (irq_handler_t timer_routine)
     int irq;
 
     /* Using PCCchip2 or MC2 chip tick timer 1 */
-    *(volatile unsigned long *)0xfff42008 = 0;
-    *(volatile unsigned long *)0xfff42004 = PCC_TIMER_CYCLES;
-    *(volatile unsigned char *)0xfff42017 |= 3;
-    *(volatile unsigned char *)0xfff4201b = 0x16;
+    out_be32(PCCTCNT1, 0);
+    out_be32(PCCTCMP1, PCC_TIMER_CYCLES);
+    out_8(PCCTOVR1, in_8(PCCTOVR1) | PCCTOVR1_TIC_EN | PCCTOVR1_COC_EN);
+    out_8(PCCTIC1, PCCTIC1_INT_EN | 6);
     if (request_irq(MVME16x_IRQ_TIMER, mvme16x_timer_int, IRQF_TIMER, "timer",
                     timer_routine))
 	panic ("Couldn't register timer int");
@@ -401,10 +414,16 @@ void mvme16x_sched_init (irq_handler_t timer_routine)
 static u64 mvme16x_read_clk(struct clocksource *cs)
 {
 	unsigned long flags;
+	u8 overflow, tmp;
 	u32 ticks;
 
 	local_irq_save(flags);
-	ticks = *(volatile u32 *)0xfff42008;
+	tmp = in_8(PCCTOVR1) >> 4;
+	ticks = in_be32(PCCTCNT1);
+	overflow = in_8(PCCTOVR1) >> 4;
+	if (overflow != tmp)
+		ticks = in_be32(PCCTCNT1);
+	ticks += overflow * PCC_TIMER_CYCLES;
 	ticks += clk_total;
 	local_irq_restore(flags);
 
-- 
2.18.1