[PATCH] watchdog: Fix watchdog enablement in SPL and TPL

[Marek Vasut]

On 9/2/21 9:49 AM, Stefan Roese wrote:
> On 02.09.21 09:30, Marcel Ziswiler wrote:
>> On Tue, 2021-08-31 at 11:17 +0200, Marek Vasut wrote:
>>> On 8/31/21 8:49 AM, Marcel Ziswiler wrote:
>>>> Hi Marek
>>>>
>>>> On Tue, 2021-08-31 at 00:03 +0200, Marek Vasut wrote:
>>>>> Commit 830d29ac372 ("watchdog: Allow to use CONFIG_WDT without 
>>>>> starting watchdog")
>>>>> completely broke WDT operation in both SPL and TPL, in either case 
>>>>> those
>>>>> WDTs are never enabled. Fix it by filling in the missing Kconfig 
>>>>> options
>>>>> for SPL and TPL.
>>>>>
>>>>> Fixes: 830d29ac372 ("watchdog: Allow to use CONFIG_WDT without 
>>>>> starting watchdog")
>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>> Cc: Pali Rohar <pali at kernel.org>
>>>>> Cc: Stefan Roese <sr at denx.de>
>>>>> ---
>>>>>    drivers/watchdog/Kconfig | 20 ++++++++++++++++++++
>>>>>    1 file changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>> index f0ff2612a6b..65d974c4dd5 100644
>>>>> --- a/drivers/watchdog/Kconfig
>>>>> +++ b/drivers/watchdog/Kconfig
>>>>> @@ -273,4 +273,24 @@ config SPL_WDT
>>>>>             Enable driver model for watchdog timer in SPL.
>>>>>             This is similar to CONFIG_WDT in U-Boot.
>>>>> +config SPL_WATCHDOG_AUTOSTART
>>>>> +       bool "Automatically start watchdog timer in SPL"
>>>>> +       depends on SPL && WDT
>>>>> +       default y
>>>>> +       help
>>>>> +         Automatically start watchdog timer and start servicing it 
>>>>> during
>>>>> +         SPL phase. Enabled by default. Disable this option if you 
>>>>> want
>>>>> +         to compile U-Boot with CONFIG_WDT support but do not want to
>>>>> +         activate watchdog, like when CONFIG_WDT option is disabled.
>>>>> +
>>>>> +config TPL_WATCHDOG_AUTOSTART
>>>>> +       bool "Automatically start watchdog timer in TPL"
>>>>> +       depends on TPL && WDT
>>>>> +       default y
>>>>> +       help
>>>>> +         Automatically start watchdog timer and start servicing it 
>>>>> during
>>>>> +         TPL phase. Enabled by default. Disable this option if you 
>>>>> want
>>>>> +         to compile U-Boot with CONFIG_WDT support but do not want to
>>>>> +         activate watchdog, like when CONFIG_WDT option is disabled.
>>>>> +
>>>>>    endmenu
>>>>
>>>> Those Kconfig entries look fine. However, I am wondering where 
>>>> exactly they get used. Am I missing
>>>> anything?
>>>
>>> Have a look at the patch this Fixes:, if those Kconfig entries are not
>>> present, the WDT is disabled in SPL and TPL unconditionally.
>>
>> Yes, I did. But I guess I was not aware of how exactly that
>> CONFIG_IS_ENABLED(WATCHDOG_AUTOSTART) behaves.
>> While I have not really found this documented or the implementation
>> thereof I assume it pre-fixes config stuff
>> with SPL_ resp. TPL_ when building those flavors, correct?
> 
> Yes.
> 
>> I am wondering how many others are unaware of this (just like the
>> author of the patch this fixes). Maybe we
>> should at least document this properly somewhere, not?
> 
> If this is not the case (I did not check this), then yes. A
> documentation for these CONFIG_IS_ENABLED() and IS_ENABLED()
> helpers would be very good.

You can possibly even auto-lint for 
CONFIG_IS_ENABLED(...)/IS_ENABLED(...) usage that is missing the Kconfig 
entries and warn about that, to prevent patches that break SPL/TPL .

>>> It could be that if you're using some non-free or proprietary preloader
>>> instead of SPL, you won't run into this problem.
>>
>> No, don't worry. I am not running anything evil (;-p).
> 
> You won't run into any issues with this problem, as it is fixed in
> a different way in master already, as Rasmus already pointed out:
> 
> https://lore.kernel.org/u-boot/f015f55e-3225-655e-2e8b-672e058a8ae5@denx.de/ 
> 
> 
> $ git describe --contains 5fc094351381c4254098a25404d8712324b6918e
> v2021.10-rc1~19^2
> 
> commit 5fc094351381c4254098a25404d8712324b6918e
> Author: Teresa Remmet <t.remmet at phytec.de>
> Date:   Thu Jul 15 13:26:32 2021 +0200

Yes.