arch_timer updates for v4.15

Hi Daniel,

Please could you pull these arch timer updates for v4.15?

The first patch is a fix from Ard that fixes probling the MMIO timers on some platforms. The other patch is a minor cleanup that prevents a warning when building tags.

Thanks,
Mark.

The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff:

Linux 4.14-rc3 (2017-10-01 14:54:54 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git tags/arch-timer-4.15

for you to fetch changes up to cb3a23a029f6af0c73498b6a9773d6ec19541bf1:

clocksource: arm_arch_timer: fix DEFINE_PER_CPU expansion (2017-10-16 15:37:16 +0100)

----------------------------------------------------------------
arch_timer updates for v4.15

- enable the MMIO CNTFRQ before reading it
- avoid a warning when building tags

----------------------------------------------------------------
Ard Biesheuvel (1):
clocksource: arm_arch_timer: validate CNTFRQ after enabling frame

Mark Rutland (1):
clocksource: arm_arch_timer: fix DEFINE_PER_CPU expansion

 drivers/clocksource/arm_arch_timer.c | 41 +++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

From: Ard Biesheuvel <[email protected]>

The ACPI GTDT code validates the CNTFRQ field of each MMIO timer frame against the CNTFRQ system register of the current CPU, to ensure that they are equal, which is mandated by the architecture.

However, reading the CNTFRQ field of a frame is not possible until the RFRQ bit in the frame's CNTACRn register is set, and doing so before that willl produce the following error:

arch_timer: [Firmware Bug]: CNTFRQ mismatch: frame @ 0x00000000e0be0000: (0x00000000), CPU: (0x0ee6b280)
arch_timer: Disabling MMIO timers due to CNTFRQ mismatch
arch_timer: Failed to initialize memory-mapped timer.

The reason is that the CNTFRQ field is RES0 if access is not enabled.

So move the validation of CNTFRQ into the loop that iterates over the timers to find the best frame, but defer it until after we have selected the best frame, which should also have enabled the RFRQ bit.

---
 drivers/clocksource/arm_arch_timer.c | 38 ++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

index fd4b7f684bd0..14e2419063e9 100644
@@ -1268,10 +1268,6 @@ arch_timer_mem_find_best_frame(struct arch_timer_mem *timer_mem)
 
 	iounmap(cntctlbase);
 
-	if (!best_frame)
-		pr_err("Unable to find a suitable frame in timer @ %pa\n",
-			&timer_mem->cntctlbase);
-
 	return best_frame;
 }
 

@@ -1372,6 +1368,8 @@ static int __init arch_timer_mem_of_init(struct device_node *np)
 
 	frame = arch_timer_mem_find_best_frame(timer_mem);
 	if (!frame) {
+		pr_err("Unable to find a suitable frame in timer @ %pa\n",
+			&timer_mem->cntctlbase);
 		ret = -EINVAL;
 		goto out;
 	}

@@ -1420,7 +1418,7 @@ arch_timer_mem_verify_cntfrq(struct arch_timer_mem *timer_mem)
 static int __init arch_timer_mem_acpi_init(int platform_timer_count)
 {
 	struct arch_timer_mem *timers, *timer;
-	struct arch_timer_mem_frame *frame;
+	struct arch_timer_mem_frame *frame, *best_frame = NULL;
 	int timer_count, i, ret = 0;
 
 	timers = kcalloc(platform_timer_count, sizeof(*timers),

@@ -1432,14 +1430,6 @@ static int __init arch_timer_mem_acpi_init(int platform_timer_count)
 	if (ret || !timer_count)
 		goto out;
 
-	for (i = 0; i < timer_count; i++) {
-		ret = arch_timer_mem_verify_cntfrq(&timers[i]);
-		if (ret) {
-			pr_err("Disabling MMIO timers due to CNTFRQ mismatch\n");
-			goto out;
-		}
-	}
-
 	/*
 	 * While unlikely, it's theoretically possible that none of the frames
 	 * in a timer expose the combination of feature we want.

@@ -1448,12 +1438,26 @@ static int __init arch_timer_mem_acpi_init(int platform_timer_count)
 		timer = &timers[i];
 
 		frame = arch_timer_mem_find_best_frame(timer);
-		if (frame)
-			break;
+		if (!best_frame)
+			best_frame = frame;
+
+		ret = arch_timer_mem_verify_cntfrq(timer);
+		if (ret) {
+			pr_err("Disabling MMIO timers due to CNTFRQ mismatch\n");
+			goto out;
+		}
+
+		if (!best_frame) /* implies !frame */
+			/*
+			 * Only complain about missing suitable frames if we
+			 * haven't already found one in a previous iteration.
+			 */
+			pr_err("Unable to find a suitable frame in timer @ %pa\n",
+				&timer->cntctlbase);
 	}
 
-	if (frame)
-		ret = arch_timer_mem_frame_register(frame);
+	if (best_frame)
+		ret = arch_timer_mem_frame_register(best_frame);
 out:
 	kfree(timers);
 	return ret;

2.11.0

Our ctags mangling script can't handle newlines inside of a DEFINE_PER_CPU(), leading to an annoying message whenever tags are built:

ctags: Warning: drivers/clocksource/arm_arch_timer.c:302: null expansion of name pattern "\1"

This was dealt with elsewhere in commit:

25528213fe9f75f4 ("tags: Fix DEFINE_PER_CPU expansions")

... by ensuring each DEFINE_PER_CPU() was contained on a single line, even where this would violate the usual code style (checkpatch warnings and all).

Let's do the same for the arch timer driver, and get rid of the distraction.

---
 drivers/clocksource/arm_arch_timer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 14e2419063e9..0ecf5beb56ec 100644
@@ -299,8 +299,7 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
 #endif
 
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
-DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
-	       timer_unstable_counter_workaround);
+DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *, timer_unstable_counter_workaround);
 EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
 
 DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);

2.11.0

Some of the definitions are very long and can't be split into multiple lines because ctags is limited.

Exempt these lines from the line length checks.

---
 scripts/checkpatch.pl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 2a8c6c3c1bdb..5fa0f5467d99 100755
@@ -2900,8 +2900,9 @@ sub process {
 				 $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) {
 				$msg_type = "";
 
-			# EFI_GUID is another special case
-			} elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/) {
+			# More special cases
+			} elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/ ||
+				 $line =~ /^\+\s*(?:\w+)?\s*DEFINE_PER_CPU/) {
 				$msg_type = "";
 
 			# Otherwise set the alternate message types

It might make sense to point at commit:

25528213fe9f75f4 ("tags: Fix DEFINE_PER_CPU expansions")

... for the gory details, but either way:

Thanks,
Mark.

Hi Mark,

I applied the patches for 4.15.

Thanks!

-- Daniel

 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog