Re: [PATCH v2 3/8] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

[Stefan Roese <sr@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

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

[-- Attachment #2: orion-timer-fix-early-init.patch --]
[-- Type: text/x-patch, Size: 3230 bytes --]

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index 6c0b8550412d..cd63ea916237 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -23,12 +23,56 @@ struct orion_timer_priv {
 
 #define MVEBU_TIMER_FIXED_RATE_25MHZ	25000000
 
+static bool early_init_done __section(".data") = false;
+
+/* Common functions for early (boot) and DM based timer */
+static void orion_timer_init(void *base, enum input_clock_type type)
+{
+	writel(~0, base + TIMER0_VAL);
+	writel(~0, base + TIMER0_RELOAD);
+
+	if (type == INPUT_CLOCK_25MHZ) {
+		/*
+		 * On Armada XP / 38x ..., the 25MHz clock source needs to
+		 * be enabled
+		 */
+		setbits_le32(base + TIMER_CTRL, BIT(11));
+	}
+
+	/* enable timer */
+	setbits_le32(base + TIMER_CTRL, TIMER0_EN | TIMER0_RELOAD_EN);
+}
+
+static uint64_t orion_timer_get_count(void *base)
+{
+	return timer_conv_64(~readl(base + TIMER0_VAL));
+}
+
+/* Early (e.g. bootstage etc) timer functions */
+static void notrace timer_early_init(void)
+{
+	/* Only init the timer once */
+	if (early_init_done)
+		return;
+	early_init_done = true;
+
+	if (IS_ENABLED(CONFIG_ARCH_MVEBU))
+		orion_timer_init((void *)MVEBU_TIMER_BASE, INPUT_CLOCK_25MHZ);
+	else
+		orion_timer_init((void *)MVEBU_TIMER_BASE, INPUT_CLOCK_NON_FIXED);
+}
+
 /**
  * timer_early_get_rate() - Get the timer rate before driver model
  */
 unsigned long notrace timer_early_get_rate(void)
 {
-	return MVEBU_TIMER_FIXED_RATE_25MHZ;
+	timer_early_init();
+
+	if (IS_ENABLED(CONFIG_ARCH_MVEBU))
+		return MVEBU_TIMER_FIXED_RATE_25MHZ;
+	else
+		return CONFIG_SYS_TCLK;
 }
 
 /**
@@ -37,7 +81,9 @@ unsigned long notrace timer_early_get_rate(void)
  */
 u64 notrace timer_early_get_count(void)
 {
-	return timer_conv_64(~readl(MVEBU_TIMER_BASE + TIMER0_VAL));
+	timer_early_init();
+
+	return orion_timer_get_count((void *)MVEBU_TIMER_BASE);
 }
 
 ulong timer_get_boot_us(void)
@@ -48,11 +94,12 @@ ulong timer_get_boot_us(void)
 	return lldiv(ticks * 1000, timer_early_get_rate());
 }
 
-static uint64_t orion_timer_get_count(struct udevice *dev)
+/* DM timer functions */
+static uint64_t dm_orion_timer_get_count(struct udevice *dev)
 {
 	struct orion_timer_priv *priv = dev_get_priv(dev);
 
-	return timer_conv_64(~readl(priv->base + TIMER0_VAL));
+	return orion_timer_get_count(priv->base);
 }
 
 static int orion_timer_probe(struct udevice *dev)
@@ -67,28 +114,17 @@ static int orion_timer_probe(struct udevice *dev)
 		return -ENOMEM;
 	}
 
-	writel(~0, priv->base + TIMER0_VAL);
-	writel(~0, priv->base + TIMER0_RELOAD);
-
-	if (type == INPUT_CLOCK_25MHZ) {
-		/*
-		 * On Armada XP / 38x ..., the 25MHz clock source needs to
-		 * be enabled
-		 */
-		setbits_le32(priv->base + TIMER_CTRL, BIT(11));
+	if (type == INPUT_CLOCK_25MHZ)
 		uc_priv->clock_rate = MVEBU_TIMER_FIXED_RATE_25MHZ;
-	} else {
+	else
 		uc_priv->clock_rate = CONFIG_SYS_TCLK;
-	}
-
-	/* enable timer */
-	setbits_le32(priv->base + TIMER_CTRL, TIMER0_EN | TIMER0_RELOAD_EN);
+	orion_timer_init(priv->base, type);
 
 	return 0;
 }
 
 static const struct timer_ops orion_timer_ops = {
-	.get_count = orion_timer_get_count,
+	.get_count = dm_orion_timer_get_count,
 };
 
 static const struct udevice_id orion_timer_ids[] = {