Re: [PATCH v5 3/3] m68k: virt: Remove LEGACY_TIMER_TICK

[Arnd Bergmann]

On Thu, Jan 13, 2022 at 12:32 PM Laurent Vivier <laurent@xxxxxxxxx> wrote:
> Le 13/01/2022 à 12:20, Arnd Bergmann a écrit :
> > On Thu, Jan 13, 2022 at 11:35 AM Laurent Vivier <laurent@xxxxxxxxx> wrote:
> >>
> >> Move virt machine to generic clockevents.
> >>
> >> cc: Arnd Bergmann <arnd@xxxxxxxx>
> >> Signed-off-by: Laurent Vivier <laurent@xxxxxxxxx>
> >
> > The change looks good, but it appears that you only just add the legacy code
> > in the same series, and it would be easier to just add the correct version
> > first.
>
> In fact, I'd like to keep it separated for two reasons:
> - it can be used as an example for people that want to move from legacy to clockevents,
> - the machine with legacy timer tick is in use for more than one year by debian to propose a m68k
> buildd and dev machine, so it is really well tested and robust. If there is a bug in my clockevents
> use it will be easier to detect.

In general, it should be easier to do a correct generic driver than
an implementation for the legacy interface.

> >> diff --git a/arch/m68k/virt/timer.c b/arch/m68k/virt/timer.c
> >> index 843bf6ed7e1a..767b01f75abb 100644
> >> --- a/arch/m68k/virt/timer.c
> >> +++ b/arch/m68k/virt/timer.c
> >
> > How about moving the entire file to drivers/clocksource/timer-goldfish.c?
> > It shouldn't even be architecture specific any more at this point. It probably
> > still is in practice, but that could be addressed when another architecture
> > wants to share the implementation.
>
> For the moment I'd like to have my m68k virt machine merged, and I think it will be easier if I hit
> only one subsystem/maintainer. Moreover I don't know if I use correctly the goldfish-rtc,  so for
> the moment I think it's better if I keep it hidden in arch/m68k/virt.
>
> But I can propose to send a patch to move this code to drivers/clocksource/timer-goldfish.c once the
> machine is merged.

If you are not sure about that implementation, I would think that's an
extra reason to
submit it to the clocksource maintainers for review (added to Cc
here). You should still
be able to merge the driver in the new location through the m68k tree
as part of your
series, but regardless of where it goes I think it needs an Ack from them.

        Arnd