diff mbox series

[v8,3/4] clocksource/drivers: Add a goldfish-timer clocksource

Message ID 20220115193245.3777833-4-laurent@vivier.eu
State Superseded
Headers show
Series m68k: Add Virtual M68k Machine | expand

Commit Message

Laurent Vivier Jan. 15, 2022, 7:32 p.m. UTC
Add a clocksource based on the goldfish-rtc device.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 drivers/clocksource/Kconfig          |   7 ++
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-goldfish.c | 139 +++++++++++++++++++++++++++
 include/clocksource/timer-goldfish.h |  11 +++
 4 files changed, 158 insertions(+)
 create mode 100644 drivers/clocksource/timer-goldfish.c
 create mode 100644 include/clocksource/timer-goldfish.h

Comments

Arnd Bergmann Jan. 16, 2022, 10:44 a.m. UTC | #1
On Sat, Jan 15, 2022 at 8:32 PM Laurent Vivier <laurent@vivier.eu> wrote:

> +
> +/* goldfish endianness depends on CPU endianness */
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> +#define goldfish_ioread32 ioread32be
> +#define goldfish_iowrite32 iowrite32be
> +#else
> +#define goldfish_ioread32 ioread32
> +#define goldfish_iowrite32 iowrite32
> +#endif

This is not what I meant here, as you are breaking big-endian support
for all other
architectures in the process.

On architectures that support both big-endian and little-endian kernels, devices
(including emulated ones) can't know which type of kernel you are running, so
this has to be fixed by architecture. Ideally this macro should be in
an architecture
specific header file, but you can also just make this a check for m68k and  hope
that qemu doesn't duplicate this bug on architectures that gain support for this
driver in the future.

         Arnd
Laurent Vivier Jan. 16, 2022, 10:55 a.m. UTC | #2
Le 16/01/2022 à 11:44, Arnd Bergmann a écrit :
> On Sat, Jan 15, 2022 at 8:32 PM Laurent Vivier <laurent@vivier.eu> wrote:
> 
>> +
>> +/* goldfish endianness depends on CPU endianness */
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +#define goldfish_ioread32 ioread32be
>> +#define goldfish_iowrite32 iowrite32be
>> +#else
>> +#define goldfish_ioread32 ioread32
>> +#define goldfish_iowrite32 iowrite32
>> +#endif
> 
> This is not what I meant here, as you are breaking big-endian support
> for all other
> architectures in the process.
> 
> On architectures that support both big-endian and little-endian kernels, devices
> (including emulated ones) can't know which type of kernel you are running, so
> this has to be fixed by architecture. Ideally this macro should be in
> an architecture
> specific header file, but you can also just make this a check for m68k and  hope
> that qemu doesn't duplicate this bug on architectures that gain support for this
> driver in the future.
> 

Is it OK if I replace "CONFIG_CPU_BIG_ENDIAN" by "CONFIG_M68K" as M68K is the only big-endian 
architecture using goldfish?

Thanks,
Laurent
Laurent Vivier Jan. 16, 2022, 11:54 a.m. UTC | #3
Le 16/01/2022 à 11:44, Arnd Bergmann a écrit :
> On Sat, Jan 15, 2022 at 8:32 PM Laurent Vivier <laurent@vivier.eu> wrote:
> 
>> +
>> +/* goldfish endianness depends on CPU endianness */
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +#define goldfish_ioread32 ioread32be
>> +#define goldfish_iowrite32 iowrite32be
>> +#else
>> +#define goldfish_ioread32 ioread32
>> +#define goldfish_iowrite32 iowrite32
>> +#endif
> 
> This is not what I meant here, as you are breaking big-endian support
> for all other
> architectures in the process.
> 
> On architectures that support both big-endian and little-endian kernels, devices
> (including emulated ones) can't know which type of kernel you are running, so
> this has to be fixed by architecture. Ideally this macro should be in
> an architecture
> specific header file, but you can also just make this a check for m68k and  hope
> that qemu doesn't duplicate this bug on architectures that gain support for this
> driver in the future.

Virtio has exactly the same problem in the past (prio to v1.0), and it uses __LITTLE_ENDIAN in 
virtio_legacy_is_little_endian() to know the endianness of the device.

So the code can become:

#ifdef __LITTLE_ENDIAN
#define goldfish_ioread32 ioread32
#define goldfish_iowrite32 iowrite32
#else
#define goldfish_ioread32 ioread32be
#define goldfish_iowrite32 iowrite32be
#endif

What do you prefer: CONFIG_M68K or __LITTLE_ENDIAN?

Thanks,
Laurent
Arnd Bergmann Jan. 16, 2022, 4:19 p.m. UTC | #4
On Sun, Jan 16, 2022 at 12:54 PM Laurent Vivier <laurent@vivier.eu> wrote:
>
> Virtio has exactly the same problem in the past (prio to v1.0), and it uses __LITTLE_ENDIAN in
> virtio_legacy_is_little_endian() to know the endianness of the device.

Right, that was my point, let's not duplicate the bug from pre-standard virtio.

> So the code can become:
>
> #ifdef __LITTLE_ENDIAN
> #define goldfish_ioread32 ioread32
> #define goldfish_iowrite32 iowrite32
> #else
> #define goldfish_ioread32 ioread32be
> #define goldfish_iowrite32 iowrite32be
> #endif

This is exactly the same as what you had before, it continues to break when
host and guest disagree on the endianess.

> What do you prefer: CONFIG_M68K or __LITTLE_ENDIAN?

Checking for CONFIG_M68K at least has a chance of working, provided
that all existing qemu implementations agree on the semantics here.

       Arnd
