[PATCH] powerpc: remove WATCHDOG_RESET call from wait_ticks()
rasmus.villemoes at prevas.dk
Tue Mar 17 16:23:52 CET 2020
On 17/03/2020 15.31, Wolfgang Denk wrote:
> Dear Rasmus,
> In message <01193803-be8f-865d-5fce-2c7cee0fd9af at prevas.dk> you wrote:
>>> 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?
I don't have much luck getting bugs in the various mpc8xxx/mpc83xx
drivers fixed. As long as the gpio and spi drivers that I do use and
tried to be a good citizen and sent patches for upstream are still
buggy, I don't really feel like fixing a driver that I don't use.
>> 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
> 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
Huh? Suppose hypothetically somebody implements a driver for an external
watchdog circuit connected to a gpio. The data sheet says the reset
sequence consists of setting the gpio high, waiting at least two
microseconds, then setting the gpio low. That's probably implemented as
Now, that works perfectly on ARM, mips, whatnot. Then some day somebody
is handed a ppc-based board with such a watchdog. Obviously that driver
doesn't work for them. Is that a bug in the original implementation?
>> 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.
I don't, actually, need this change, but suggested it as a way towards
making the primitives available a little more consistent across
architectures since I stumbled on this implementation detail while
single-stepping my board to figure out why it would reset in the SPL. As
I'm unable to convince you that the benefits of that outweigh the risk
of introducing a regression, feel free to drop it.
I do, however, need the other ppc patch I sent yesterday:
https://patchwork.ozlabs.org/patch/1255844/ . That one should satisfy
the requirement that it cannot possibly break anything for existing boards.
More information about the U-Boot