[PATCH] arm: kirkwood: 88f6281: Detect CONFIG_SYS_TCLK from SAR register

[Pali Rohár]

On Tuesday 16 August 2022 23:34:13 Michael Walle wrote:
> Am 2022-08-16 22:00, schrieb Pali Rohár:
> > Bit 21 in SAR register specifies if TCLK is running at 166 MHz or 200
> > MHz.
> > This information is undocumented in public Marvell Kirkwood Functional
> > Specifications [2], but is available in Linux v3.15 kirkwood code [1].
> > 
> > Commit 8ac303d49f89 ("arm: kirkwood: Do not overwrite CONFIG_SYS_TCLK")
> > broke support for Marvell 88F6281 SoCs because it was expected that all
> > those SoCs have TCLK running at 200 MHz as specified in Marvell 88F6281
> > Hardware Specifications [3].
> > 
> > Fix broken support for 88F6281 by detecting CONFIG_SYS_TCLK from SAR
> > register, like it was doing Linux v3.15.
> > 
> > [1] -
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/arch/arm/mach-kirkwood/common.c?h=v3.15#n542
> > [2] -
> > https://web.archive.org/web/20130730091033/http://www.marvell.com/embedded-processors/kirkwood/assets/FS_88F6180_9x_6281_OpenSource.pdf
> > [3] -
> > https://web.archive.org/web/20120620073511/http://www.marvell.com/embedded-processors/kirkwood/assets/HW_88F6281_OpenSource.pdf
> > 
> > Fixes: 8ac303d49f89 ("arm: kirkwood: Do not overwrite CONFIG_SYS_TCLK")
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> > Michael, please test this patch, if it fixes your boards.
> 
> Thanks, but this doesn't compile:
> 
> In file included from lib/time.c:19:
> lib/time.c: In function ‘timer_get_boot_us’:
> ./arch/arm/include/asm/io.h:108:19: error: token "{" is not valid in
> preprocessor expressions
>   108 | #define readl(c) ({ u32 __v = __arch_getl(c); __iormb(); __v; })
>       |                   ^
> ./arch/arm/include/asm/arch/kw88f6281.h:18:29: note: in expansion of macro
> ‘readl’
>    18 | #define CONFIG_SYS_TCLK   ((readl(CONFIG_SAR_REG) & BIT(21)) ? \
>       |                             ^~~~~
> ./arch/arm/include/asm/arch/config.h:56:32: note: in expansion of macro
> ‘CONFIG_SYS_TCLK’
>    56 | #define CONFIG_SYS_TIMER_RATE  CONFIG_SYS_TCLK
>       |                                ^~~~~~~~~~~~~~~
> lib/time.c:50:5: note: in expansion of macro ‘CONFIG_SYS_TIMER_RATE’
>    50 | #if CONFIG_SYS_TIMER_RATE == 1000000
>       |     ^~~~~~~~~~~~~~~~~~~~~
> ./arch/arm/include/asm/io.h:108:19: error: token "{" is not valid in
> preprocessor expressions
>   108 | #define readl(c) ({ u32 __v = __arch_getl(c); __iormb(); __v; })
>       |                   ^
> ./arch/arm/include/asm/arch/kw88f6281.h:18:29: note: in expansion of macro
> ‘readl’
>    18 | #define CONFIG_SYS_TCLK   ((readl(CONFIG_SAR_REG) & BIT(21)) ? \
>       |                             ^~~~~
> ./arch/arm/include/asm/arch/config.h:56:32: note: in expansion of macro
> ‘CONFIG_SYS_TCLK’
>    56 | #define CONFIG_SYS_TIMER_RATE  CONFIG_SYS_TCLK
>       |                                ^~~~~~~~~~~~~~~
> lib/time.c:52:7: note: in expansion of macro ‘CONFIG_SYS_TIMER_RATE’
>    52 | #elif CONFIG_SYS_TIMER_RATE > 1000000
>       |       ^~~~~~~~~~~~~~~~~~~~~
> make[1]: *** [scripts/Makefile.build:257: lib/time.o] Fehler 1
> 
> -michael

Ah :-( And why it works on other Armada mvebu boards? Because of this:

$ git grep CONFIG_SYS_TIMER_RATE arch/arm/mach-mvebu
arch/arm/mach-mvebu/include/mach/config.h:#define CONFIG_SYS_TIMER_RATE         25000000

I have looked how is this option used and it is unit for
CONFIG_SYS_TIMER_COUNTER option. On all kirkwood and mvebu platforms is
that register set to SoC Global Timer 0 which on Armada 38x uses either
fixed 25MHz clock or NBCLK clock, together with SoC Global Timer 0
Ratio. When NBCLK then the frequency of that timer register is:
NBCLK / 2 / (2 ^ ratio). Default ratio is 0 (but can be configured up to
7). By default fixed 25MHz clock is used.

Hence for mvebu it is OK as U-Boot does not change default values of SoC
Global Timer 0.

For kirkwood it looks like that those timers are based on TCLK (page 283
in Functional Specifications).

How to fix it? I do not know right now. I have just quirk & dirty
solution by moving code from preprocessor to compiler:

diff --git a/lib/time.c b/lib/time.c
index 96074b84af69..f2ffc0498d39 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -46,13 +46,15 @@ unsigned long notrace timer_read_counter(void)
 ulong timer_get_boot_us(void)
 {
 	ulong count = timer_read_counter();
-
-#if CONFIG_SYS_TIMER_RATE == 1000000
-	return count;
-#elif CONFIG_SYS_TIMER_RATE > 1000000
-	return lldiv(count, CONFIG_SYS_TIMER_RATE / 1000000);
-#elif defined(CONFIG_SYS_TIMER_RATE)
-	return (unsigned long long)count * 1000000 / CONFIG_SYS_TIMER_RATE;
+#ifdef CONFIG_SYS_TIMER_RATE
+	const ulong timer_rate = CONFIG_SYS_TIMER_RATE;
+
+	if (timer_rate)
+		return count;
+	else if (timer_rate > 1000000)
+		return lldiv(count, timer_rate / 1000000);
+	else
+		return (unsigned long long)count * 1000000 / timer_rate;
 #else
 	/* Assume the counter is in microseconds */
 	return count;

At least it could be used for verifying if CONFIG_SYS_TCLK is working.