Messages in this thread | | | Date | Wed, 6 Apr 2022 13:26:33 +0200 | Subject | Re: [PATCH v15 4/5] clocksource/drivers: Add a goldfish-timer clocksource | From | Laurent Vivier <> |
| |
Le 06/04/2022 à 12:27, Daniel Lezcano a écrit : > > Hi Laurent, > > > On 01/04/2022 12:20, Laurent Vivier wrote: >> >> Daniel ? >> >> Thanks, >> Laurent >> >> Le 10/03/2022 à 10:00, Laurent Vivier a écrit : >>> Add a clocksource based on the goldfish-rtc device. >>> >>> Move the timer register definition to <clocksource/timer-goldfish.h> >>> >>> This kernel implementation is based on the QEMU upstream implementation: >>> >>> https://git.qemu.org/?p=qemu.git;a=blob_plain;f=hw/rtc/goldfish_rtc.c >>> >>> goldfish-timer is a high-precision signed 64-bit nanosecond timer. >>> It is part of the 'goldfish' virtual hardware platform used to run >>> some emulated Android systems under QEMU. >>> This timer only supports oneshot event. >>> >>> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >>> --- >>> drivers/clocksource/Kconfig | 7 ++ >>> drivers/clocksource/Makefile | 1 + >>> drivers/clocksource/timer-goldfish.c | 153 +++++++++++++++++++++++++++ >>> drivers/rtc/rtc-goldfish.c | 13 +-- >>> include/clocksource/timer-goldfish.h | 31 ++++++ >>> 5 files changed, 193 insertions(+), 12 deletions(-) >>> create mode 100644 drivers/clocksource/timer-goldfish.c >>> create mode 100644 include/clocksource/timer-goldfish.h >>> >>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig >>> index cfb8ea0df3b1..94f00374cebb 100644 >>> --- a/drivers/clocksource/Kconfig >>> +++ b/drivers/clocksource/Kconfig >>> @@ -721,4 +721,11 @@ config MICROCHIP_PIT64B >>> modes and high resolution. It is used as a clocksource >>> and a clockevent. >>> +config GOLDFISH_TIMER >>> + bool "Clocksource using goldfish-rtc" >>> + depends on M68K || COMPILE_TEST >>> + depends on RTC_DRV_GOLDFISH >>> + help >>> + Support for the timer/counter of goldfish-rtc >>> + >>> endmenu >>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >>> index fa5f624eadb6..12f5d7e8cc2d 100644 >>> --- a/drivers/clocksource/Makefile >>> +++ b/drivers/clocksource/Makefile >>> @@ -89,3 +89,4 @@ obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o >>> obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o >>> obj-$(CONFIG_MICROCHIP_PIT64B) += timer-microchip-pit64b.o >>> obj-$(CONFIG_MSC313E_TIMER) += timer-msc313e.o >>> +obj-$(CONFIG_GOLDFISH_TIMER) += timer-goldfish.o >>> diff --git a/drivers/clocksource/timer-goldfish.c b/drivers/clocksource/timer-goldfish.c >>> new file mode 100644 >>> index 000000000000..0512d5eabc82 >>> --- /dev/null >>> +++ b/drivers/clocksource/timer-goldfish.c >>> @@ -0,0 +1,153 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> + >>> +#include <linux/interrupt.h> >>> +#include <linux/ioport.h> >>> +#include <linux/clocksource.h> >>> +#include <linux/clockchips.h> >>> +#include <linux/module.h> >>> +#include <linux/slab.h> >>> +#include <linux/goldfish.h> > > Not related to this patch: would it make sense to move it to linux/soc/... ?
I don't know. goldfish is not really a soc. I didn't really find a good place where it should be.
> > >>> +#include <clocksource/timer-goldfish.h> >>> + >>> +struct goldfish_timer { >>> + struct clocksource cs; >>> + struct clock_event_device ced; >>> + struct resource res; >>> + void __iomem *base; >>> +}; >>> + >>> +static struct goldfish_timer *ced_to_gf(struct clock_event_device *ced) >>> +{ >>> + return container_of(ced, struct goldfish_timer, ced); >>> +} >>> + >>> +static struct goldfish_timer *cs_to_gf(struct clocksource *cs) >>> +{ >>> + return container_of(cs, struct goldfish_timer, cs); >>> +} >>> + >>> +static u64 goldfish_timer_read(struct clocksource *cs) >>> +{ >>> + struct goldfish_timer *timerdrv = cs_to_gf(cs); >>> + void __iomem *base = timerdrv->base; >>> + u32 time_low, time_high; >>> + u64 ticks; >>> + >>> + /* >>> + * time_low: get low bits of current time and update time_high >>> + * time_high: get high bits of time at last time_low read >>> + */ >>> + time_low = gf_ioread32(base + TIMER_TIME_LOW); >>> + time_high = gf_ioread32(base + TIMER_TIME_HIGH); > > There is a risk here to have the counter rolling over between low and high reading, no ?
No, because value of high is frozen when we read low, so the 64bit value doesn't move between the two ioread32().
Thanks, Laurent
| |