[PATCH] powerpc: remove WATCHDOG_RESET call from wait_ticks()

Wolfgang Denk wd at denx.de
Tue Mar 17 15:31:28 CET 2020


Dear Rasmus,

In message <01193803-be8f-865d-5fce-2c7cee0fd9af at prevas.dk> you wrote:
>
> >> An MPC8309-derived board which does not utilize the SOCs watchdog but
> >> has an external gpio-triggered (always-running) watchdog circuit.
> > 
> > This is not even close to global coverage.
>
> No, of course not. Shall I list all __udelay() calls in the tree and
> exclude the ones that are in board-specific code for non-ppc boards? I'm
> pretty sure that leaves a very small handful. I can do that.

No, I mean there is a pretty large numbe rof different processor
families out there which were not covered by any of your tests.
I think I still know a bit of the Power Architecture, but I would
not dare to guarantee that your suggested change does not break any
of these.  Testing on a single processor / family is definitely not
good enoug to give any confidence.

> > Are you aware that there is the PPC-global implementation of
> > wait_ticks() in arch/powerpc/lib/ticks.S , plus another MPC83xx
> > specific one in drivers/timer/mpc83xx_timer.c?
>
> Yes, that latter one doesn't even compile in the presence of
> CONFIG_WATCHDOG or CONFIG_HW_WATCHDOG.

Maybe you focus on cleaning up this first?

> It doesn't. I don't understand why powerpc's __udelay needs to be
> different from all other architectures'. This is not really about the

You misunderstand.  The Power Architecture has been the first ever
running U-Boot.  In the beginning, two decades back, the project was
not called U-Boot but PPCBoot - guess why.

So you should rephrase your question into: why did all other
architectures deviate from the reference implementation?

> quirks of ppc SOCs' watchdogs or not, it's about providing a __udelay
> primitive that generic code can rely on has certain properties, and in
> particular, that watchdog drivers can use without risking being called
> recursively.

If any such recursive calls happen, then this is a bug in their
implementation, isn't it?  And it should be trivial to detect and to
fix.

> The point is, we're being told that everything is moving to DM and
> better convert your board or else..., and nowadays CONFIG_WDT comes with
> it's own watchdog_reset() which is a rather more complicated beast than
> the board-specific ones that used to be sprinkled throughout (and
> out-of) the tree. So yes, for the past 18 years, nothing bad has
> probably come from doing a WATCHDOG_RESET even deep down in the guts of
> arch-specific primitives.

You certainly have a point here.  But when you try to improve any
code, the first thing you must do is to guarantee it continues to
work on all affected systems.  I don't dare to give even a prognosis
before testing this on a number of different hardware configurations.

Testing on a single platform (which apparently has aother problems,
or you would not need any such change) is not convincing.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"A dirty mind is a joy forever."                       - Randy Kunkee


More information about the U-Boot mailing list