Move virt machine to generic clockevents.
cc: Arnd Bergmann <[email protected]>
Signed-off-by: Laurent Vivier <[email protected]>
---
arch/m68k/Kconfig.machine | 2 +-
arch/m68k/virt/timer.c | 56 ++++++++++++++++++++++++++++++---------
2 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/arch/m68k/Kconfig.machine b/arch/m68k/Kconfig.machine
index 769c5b38fe16..03e5254ed73a 100644
--- a/arch/m68k/Kconfig.machine
+++ b/arch/m68k/Kconfig.machine
@@ -152,7 +152,7 @@ config SUN3
config VIRT
bool "Virtual M68k Machine support"
depends on MMU
- select LEGACY_TIMER_TICK
+ select GENERIC_CLOCKEVENTS
select M68040
select MMU_MOTOROLA if MMU
select GOLDFISH
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
@@ -3,6 +3,7 @@
#include <linux/interrupt.h>
#include <linux/ioport.h>
#include <linux/clocksource.h>
+#include <linux/clockchips.h>
#include <asm/virt.h>
struct goldfish_timer {
@@ -41,7 +42,25 @@ static struct clocksource goldfish_timer = {
.max_idle_ns = LONG_MAX,
};
-static irqreturn_t golfish_timer_handler(int irq, void *dev_id)
+static int goldfish_timer_set_oneshot(struct clock_event_device *evt)
+{
+ gf_timer->alarm_high = 0;
+ gf_timer->alarm_low = 0;
+
+ gf_timer->irq_enabled = 1;
+
+ return 0;
+}
+
+static int goldfish_timer_shutdown(struct clock_event_device *evt)
+{
+ gf_timer->irq_enabled = 0;
+
+ return 0;
+}
+
+static int goldfish_timer_next_event(unsigned long delta,
+ struct clock_event_device *evt)
{
u64 now;
@@ -49,19 +68,35 @@ static irqreturn_t golfish_timer_handler(int irq, void *dev_id)
now = goldfish_timer_read(NULL);
- legacy_timer_tick(1);
+ now += delta;
- now += NSEC_PER_SEC / HZ;
gf_timer->alarm_high = upper_32_bits(now);
gf_timer->alarm_low = lower_32_bits(now);
+ return 0;
+}
+
+struct clock_event_device goldfish_timer_clockevent = {
+ .name = "goldfish_timer",
+ .features = CLOCK_EVT_FEAT_ONESHOT,
+ .set_state_shutdown = goldfish_timer_shutdown,
+ .set_state_oneshot = goldfish_timer_set_oneshot,
+ .set_next_event = goldfish_timer_next_event,
+ .shift = 32,
+};
+
+static irqreturn_t golfish_timer_tick(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = &goldfish_timer_clockevent;
+
+ evt->event_handler(evt);
+
return IRQ_HANDLED;
}
void __init virt_sched_init(void)
{
static struct resource sched_res;
- u64 now;
sched_res.name = "goldfish_timer";
sched_res.start = virt_bi_data.rtc.mmio;
@@ -72,19 +107,14 @@ void __init virt_sched_init(void)
return;
}
- if (request_irq(virt_bi_data.rtc.irq, golfish_timer_handler, IRQF_TIMER,
+ clockevents_config_and_register(&goldfish_timer_clockevent, NSEC_PER_SEC,
+ 1, 0xffffffff);
+
+ if (request_irq(virt_bi_data.rtc.irq, golfish_timer_tick, IRQF_TIMER,
"timer", NULL)) {
pr_err("Couldn't register timer interrupt\n");
return;
}
- now = goldfish_timer_read(NULL);
- now += NSEC_PER_SEC / HZ;
-
- gf_timer->clear_interrupt = 1;
- gf_timer->alarm_high = upper_32_bits(now);
- gf_timer->alarm_low = lower_32_bits(now);
- gf_timer->irq_enabled = 1;
-
clocksource_register_hz(&goldfish_timer, NSEC_PER_SEC);
}
--
2.34.1
On Thu, Jan 13, 2022 at 11:35 AM Laurent Vivier <[email protected]> wrote:
>
> Move virt machine to generic clockevents.
>
> cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Laurent Vivier <[email protected]>
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.
> 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.
Arnd
Le 13/01/2022 à 12:20, Arnd Bergmann a écrit :
> On Thu, Jan 13, 2022 at 11:35 AM Laurent Vivier <[email protected]> wrote:
>>
>> Move virt machine to generic clockevents.
>>
>> cc: Arnd Bergmann <[email protected]>
>> Signed-off-by: Laurent Vivier <[email protected]>
>
> 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.
>> 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.
Thanks,
Laurent
On Thu, Jan 13, 2022 at 12:32 PM Laurent Vivier <[email protected]> wrote:
> Le 13/01/2022 à 12:20, Arnd Bergmann a écrit :
> > On Thu, Jan 13, 2022 at 11:35 AM Laurent Vivier <[email protected]> wrote:
> >>
> >> Move virt machine to generic clockevents.
> >>
> >> cc: Arnd Bergmann <[email protected]>
> >> Signed-off-by: Laurent Vivier <[email protected]>
> >
> > 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
Le 13/01/2022 à 12:42, Arnd Bergmann a écrit :
> On Thu, Jan 13, 2022 at 12:32 PM Laurent Vivier <[email protected]> wrote:
>> Le 13/01/2022 à 12:20, Arnd Bergmann a écrit :
>>> On Thu, Jan 13, 2022 at 11:35 AM Laurent Vivier <[email protected]> wrote:
>>>>
>>>> Move virt machine to generic clockevents.
>>>>
>>>> cc: Arnd Bergmann <[email protected]>
>>>> Signed-off-by: Laurent Vivier <[email protected]>
>>>
>>> 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.
>
OK, I move my code to drivers/clocksource/timer-goldfish.c and send a new version of the series.
Thanks,
Laurent