diff mbox series

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index f65e31bab9ae..fa66fb7156da 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -711,4 +711,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 c17ee32a7151..e624a1a27027 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -88,3 +88,4 @@  obj-$(CONFIG_CSKY_MP_TIMER)		+= timer-mp-csky.o
 obj-$(CONFIG_GX6605S_TIMER)		+= timer-gx6605s.o
 obj-$(CONFIG_HYPERV_TIMER)		+= hyperv_timer.o
 obj-$(CONFIG_MICROCHIP_PIT64B)		+= timer-microchip-pit64b.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..9bf9ac8c4f5c
--- /dev/null
+++ b/drivers/clocksource/timer-goldfish.c
@@ -0,0 +1,139 @@ 
+// 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 <clocksource/timer-goldfish.h>
+
+#define TIMER_TIME_LOW		0x00	/* get low bits of current time  */
+					/*   and update TIMER_TIME_HIGH  */
+#define TIMER_TIME_HIGH		0x04	/* get high bits of time at last */
+					/*   TIMER_TIME_LOW read         */
+#define TIMER_ALARM_LOW		0x08	/* set low bits of alarm and     */
+					/*   activate it                 */
+#define TIMER_ALARM_HIGH	0x0c	/* set high bits of next alarm   */
+#define TIMER_IRQ_ENABLED	0x10
+#define TIMER_CLEAR_ALARM	0x14
+#define TIMER_ALARM_STATUS	0x18
+#define TIMER_CLEAR_INTERRUPT	0x1c
+
+/* goldfish endianness depends on CPU endianness */
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#define goldfish_ioread32 ioread32be
+#define goldfish_iowrite32 iowrite32be
+#else
+#define goldfish_ioread32 ioread32
+#define goldfish_iowrite32 iowrite32
+#endif
+
+struct goldfish_timer {
+	struct clock_event_device ced;
+	struct resource res;
+	void __iomem *base;
+	int irq;
+};
+
+static struct goldfish_timer *ced_to_gf(struct clock_event_device *ced)
+{
+	return container_of(ced, struct goldfish_timer, ced);
+}
+
+static int goldfish_timer_set_oneshot(struct clock_event_device *evt)
+{
+	struct goldfish_timer *timerdrv = ced_to_gf(evt);
+	void __iomem *base = timerdrv->base;
+
+	goldfish_iowrite32(0, base + TIMER_ALARM_HIGH);
+	goldfish_iowrite32(0, base + TIMER_ALARM_LOW);
+	goldfish_iowrite32(1, base + TIMER_IRQ_ENABLED);
+
+	return 0;
+}
+
+static int goldfish_timer_shutdown(struct clock_event_device *evt)
+{
+	struct goldfish_timer *timerdrv = ced_to_gf(evt);
+	void __iomem *base = timerdrv->base;
+
+	goldfish_iowrite32(0, base + TIMER_IRQ_ENABLED);
+
+	return 0;
+}
+
+static int goldfish_timer_next_event(unsigned long delta,
+				     struct clock_event_device *evt)
+{
+	struct goldfish_timer *timerdrv = ced_to_gf(evt);
+	void __iomem *base = timerdrv->base;
+	u64 now;
+
+	goldfish_iowrite32(1, base + TIMER_CLEAR_INTERRUPT);
+
+	/*
+	 * time_low: get low bits of current time and update time_high
+	 * time_high: get high bits of time at last time_low read
+	 */
+	now = goldfish_ioread32(base + TIMER_TIME_LOW);
+	now += (u64)goldfish_ioread32(base + TIMER_TIME_HIGH) << 32;
+
+	now += delta;
+
+	goldfish_iowrite32(upper_32_bits(now), base + TIMER_ALARM_HIGH);
+	goldfish_iowrite32(lower_32_bits(now), base + TIMER_ALARM_LOW);
+
+	return 0;
+}
+
+static irqreturn_t golfish_timer_tick(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+void __init goldfish_timer_init(int irq, void __iomem *base)
+{
+	struct goldfish_timer *timerdrv;
+	int ret;
+
+	timerdrv = kzalloc(sizeof(*timerdrv), GFP_KERNEL);
+	if (!timerdrv)
+		return;
+
+	timerdrv->base = base;
+	timerdrv->irq = irq;
+
+	timerdrv->ced = (struct clock_event_device){
+		.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,
+	};
+	timerdrv->res = (struct resource){
+		.name  = "goldfish_timer",
+		.start = (unsigned long)base,
+		.end   = (unsigned long)base + 0xfff,
+	};
+
+	if (request_resource(&iomem_resource, &timerdrv->res)) {
+		pr_err("Cannot allocate goldfish-timer resource\n");
+		return;
+	}
+
+	ret = request_irq(timerdrv->irq, golfish_timer_tick, IRQF_TIMER,
+			  "goldfish_timer", &timerdrv->ced);
+	if (ret) {
+		pr_err("Couldn't register goldfish-timer interrupt\n");
+		return;
+	}
+
+	clockevents_config_and_register(&timerdrv->ced, NSEC_PER_SEC,
+					1, 0xffffffff);
+}
diff --git a/include/clocksource/timer-goldfish.h b/include/clocksource/timer-goldfish.h
new file mode 100644
index 000000000000..b237267844f1
--- /dev/null
+++ b/include/clocksource/timer-goldfish.h
@@ -0,0 +1,11 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * goldfish-timer clocksource
+ */
+
+#ifndef _CLOCKSOURCE_GOLDFISH_TIMER_H
+#define _CLOCKSOURCE_GOLDFISH_TIMER_H
+
+extern void goldfish_timer_init(int irq, void __iomem *base);
+
+#endif /* _CLOCKSOURCE_GOLDFISH_TIMER_H */