On Sat, Nov 10, 2018 at 10:47 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
On Fri, Nov 9, 2018 at 12:42 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Sun, 28 Oct 2018, Geert Uytterhoeven wrote:
The example I gave was GENERIC_CLOCKEVENTS on m68, which is
supported on most but not all machines there.
That suggests that the removal of just those machines would suffice
(as opposed to the removal of the entire arch).
Also, Documentation/features/time/clockevents/arch-support.txt says,
| m68k: | ok |
These two observations make me wonder whether the clockevents feature
is related to the discussion quoted above (?)
GENERIC_CLOCKEVENTS is selected only for a few Coldfire CPU types.
!GENERIC_CLOCKEVENTS implies PREEMPT_NONE, and disables the "Timers
subsystem" (i.e. the NO_HZ config options), but it works fine -- I was
able to convert the Mac port to !ARCH_USES_GETTIMEOFFSET &&
!GENERIC_CLOCKEVENTS. (Like many of the Coldfire CPU types.)
You can see those patches here,
https://github.com/fthain/linux/commits/mac68k-queue/
This is looking very good. FWIW:
Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Apart from this (which is the most important step!) I think the custom
LED heartbeat code in kernel/time.c needs to be replaced with a standard
drivers/leds driver for each LED using the "heartbeat" trigger as is custom
these days.
That should clean out another chunk of legacy time-related code.
I suppose you are currently keeping the call to timer_interrupt() for
exactly this reason (i.e. keep the heartbeat LED blinking)?
Note that !ARCH_USES_GETTIMEOFFSET is a build-time choice, meaning that
all platforms need to be converted together.
Also, some platforms will need to adopt the clocksource API, otherwise the
built-in "jiffies" clocksource will get used, causing a regression in
timer accuracy.
Conversion to the clocksource API is straight-forward where the platform
already implements arch_gettimeoffset. The conversion is discussed in
include/linux/time.h:
/* Some architectures do not supply their own clocksource.
* This is mainly the case in architectures that get their
* inter-tick times by reading the counter on their interval
* timer. Since these timers wrap every tick, they're not really
* useful as clocksources. Wrapping them to act like one is possible
* but not very efficient. So we provide a callout these arches
* can implement for use with the jiffies clocksource to provide
* finer then tick granular time.
*/
(Not sure what is meant by "not very efficient" here.)
Certain 680x0 platforms do not implement arch_gettimeoffset: apollo, q40,
sun3, sun3x. These platforms can fall back on the "jiffies" clocksource
with no loss of timer accuracy, so conversion for these platforms is
trivial.
Should I continue with the clocksource API conversion for the other
platforms: amiga, atari, bvme6000, hp300, mvme147, mvme16x? This would
allow for removal of "select ARCH_USES_GETTIMEOFFSET" (and satisfy
Documentation/features/time/modern-timekeeping) without loss of timer
accuracy.
If you ask me: forge ahead.
Alternatively, we could defer the clocksource API conversion, leaving it
up to the platform maintainers (who can actually test the driver changes).
But that would mean that many platforms would suffer a regression in timer
accuracy upon removal of arch_gettimeoffset.
Adding the timekeeping maintainers and Linus Walleij to Cc. Linus
worked on removing ARCH_USES_GETTIMEOFFSET on ARM
several platforms in the past.
I noticed that the last timer rework involving
CONFIG_ARCH_USES_GETTIMEOFFSET was back in 2012 with
commit 7b1f62076bba ("time: convert arch_gettimeoffset to a pointer").
At the time, we had cris, m32r, blackfin, m68k and lots of ARM
platforms, now it's only two StrongARM platforms (RPC and
ARCH_EBSA110) and classic m68k.
Believe it or not, I managed to procure both machines (RPC and
EBSA110). Whether I can get them to boot Linux is still an open
question.
I suppose it would be possible to use a conversion strategy
similar to Finn's and get rid of gettimeoffset altogether, if I could
only test it on these boards.
Yours.
Linus Walleij
