[PATCH 3/4 v5] watchdog: mpc8xx_wdt: Watchdog driver and macros cleanup

Christophe Leroy christophe.leroy at c-s.fr
Thu Feb 20 07:50:22 CET 2020



On 02/20/2020 05:37 AM, Stefan Roese wrote:
> Hi Christophe,
> 
> On 19.02.20 20:15, Christophe Leroy wrote:
>> Hi Stefan,
>>
>> On 04/25/2019 07:17 AM, Stefan Roese wrote:
>>> With the generic watchdog driver now implemented, this patch removes
>>> some legacy stuff from the MPC8xx watchdog driver and its Kconfig
>>> integration. CONFIG_MPC8xx_WATCHDOG is completely removed and
>>> hw_watchdog_reset() is made static, as the watchdog will now get
>>> serviced via the DM infrastructure if enabled via CONFIG_WATCHDOG.
>>>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Christophe Leroy <christophe.leroy at c-s.fr>
>>> ---
>>> v5:
>>> - No change
>>>
>>> v4:
>>> - New patch
>>>
>>> arch/powerpc/Kconfig            | 2 +-
>>>   arch/powerpc/cpu/mpc8xx/Kconfig | 6 +++---
>>>   drivers/watchdog/Kconfig        | 1 -
>>>   drivers/watchdog/Makefile       | 2 +-
>>>   drivers/watchdog/mpc8xx_wdt.c   | 4 +---
>>>   5 files changed, 6 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index c727d9162c..0b1629ba62 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -35,7 +35,7 @@ config MPC8xx
>>>       bool "MPC8xx"
>>>       select BOARD_EARLY_INIT_F
>>>       imply CMD_REGINFO
>>> -    imply MPC8xx_WATCHDOG
>>> +    imply WDT_MPC8xx
>>>   endchoice
>>> diff --git a/arch/powerpc/cpu/mpc8xx/Kconfig 
>>> b/arch/powerpc/cpu/mpc8xx/Kconfig
>>> index b0e90a0f20..3e8ea38529 100644
>>> --- a/arch/powerpc/cpu/mpc8xx/Kconfig
>>> +++ b/arch/powerpc/cpu/mpc8xx/Kconfig
>>> @@ -25,9 +25,9 @@ config MPC885
>>>   endchoice
>>> -config MPC8xx_WATCHDOG
>>> -    bool "Watchdog"
>>> -    select HW_WATCHDOG
>>> +#config MPC8xx_WATCHDOG
>>> +#    bool "Watchdog"
>>> +#    select HW_WATCHDOG
>>
>> If HW_WATCHDOG is not selected, we have a problem in cpu_init_f() 
>> (arch/powerpc/cpu/mpc8xx/cpu_init.c) with the following line, and the 
>> board with restart every second.
>>
>> #ifndef CONFIG_HW_WATCHDOG
>>      /* deactivate watchdog if not enabled in config */
>>      out_be32(&immr->im_siu_conf.sc_sypcr, CONFIG_SYS_SYPCR & 
>> ~SYPCR_SWE);
>> #endif
>>
>> Should this be changed to use CONFIG_WATCHDOG instead ?
> 
> Yes, that sound reasonable.
> 

Looking at it more closely, I think there is something going wrong.

watchdog_reset() in wdt-uclass.c bails out without pinging the watchdog 
until GD_FLG_WDT_READY is set. But this is set in initr_watchdog().
It means that all WATCHDOG_RESET() until then are now ignored. This 
means all the phase during which Uboot execute from firmware until 
relocation do not service the watchdog anymore. The watchdog is ON since 
the CPU reset and must be serviced until someone decides to disable it, 
in which case it can't be re-enabled later. This means we can't disable 
it early to re-enable it once DM is ready.

And if I understand correctly doc/README.watchdog, that's exactly what 
CONFIG_HW_WATCHDOG is made for.

So I think that we just can't switch mpc8xx to CONFIG_WATCHDOG 
(include/wdt.c clears makes the difference between Hardware watchdogs 
and software watchdogs), it must remain with CONFIG_HW_WATCHDOG

Christophe




More information about the U-Boot mailing list