Thread (18 messages) 18 messages, 6 authors, 2025-10-30

Re: [PATCH 2/4] clocksource/drivers/arm_arch_timer: Add standalone MMIO driver

From: Steven Price <steven.price@arm.com>
Date: 2025-08-14 10:13:51
Also in: linux-acpi, lkml

On 07/08/2025 17:02, Marc Zyngier wrote:
Add a new driver for the MMIO side of the ARM architected timer.
Most of it has been lifted from the existing arch timer code,
massaged, and finally rewritten.

It supports both DT and ACPI as firmware descriptions.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 MAINTAINERS                               |   1 +
 drivers/clocksource/arm_arch_timer_mmio.c | 420 ++++++++++++++++++++++
 2 files changed, 421 insertions(+)
 create mode 100644 drivers/clocksource/arm_arch_timer_mmio.c
[...]
+static void arch_timer_mmio_setup(struct arch_timer *at, int irq)
+{
+	at->evt = (struct clock_event_device) {
+		.features		   = (CLOCK_EVT_FEAT_ONESHOT |
+					      CLOCK_EVT_FEAT_DYNIRQ),
+		.name			   = "arch_mem_timer",
+		.rating			   = 400,
+		.cpumask		   = cpu_possible_mask,
+		.irq 			   = irq,
+		.set_next_event		   = arch_timer_mmio_set_next_event,
+		.set_state_oneshot_stopped = arch_timer_mmio_shutdown,
+		.set_state_shutdown	   = arch_timer_mmio_shutdown,
+	};
+
+	at->evt.set_state_shutdown(&at->evt);
+
+	clockevents_config_and_register(&at->evt, at->rate, 0xf, CLOCKSOURCE_MASK(56));
This doesn't work on 32 bit - clockevents_config_and_register()'s final
argument is an unsigned long, and a 56 bit mask doesn't fit. This
triggers a compiler warning:

 drivers/clocksource/arm_arch_timer_mmio.c: In function
'arch_timer_mmio_setup':
 ./include/linux/bits.h:47:9: error: conversion from 'long long unsigned
int' to 'long unsigned int' changes value from '72057594037927935' to
'4294967295' [-Werror=overflow]
    47 |         ((t)(GENMASK_INPUT_CHECK(h, l) +                        \
       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    48 |              (type_max(t) << (l) &                              \
       |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    49 |               type_max(t) >> (BITS_PER_TYPE(t) - 1 - (h)))))
       |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ./include/linux/bits.h:52:33: note: in expansion of macro 'GENMASK_TYPE'
    52 | #define GENMASK_ULL(h, l)       GENMASK_TYPE(unsigned long
long, h, l)
       |                                 ^~~~~~~~~~~~
 ./include/linux/clocksource.h:153:32: note: in expansion of macro
'GENMASK_ULL'
   153 | #define CLOCKSOURCE_MASK(bits) GENMASK_ULL((bits) - 1, 0)
       |                                ^~~~~~~~~~~
 drivers/clocksource/arm_arch_timer_mmio.c:255:66: note: in expansion of
macro 'CLOCKSOURCE_MASK'
   255 |         clockevents_config_and_register(&at->evt, at->rate,
0xf, CLOCKSOURCE_MASK(56));
       |
 ^~~~~~~~~~~~~~~~

Possible this should really be min(CLOCKSOURCE_MASK(56), ULONG_MAX)? But
I'm not familiar enough with this code. Most likely it's dead code on a
32 bit platform.

Thanks,
Steve

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help