| Message ID | 20220902062554.1197435-4-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | Enable CONFIG_TIMER for all Kirkwood / MVEBU boards | expand |
Hi Stefan, On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: > > Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > enabled, like pogo_v4. > > Signed-off-by: Stefan Roese <sr@denx.de> > --- > v2: > - Change timer_get_boot_us() to use the timer_early functions > - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > > Simon, I'm currently looking into this timer_get_boot_us() to using > timer_early_get_count() etc consolidation. Indeed, as you've mentioned above, I think timer_early_get_count() and timer_early_get_rate() do need to take into consideration what the input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as the Pogo V4. I'm seeing on the Pogo V4 test, the timer command reports time about 6 times slower than it should. It does seem to jive with the fact that the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. All the best, Tony > > drivers/timer/orion-timer.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c > index 14f318e57d4d..6c0b8550412d 100644 > --- a/drivers/timer/orion-timer.c > +++ b/drivers/timer/orion-timer.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0+ > #include <asm/io.h> > #include <common.h> > +#include <div64.h> > #include <dm/device.h> > #include <dm/fdtaddr.h> > #include <timer.h> > @@ -39,6 +40,14 @@ u64 notrace timer_early_get_count(void) > return timer_conv_64(~readl(MVEBU_TIMER_BASE + TIMER0_VAL)); > } > > +ulong timer_get_boot_us(void) > +{ > + u64 ticks; > + > + ticks = timer_early_get_count(); > + return lldiv(ticks * 1000, timer_early_get_rate()); > +} > + > static uint64_t orion_timer_get_count(struct udevice *dev) > { > struct orion_timer_priv *priv = dev_get_priv(dev); > -- > 2.37.3 >
Hi Tony, On 03.09.22 11:44, Tony Dinh wrote: > Hi Stefan, > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: >> >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE >> enabled, like pogo_v4. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> --- >> v2: >> - Change timer_get_boot_us() to use the timer_early functions >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) >> >> Simon, I'm currently looking into this timer_get_boot_us() to using >> timer_early_get_count() etc consolidation. > > Indeed, as you've mentioned above, I think timer_early_get_count() and > timer_early_get_rate() do need to take into consideration what the > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > the Pogo V4. > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > times slower than it should. It does seem to jive with the fact that > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. Ah, I've missing updating the early functions to also differentiate between fixed clocks and TCLK timer. Please give the attached patch a try - should be applied on top of this latest patchset. Thanks, Stefan
Hi Stefan, On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese <sr@denx.de> wrote: > > Hi Tony, > > On 03.09.22 11:44, Tony Dinh wrote: > > Hi Stefan, > > > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: > >> > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > >> enabled, like pogo_v4. > >> > >> Signed-off-by: Stefan Roese <sr@denx.de> > >> --- > >> v2: > >> - Change timer_get_boot_us() to use the timer_early functions > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > >> > >> Simon, I'm currently looking into this timer_get_boot_us() to using > >> timer_early_get_count() etc consolidation. > > > > Indeed, as you've mentioned above, I think timer_early_get_count() and > > timer_early_get_rate() do need to take into consideration what the > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > > the Pogo V4. > > > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > > times slower than it should. It does seem to jive with the fact that > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. > > Ah, I've missing updating the early functions to also differentiate > between fixed clocks and TCLK timer. > > Please give the attached patch a try - should be applied on top of this > latest patchset. That looks promising, but I think we are still missing something. After applying the attached patch, I ran the test again and it behaved the same way (clock rate 6 times slower). So I did another test. > Thanks, > Stefan
Hi Stefan, Sorry, that message was prematurely sent (fat finger). Please see the continuation below. On Sat, Sep 3, 2022 at 4:43 PM Tony Dinh <mibodhi@gmail.com> wrote: > > Hi Stefan, > > On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese <sr@denx.de> wrote: > > > > Hi Tony, > > > > On 03.09.22 11:44, Tony Dinh wrote: > > > Hi Stefan, > > > > > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: > > >> > > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > > >> enabled, like pogo_v4. > > >> > > >> Signed-off-by: Stefan Roese <sr@denx.de> > > >> --- > > >> v2: > > >> - Change timer_get_boot_us() to use the timer_early functions > > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > > >> > > >> Simon, I'm currently looking into this timer_get_boot_us() to using > > >> timer_early_get_count() etc consolidation. > > > > > > Indeed, as you've mentioned above, I think timer_early_get_count() and > > > timer_early_get_rate() do need to take into consideration what the > > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > > > the Pogo V4. > > > > > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > > > times slower than it should. It does seem to jive with the fact that > > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. > > > > Ah, I've missing updating the early functions to also differentiate > > between fixed clocks and TCLK timer. > > > > Please give the attached patch a try - should be applied on top of this > > latest patchset. > That looks promising, but I think we are still missing something. After applying the attached patch, I ran the test again and it behaved the same way (clock rate 6 times slower). So I did another test. --- Test 1 Pogo_V4> timer start; sleep 60; timer get; sleep 30; timer get 60.000 90.000 So apparently the sleep cmd has reset the correct clock rate. --- Test 2 Pogo_V4> timer start; sleep 30; timer get; sleep 30; timer get 30.000 60.000 And then wait for 30 seconds, do another "timer get" (I expected to see about 90 to 95 seconds). Pogo_V4> timer get 66.237 The different call tree with the timer vs the sleep command made a difference. But I could not see where. --- Test 3 Then I recalled about CONFIG_TIMER_EARLY. It is not selected by default in Kconfig currently. So I enabled that to see the behavior. But running this build hangs right off the bat. No u-boot version banner is printed out. Thanks, Tony > > > > > Thanks, > > Stefan
Hi Pali, On Sat, Sep 3, 2022 at 5:02 PM Tony Dinh <mibodhi@gmail.com> wrote: > > Hi Stefan, > > Sorry, that message was prematurely sent (fat finger). Please see the > continuation below. > > On Sat, Sep 3, 2022 at 4:43 PM Tony Dinh <mibodhi@gmail.com> wrote: > > > > Hi Stefan, > > > > On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese <sr@denx.de> wrote: > > > > > > Hi Tony, > > > > > > On 03.09.22 11:44, Tony Dinh wrote: > > > > Hi Stefan, > > > > > > > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: > > > >> > > > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > > > >> enabled, like pogo_v4. > > > >> > > > >> Signed-off-by: Stefan Roese <sr@denx.de> > > > >> --- > > > >> v2: > > > >> - Change timer_get_boot_us() to use the timer_early functions > > > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > > > >> > > > >> Simon, I'm currently looking into this timer_get_boot_us() to using > > > >> timer_early_get_count() etc consolidation. > > > > > > > > Indeed, as you've mentioned above, I think timer_early_get_count() and > > > > timer_early_get_rate() do need to take into consideration what the > > > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > > > > the Pogo V4. > > > > > > > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > > > > times slower than it should. It does seem to jive with the fact that > > > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. > > > > > > Ah, I've missing updating the early functions to also differentiate > > > between fixed clocks and TCLK timer. > > > > > > Please give the attached patch a try - should be applied on top of this > > > latest patchset. > > > > That looks promising, but I think we are still missing something. > After applying the attached patch, I ran the test again and it behaved > the same way (clock rate 6 times slower). So I did another test. > > --- Test 1 > Pogo_V4> timer start; sleep 60; timer get; sleep 30; timer get > 60.000 > 90.000 > > So apparently the sleep cmd has reset the correct clock rate. > > --- Test 2 > > Pogo_V4> timer start; sleep 30; timer get; sleep 30; timer get > 30.000 > 60.000 > > And then wait for 30 seconds, do another "timer get" (I expected to > see about 90 to 95 seconds). > > Pogo_V4> timer get > 66.237 > > The different call tree with the timer vs the sleep command made a > difference. But I could not see where. > > --- Test 3 > > Then I recalled about CONFIG_TIMER_EARLY. It is not selected by > default in Kconfig currently. So I enabled that to see the behavior. > But running this build hangs right off the bat. No u-boot version > banner is printed out. > Is there a way to get the CPU frequency from the board upon start up? Thanks, Tony
On Saturday 03 September 2022 17:38:01 Tony Dinh wrote: > Hi Pali, > > Is there a way to get the CPU frequency from the board upon start up? > > Thanks, > Tony Hello! Each SoC has either fixed CPU frequency or has some bits in SAR register which say on which frequency is CPU running. SAR bits are SoC specific and are documented either in Functional Specifications or in Hardware Specifications (both documents are available for free all 32-bit Marvell SoCs except 375 and 39x).
Hi Pali, On Sat, Sep 3, 2022 at 6:08 PM Pali Rohár <pali@kernel.org> wrote: > > On Saturday 03 September 2022 17:38:01 Tony Dinh wrote: > > Hi Pali, > > > > Is there a way to get the CPU frequency from the board upon start up? > > > > Thanks, > > Tony > > Hello! Each SoC has either fixed CPU frequency or has some bits in SAR > register which say on which frequency is CPU running. SAR bits are SoC > specific and are documented either in Functional Specifications or in > Hardware Specifications (both documents are available for free all > 32-bit Marvell SoCs except 375 and 39x). Thanks for the recap. I do remember you posted a patch for that detection. https://lists.denx.de/pipermail/u-boot/2022-August/492034.html Just want to make sure that the SAR register is the only way we can detect it in u-boot. The Kirkwood 6192 SoC on this Pogo V4 board can run at 200 Mhz and 800 Mhz with frequency scaling in Linux. Thanks, Tony
Am 2022-09-04 02:02, schrieb Tony Dinh: > Hi Stefan, > > Sorry, that message was prematurely sent (fat finger). Please see the > continuation below. > > On Sat, Sep 3, 2022 at 4:43 PM Tony Dinh <mibodhi@gmail.com> wrote: >> >> Hi Stefan, >> >> On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese <sr@denx.de> wrote: >> > >> > Hi Tony, >> > >> > On 03.09.22 11:44, Tony Dinh wrote: >> > > Hi Stefan, >> > > >> > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: >> > >> >> > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE >> > >> enabled, like pogo_v4. >> > >> >> > >> Signed-off-by: Stefan Roese <sr@denx.de> >> > >> --- >> > >> v2: >> > >> - Change timer_get_boot_us() to use the timer_early functions >> > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) >> > >> >> > >> Simon, I'm currently looking into this timer_get_boot_us() to using >> > >> timer_early_get_count() etc consolidation. >> > > >> > > Indeed, as you've mentioned above, I think timer_early_get_count() and >> > > timer_early_get_rate() do need to take into consideration what the >> > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as >> > > the Pogo V4. >> > > >> > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 >> > > times slower than it should. It does seem to jive with the fact that >> > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. >> > >> > Ah, I've missing updating the early functions to also differentiate >> > between fixed clocks and TCLK timer. >> > >> > Please give the attached patch a try - should be applied on top of this >> > latest patchset. >> > > That looks promising, but I think we are still missing something. > After applying the attached patch, I ran the test again and it behaved > the same way (clock rate 6 times slower). So I did another test. > > --- Test 1 > Pogo_V4> timer start; sleep 60; timer get; sleep 30; timer get > 60.000 > 90.000 > > So apparently the sleep cmd has reset the correct clock rate. > > --- Test 2 > > Pogo_V4> timer start; sleep 30; timer get; sleep 30; timer get > 30.000 > 60.000 > > And then wait for 30 seconds, do another "timer get" (I expected to > see about 90 to 95 seconds). > > Pogo_V4> timer get > 66.237 I've did the same test and can confirm it. But I think that is a drawback from the timer command. With a 32bit timer and a TCLK of 166MHz (on my board), the timer will wrap around about every 26s. So if you don't do a get_timer() call for that whole period, the overflow won't be counted in timer_conv_64(). For the sleep command it works correctly, because it will poll get_timer() every 100us. In summary, you cannot expect a "timer get" on the console to work if you cannot make sure, the you call it at least every "2^32 / TCLK" seconds. -michael
On Saturday 03 September 2022 19:36:03 Tony Dinh wrote: > Hi Pali, > > On Sat, Sep 3, 2022 at 6:08 PM Pali Rohár <pali@kernel.org> wrote: > > > > On Saturday 03 September 2022 17:38:01 Tony Dinh wrote: > > > Hi Pali, > > > > > > Is there a way to get the CPU frequency from the board upon start up? > > > > > > Thanks, > > > Tony > > > > Hello! Each SoC has either fixed CPU frequency or has some bits in SAR > > register which say on which frequency is CPU running. SAR bits are SoC > > specific and are documented either in Functional Specifications or in > > Hardware Specifications (both documents are available for free all > > 32-bit Marvell SoCs except 375 and 39x). > > Thanks for the recap. I do remember you posted a patch for that detection. > https://lists.denx.de/pipermail/u-boot/2022-August/492034.html > > Just want to make sure that the SAR register is the only way we can > detect it in u-boot. The Kirkwood 6192 SoC on this Pogo V4 board can > run at 200 Mhz and 800 Mhz with frequency scaling in Linux. > > Thanks, > Tony Well, All those SoCs have more clocks which are used for different peripherals. TCLK is something like base block and CPU is connected to different clock. Settings for it CPU clock is also in SAR register (different bits). And then there can be something like CPU scaling and cpufreq driver with configuration via different registers which can modify divisor of the main CPU clock. Which effectively can lower CPU speed. This cpufreq part is not implemented in U-Boot and IIRC CPU clock divisor should be at 1 = full CPU speed.
Hi Michael, On Sun, Sep 4, 2022 at 1:54 AM Michael Walle <michael@walle.cc> wrote: > > Am 2022-09-04 02:02, schrieb Tony Dinh: > > Hi Stefan, > > > > Sorry, that message was prematurely sent (fat finger). Please see the > > continuation below. > > > > On Sat, Sep 3, 2022 at 4:43 PM Tony Dinh <mibodhi@gmail.com> wrote: > >> > >> Hi Stefan, > >> > >> On Sat, Sep 3, 2022 at 3:44 AM Stefan Roese <sr@denx.de> wrote: > >> > > >> > Hi Tony, > >> > > >> > On 03.09.22 11:44, Tony Dinh wrote: > >> > > Hi Stefan, > >> > > > >> > > On Thu, Sep 1, 2022 at 11:25 PM Stefan Roese <sr@denx.de> wrote: > >> > >> > >> > >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE > >> > >> enabled, like pogo_v4. > >> > >> > >> > >> Signed-off-by: Stefan Roese <sr@denx.de> > >> > >> --- > >> > >> v2: > >> > >> - Change timer_get_boot_us() to use the timer_early functions > >> > >> - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) > >> > >> > >> > >> Simon, I'm currently looking into this timer_get_boot_us() to using > >> > >> timer_early_get_count() etc consolidation. > >> > > > >> > > Indeed, as you've mentioned above, I think timer_early_get_count() and > >> > > timer_early_get_rate() do need to take into consideration what the > >> > > input_clock_type is for Kirkwood boards with CONFIG_BOOTSTAGE such as > >> > > the Pogo V4. > >> > > > >> > > I'm seeing on the Pogo V4 test, the timer command reports time about 6 > >> > > times slower than it should. It does seem to jive with the fact that > >> > > the Pogo V4 CONFIG_SYS_TCLK is 166Mhz, versus MVEBU 25MHz clock rate. > >> > > >> > Ah, I've missing updating the early functions to also differentiate > >> > between fixed clocks and TCLK timer. > >> > > >> > Please give the attached patch a try - should be applied on top of this > >> > latest patchset. > >> > > > > That looks promising, but I think we are still missing something. > > After applying the attached patch, I ran the test again and it behaved > > the same way (clock rate 6 times slower). So I did another test. > > > > --- Test 1 > > Pogo_V4> timer start; sleep 60; timer get; sleep 30; timer get > > 60.000 > > 90.000 > > > > So apparently the sleep cmd has reset the correct clock rate. > > > > --- Test 2 > > > > Pogo_V4> timer start; sleep 30; timer get; sleep 30; timer get > > 30.000 > > 60.000 > > > > And then wait for 30 seconds, do another "timer get" (I expected to > > see about 90 to 95 seconds). > > > > Pogo_V4> timer get > > 66.237 > > I've did the same test and can confirm it. But I think that is > a drawback from the timer command. With a 32bit timer and a > TCLK of 166MHz (on my board), the timer will wrap around about > every 26s. So if you don't do a get_timer() call for that whole > period, the overflow won't be counted in timer_conv_64(). > > For the sleep command it works correctly, because it will > poll get_timer() every 100us. > > In summary, you cannot expect a "timer get" on the console > to work if you cannot make sure, the you call it at least > every "2^32 / TCLK" seconds. > > -michael Thanks for confirming that and the explanation. It makes perfect sense! I think I got side tracked and never noticed that it is a 32-bit timer wrap- around :) All the best, Tony
diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c index 14f318e57d4d..6c0b8550412d 100644 --- a/drivers/timer/orion-timer.c +++ b/drivers/timer/orion-timer.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ #include <asm/io.h> #include <common.h> +#include <div64.h> #include <dm/device.h> #include <dm/fdtaddr.h> #include <timer.h> @@ -39,6 +40,14 @@ u64 notrace timer_early_get_count(void) return timer_conv_64(~readl(MVEBU_TIMER_BASE + TIMER0_VAL)); } +ulong timer_get_boot_us(void) +{ + u64 ticks; + + ticks = timer_early_get_count(); + return lldiv(ticks * 1000, timer_early_get_rate()); +} + static uint64_t orion_timer_get_count(struct udevice *dev) { struct orion_timer_priv *priv = dev_get_priv(dev);
Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE enabled, like pogo_v4. Signed-off-by: Stefan Roese <sr@denx.de> --- v2: - Change timer_get_boot_us() to use the timer_early functions - Remove #if CONFIG_IS_ENABLED(BOOTSTAGE) Simon, I'm currently looking into this timer_get_boot_us() to using timer_early_get_count() etc consolidation. drivers/timer/orion-timer.c | 9 +++++++++ 1 file changed, 9 insertions(+)