[U-Boot] [PATCH 04/11] watchdog: Handle SPL build with watchdog disabled

[Eugen.Hristev at microchip.com]

On 20.03.2019 09:48, Stefan Roese wrote:
> External E-Mail
> 
> 
> On 20.03.19 08:41, Eugen.Hristev at microchip.com wrote:
>>
>>
>> On 20.03.2019 09:33, Stefan Roese wrote:
>>> External E-Mail
>>>
>>>
>>> On 20.03.19 08:30, Eugen.Hristev at microchip.com wrote:
>>>>
>>>>
>>>> On 19.03.2019 17:56, Stefan Roese wrote:
>>>>> External E-Mail
>>>>>
>>>>>
>>>>> This patch adds some checks, so that the watchdog can be enabled in 
>>>>> main
>>>>> U-Boot proper but can be disabled in SPL.
>>>>
>>>> Hi Stefan,
>>>>
>>>> Actually your code looks at CONFIG_SPL_WATCHDOG_SUPPORT , so , if this
>>>> is disabled in the config, you say that the watchdog was still enabled?
>>>> (thus broken CONFIG_SPL_WATCHDOG_SUPPORT ?)
>>>
>>> Yes, in my case here, the watchdog is disabled in SPL and enabled in
>>> main U-Boot proper. This use case is what this patch fixes.
>>>
>>> Is this still unclear? Sorry, I didn't fully understand your question.
>>
>> There is a Kconfig named CONFIG_SPL_WATCHDOG_SUPPORT
>> If this is y, then the watchdog support should be included in SPL
>> If this is n, then the watchdog support should not be included in SPL.
>>
>> Considering your use case, you want CONFIG_SPL_WATCHDOG_SUPPORT=n
> 
> Correct.
> 
>> Configuring this, the watchdog is still enabled in SPL?
> 
> I don't want the U-Boot SPL support enabled (mainly because of code
> size). AFAIU, the AT91SAM watchdog is enabled by default. So its
> enabled in the SoC when SPL is running, as I don't want to disable
> it (as done e.g. in AT91Bootrap if configured this way).
> 
>> So my question: is the behavior of CONFIG_SPL_WATCHDOG_SUPPORT=n not
>> aligned with your use case ? So you are actually fixing the behavior of
>> CONFIG_SPL_WATCHDOG_SUPPORT=n ?
> 
> Yes. Without this patch I do get this build error:
> 
> ...
>    LD      spl/u-boot-spl
> lib/built-in.o: In function `udelay':
> /home/stefan/git/u-boot/u-boot/lib/time.c:167: undefined reference to 
> `watchdog_reset'
> drivers/built-in.o: In function `atmel_nand_pmecc_write_page':
> /home/stefan/git/u-boot/u-boot/drivers/mtd/nand/raw/atmel_nand.c:592: 
> undefined reference to `watchdog_reset'
> drivers/built-in.o: In function `atmel_nand_pmecc_read_page':
> /home/stefan/git/u-boot/u-boot/drivers/mtd/nand/raw/atmel_nand.c:552: 
> undefined reference to `watchdog_reset'
> drivers/built-in.o: In function `pmecc_err_location':
> /home/stefan/git/u-boot/u-boot/drivers/mtd/nand/raw/atmel_nand.c:416: 
> undefined reference to `watchdog_reset'
> scripts/Makefile.spl:384: recipe for target 'spl/u-boot-spl' failed
> make[1]: *** [spl/u-boot-spl] Error 1
> Makefile:1651: recipe for target 'spl/u-boot-spl' failed
> make: *** [spl/u-boot-spl] Error 2
> 

OK so that's what I want to settle: you are actually fixing the issue of 
CONFIG_SPL_WATCHDOG_SUPPORT=n not working properly

Also, we may have to look inside the atmel_nand.c as it may be affected?

Thanks,
Eugen

> Thanks,
> Stefan
> 
>>
>>> Thanks,
>>> Stefan
>>>
>>>> Eugen
>>>>
>>>>>
>>>>> This will be used by some AT91SAM based boards, which might enable the
>>>>> watchdog in the main U-Boot proper and not in SPL. It will be 
>>>>> enabled in
>>>>> SPL by default there, so no need to configure it there. This approach
>>>>> saves some space in SPL.
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>>> Cc: Heiko Schocher <hs at denx.de>
>>>>> Cc: Andreas Bießmann <andreas at biessmann.org>
>>>>> Cc: Eugen Hristev <eugen.hristev at microchip.com>
>>>>> ---
>>>>>     include/watchdog.h | 10 ++++++++--
>>>>>     1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/watchdog.h b/include/watchdog.h
>>>>> index 14073cfdd2..3a357de903 100644
>>>>> --- a/include/watchdog.h
>>>>> +++ b/include/watchdog.h
>>>>> @@ -51,9 +51,15 @@ int init_func_watchdog_reset(void);
>>>>>             #if defined(__ASSEMBLY__)
>>>>>                 #define WATCHDOG_RESET bl watchdog_reset
>>>>>             #else
>>>>> -            extern void watchdog_reset(void);
>>>>> +            /* Don't require the watchdog to be enabled in SPL */
>>>>> +            #if defined(CONFIG_SPL_BUILD) &&        \
>>>>> +                !defined(CONFIG_SPL_WATCHDOG_SUPPORT)
>>>>> +                #define WATCHDOG_RESET() {}
>>>>> +            #else
>>>>> +                extern void watchdog_reset(void);
>>>>> -            #define WATCHDOG_RESET watchdog_reset
>>>>> +                #define WATCHDOG_RESET watchdog_reset
>>>>> +            #endif
>>>>>             #endif
>>>>>         #else
>>>>>             /*
>>>>>
>>>
>>> Viele Grüße,
>>> Stefan
>>>
> 
> Viele Grüße,
> Stefan
>