| Message ID | 20230116153445.145580-1-sr@denx.de |
|---|---|
| State | Accepted |
| Commit | 9a13a76e6256c51d04f41139733dbb31755e8d30 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | timer: orion-timer: Fix problem in early_init_done() | expand |
On Monday 16 January 2023 16:34:45 Stefan Roese wrote: > It was noticed that Clearfog is currently broken with this newly > introduced early_init_done() function. Apparently the timer is enabled > here when U-Boot is run but not configured - at least not correctly. > Resulting in a hangup in the timer reading functions. > > To fix this, also read the value of the reload register and check it's > value with the one written to by U-Boot. Only if this matches, the > init has already been done. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Martin Rowe <martin.p.rowe@gmail.com> > Cc: Tony Dinh <mibodhi@gmail.com> > Cc: Pali Rohár <pali@kernel.org> > Cc: Michael Walle <michael@walle.cc> Looks good. Acked-by: Pali Rohár <pali@kernel.org> Maybe you should also fixed line: Fixes: 5387b093cb79 ("timer: orion-timer: Fix problem with early static variable") > --- > drivers/timer/orion-timer.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c > index 810a03d54960..9cab27f2e48b 100644 > --- a/drivers/timer/orion-timer.c > +++ b/drivers/timer/orion-timer.c > @@ -25,7 +25,8 @@ struct orion_timer_priv { > > static bool early_init_done(void *base) > { > - if (readl(base + TIMER_CTRL) & TIMER0_EN) > + if ((readl(base + TIMER_CTRL) & TIMER0_EN) && > + (readl(base + TIMER0_RELOAD) == ~0)) > return true; > return false; > } > -- > 2.39.0 >
Hi Pali, On 1/16/23 18:54, Pali Rohár wrote: > On Monday 16 January 2023 16:34:45 Stefan Roese wrote: >> It was noticed that Clearfog is currently broken with this newly >> introduced early_init_done() function. Apparently the timer is enabled >> here when U-Boot is run but not configured - at least not correctly. >> Resulting in a hangup in the timer reading functions. >> >> To fix this, also read the value of the reload register and check it's >> value with the one written to by U-Boot. Only if this matches, the >> init has already been done. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Martin Rowe <martin.p.rowe@gmail.com> >> Cc: Tony Dinh <mibodhi@gmail.com> >> Cc: Pali Rohár <pali@kernel.org> >> Cc: Michael Walle <michael@walle.cc> > > Looks good. > > Acked-by: Pali Rohár <pali@kernel.org> > > Maybe you should also fixed line: > > Fixes: 5387b093cb79 ("timer: orion-timer: Fix problem with early static variable") Will do when applying. Thanks. Thanks, Stefan >> --- >> drivers/timer/orion-timer.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c >> index 810a03d54960..9cab27f2e48b 100644 >> --- a/drivers/timer/orion-timer.c >> +++ b/drivers/timer/orion-timer.c >> @@ -25,7 +25,8 @@ struct orion_timer_priv { >> >> static bool early_init_done(void *base) >> { >> - if (readl(base + TIMER_CTRL) & TIMER0_EN) >> + if ((readl(base + TIMER_CTRL) & TIMER0_EN) && >> + (readl(base + TIMER0_RELOAD) == ~0)) >> return true; >> return false; >> } >> -- >> 2.39.0 >> Viele Grüße, Stefan Roese
On 1/16/23 18:54, Pali Rohár wrote: > On Monday 16 January 2023 16:34:45 Stefan Roese wrote: >> It was noticed that Clearfog is currently broken with this newly >> introduced early_init_done() function. Apparently the timer is enabled >> here when U-Boot is run but not configured - at least not correctly. >> Resulting in a hangup in the timer reading functions. >> >> To fix this, also read the value of the reload register and check it's >> value with the one written to by U-Boot. Only if this matches, the >> init has already been done. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Martin Rowe <martin.p.rowe@gmail.com> >> Cc: Tony Dinh <mibodhi@gmail.com> >> Cc: Pali Rohár <pali@kernel.org> >> Cc: Michael Walle <michael@walle.cc> > > Looks good. > > Acked-by: Pali Rohár <pali@kernel.org> > > Maybe you should also fixed line: > > Fixes: 5387b093cb79 ("timer: orion-timer: Fix problem with early static variable") Applied to u-boot-marvell/master Thanks, Stefan >> --- >> drivers/timer/orion-timer.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c >> index 810a03d54960..9cab27f2e48b 100644 >> --- a/drivers/timer/orion-timer.c >> +++ b/drivers/timer/orion-timer.c >> @@ -25,7 +25,8 @@ struct orion_timer_priv { >> >> static bool early_init_done(void *base) >> { >> - if (readl(base + TIMER_CTRL) & TIMER0_EN) >> + if ((readl(base + TIMER_CTRL) & TIMER0_EN) && >> + (readl(base + TIMER0_RELOAD) == ~0)) >> return true; >> return false; >> } >> -- >> 2.39.0 >> Viele Grüße, Stefan Roese
diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c index 810a03d54960..9cab27f2e48b 100644 --- a/drivers/timer/orion-timer.c +++ b/drivers/timer/orion-timer.c @@ -25,7 +25,8 @@ struct orion_timer_priv { static bool early_init_done(void *base) { - if (readl(base + TIMER_CTRL) & TIMER0_EN) + if ((readl(base + TIMER_CTRL) & TIMER0_EN) && + (readl(base + TIMER0_RELOAD) == ~0)) return true; return false; }
It was noticed that Clearfog is currently broken with this newly introduced early_init_done() function. Apparently the timer is enabled here when U-Boot is run but not configured - at least not correctly. Resulting in a hangup in the timer reading functions. To fix this, also read the value of the reload register and check it's value with the one written to by U-Boot. Only if this matches, the init has already been done. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Martin Rowe <martin.p.rowe@gmail.com> Cc: Tony Dinh <mibodhi@gmail.com> Cc: Pali Rohár <pali@kernel.org> Cc: Michael Walle <michael@walle.cc> --- drivers/timer/orion-timer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)