[PATCH] arm: kirkwood: 88f6281: Detect CONFIG_SYS_TCLK from SAR register
Michael Walle
michael at walle.cc
Wed Aug 17 01:23:34 CEST 2022
Am 2022-08-17 00:39, schrieb Pali Rohár:
> On Wednesday 17 August 2022 00:14:20 Pali Rohár wrote:
>> 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)
> ~~~~~~~~~~
> Here needs to be if (timer_rate == 1000000)
>
>> + 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.
FWIW, this is working.
> Anyway, it looks like that this timer code is deprecated and boards
> should migrate to use DM.
>
> So maybe the proper fix for this would be to add kirkwood dm driver?
Eventually sure, but for now I'd like to have a working board without
spending too much time on fixing this. I'd propose to revert the
broken commit for now and then, we can work on a proper timer driver.
But I can already say that the u-boot binary is almost at its size
limit. I've looked into CONFIG_TIMER and it seems that we'd also
need the whole clock infrastructure (and DM_CLK). No?
-michael
More information about the U-Boot
mailing list