Re: [PATCH] timer: orion-timer: Fix problem with early static variable

[Stefan Roese]

On 12/21/22 10:18, Stefan Roese wrote:

We've noticed that at least one Kirkwood board (Pogo v4) has problems
with the new orion DM timer implementation. Debugging revealed that this
issue is related with the static variable "early_init_done" which does
not work correctly before relocation in all cases.

This patch removes this static variable and replaces it's functionality
via a function that detects if the timer is already initialized.

Signed-off-by: Stefan Roese 
Cc: Pali Rohár 
Cc: Michael Walle 
Cc: Tony Dinh 


Applied to u-boot-marvell/master

Thanks,
Stefan


---
  drivers/timer/orion-timer.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index d0eab3ce781d..6804bf0fa2cb 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -23,15 +23,19 @@ struct orion_timer_priv {
  
  #define MVEBU_TIMER_FIXED_RATE_25MHZ	2500
  
-static bool early_init_done __section(".data") = false;

+static bool early_init_done(void *base)
+{
+   if (readl(base + TIMER_CTRL) & TIMER0_EN)
+   return true;
+   return false;
+}
  
  /* Common functions for early (boot) and DM based timer */

  static void orion_timer_init(void *base, enum input_clock_type type)
  {
/* Only init the timer once */
-   if (early_init_done)
+   if (early_init_done(base))
return;
-   early_init_done = true;
  
  	writel(~0, base + TIMER0_VAL);

writel(~0, base + TIMER0_RELOAD);


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Re: [PATCH] timer: orion-timer: Fix problem with early static variable

[Tony Dinh]

Hi Stefan,

On Wed, Dec 21, 2022 at 1:18 AM Stefan Roese  wrote:
>
> We've noticed that at least one Kirkwood board (Pogo v4) has problems
> with the new orion DM timer implementation. Debugging revealed that this
> issue is related with the static variable "early_init_done" which does
> not work correctly before relocation in all cases.
>
> This patch removes this static variable and replaces it's functionality
> via a function that detects if the timer is already initialized.
>
> Signed-off-by: Stefan Roese 
> Cc: Pali Rohár 
> Cc: Michael Walle 
> Cc: Tony Dinh 
> ---
>  drivers/timer/orion-timer.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
> index d0eab3ce781d..6804bf0fa2cb 100644
> --- a/drivers/timer/orion-timer.c
> +++ b/drivers/timer/orion-timer.c
> @@ -23,15 +23,19 @@ struct orion_timer_priv {
>
>  #define MVEBU_TIMER_FIXED_RATE_25MHZ   2500
>
> -static bool early_init_done __section(".data") = false;
> +static bool early_init_done(void *base)
> +{
> +   if (readl(base + TIMER_CTRL) & TIMER0_EN)
> +   return true;
> +   return false;
> +}
>
>  /* Common functions for early (boot) and DM based timer */
>  static void orion_timer_init(void *base, enum input_clock_type type)
>  {
> /* Only init the timer once */
> -   if (early_init_done)
> +   if (early_init_done(base))
> return;
> -   early_init_done = true;
>
> writel(~0, base + TIMER0_VAL);
> writel(~0, base + TIMER0_RELOAD);
> --
> 2.39.0
>

I've tested with a couple Kirkwood boards, and they are working fine:
Pogo V4 (Kirkwood 88F6192) and Goflex Home (Kirkwood 88F6281). Note
that the Goflex Home does not have CONFIG_BOOTSTAGE (i.e. a regression
test, with a few sleep commands at the u-boot prompt).

Tested-by: Tony Dinh 

Thanks,
Tony

[PATCH] timer: orion-timer: Fix problem with early static variable

[Stefan Roese]

We've noticed that at least one Kirkwood board (Pogo v4) has problems
with the new orion DM timer implementation. Debugging revealed that this
issue is related with the static variable "early_init_done" which does
not work correctly before relocation in all cases.

This patch removes this static variable and replaces it's functionality
via a function that detects if the timer is already initialized.

Signed-off-by: Stefan Roese 
Cc: Pali Rohár 
Cc: Michael Walle 
Cc: Tony Dinh 
---
 drivers/timer/orion-timer.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index d0eab3ce781d..6804bf0fa2cb 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -23,15 +23,19 @@ struct orion_timer_priv {
 
 #define MVEBU_TIMER_FIXED_RATE_25MHZ   2500
 
-static bool early_init_done __section(".data") = false;
+static bool early_init_done(void *base)
+{
+   if (readl(base + TIMER_CTRL) & TIMER0_EN)
+   return true;
+   return false;
+}
 
 /* Common functions for early (boot) and DM based timer */
 static void orion_timer_init(void *base, enum input_clock_type type)
 {
/* Only init the timer once */
-   if (early_init_done)
+   if (early_init_done(base))
return;
-   early_init_done = true;
 
writel(~0, base + TIMER0_VAL);
writel(~0, base + TIMER0_RELOAD);
-- 
2.39.0