[U-Boot] [PATCH 1/2] watchdog: Split WDT from SPL_WDT

[Marek Vasut]

On 6/6/19 10:16 AM, Lukasz Majewski wrote:
> On Thu, 6 Jun 2019 10:08:04 +0200
> Marek Vasut <marex at denx.de> wrote:
> 
>> On 6/6/19 10:00 AM, Lukasz Majewski wrote:
>>> On Thu, 6 Jun 2019 09:26:04 +0200
>>> Marek Vasut <marex at denx.de> wrote:
>>>   
>>>> On 6/6/19 9:09 AM, Lukasz Majewski wrote:  
>>>>> On Wed, 5 Jun 2019 18:24:11 +0200
>>>>> Lukasz Majewski <lukma at denx.de> wrote:
>>>>>     
>>>>>> Hi Marek,
>>>>>>    
>>>>>>> On Sun, 12 May 2019 23:34:52 +0200
>>>>>>> Marek Vasut <marex at denx.de> wrote:
>>>>>>>       
>>>>>>>> Use CONFIG_IS_ENABLED(WDT) to permit use of WDT in SPL without
>>>>>>>> DM, while the full U-Boot can use rich DM/DT WDT driver.
>>>>>>>>
>>>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>>>>>> Cc: Peng Fan <Peng.Fan at freescale.com>
>>>>>>>> Cc: Stefano Babic <sbabic at denx.de>
>>>>>>>> ---
>>>>>>>>  common/board_r.c                  | 2 +-
>>>>>>>>  common/spl/spl.c                  | 2 +-
>>>>>>>>  drivers/watchdog/Makefile         | 2 +-
>>>>>>>>  include/asm-generic/global_data.h | 2 +-
>>>>>>>>  include/wdt.h                     | 2 +-
>>>>>>>>  5 files changed, 5 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/common/board_r.c b/common/board_r.c
>>>>>>>> index 150e8cd424..988e40abb2 100644
>>>>>>>> --- a/common/board_r.c
>>>>>>>> +++ b/common/board_r.c
>>>>>>>> @@ -678,7 +678,7 @@ static init_fnc_t init_sequence_r[] = {
>>>>>>>>  #ifdef CONFIG_DM
>>>>>>>>  	initr_dm,
>>>>>>>>  #endif
>>>>>>>> -#if defined(CONFIG_WDT)
>>>>>>>> +#if CONFIG_IS_ENABLED(WDT)
>>>>>>>>  	initr_watchdog,
>>>>>>>>  #endif
>>>>>>>>  #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) ||
>>>>>>>> defined(CONFIG_RISCV) || \ diff --git a/common/spl/spl.c
>>>>>>>> b/common/spl/spl.c index 0a6a47c202..f22f854718 100644
>>>>>>>> --- a/common/spl/spl.c
>>>>>>>> +++ b/common/spl/spl.c
>>>>>>>> @@ -601,7 +601,7 @@ void board_init_r(gd_t *dummy1, ulong
>>>>>>>> dummy2) spl_board_init();
>>>>>>>>  #endif
>>>>>>>>  
>>>>>>>> -#if defined(CONFIG_SPL_WATCHDOG_SUPPORT) &&
>>>>>>>> defined(CONFIG_WDT) +#if defined(CONFIG_SPL_WATCHDOG_SUPPORT)
>>>>>>>> && CONFIG_IS_ENABLED(WDT) initr_watchdog();
>>>>>>>>  #endif
>>>>>>>>  
>>>>>>>> diff --git a/drivers/watchdog/Makefile
>>>>>>>> b/drivers/watchdog/Makefile index 40b2f4bc66..4b94ae988c 100644
>>>>>>>> --- a/drivers/watchdog/Makefile
>>>>>>>> +++ b/drivers/watchdog/Makefile
>>>>>>>> @@ -16,7 +16,7 @@ obj-$(CONFIG_OMAP_WATCHDOG) += omap_wdt.o
>>>>>>>>  obj-$(CONFIG_DESIGNWARE_WATCHDOG) += designware_wdt.o
>>>>>>>>  obj-$(CONFIG_TANGIER_WATCHDOG) += tangier_wdt.o
>>>>>>>>  obj-$(CONFIG_ULP_WATCHDOG) += ulp_wdog.o
>>>>>>>> -obj-$(CONFIG_WDT) += wdt-uclass.o
>>>>>>>> +obj-$(CONFIG_$(SPL_TPL_)WDT) += wdt-uclass.o
>>>>>>>>  obj-$(CONFIG_WDT_SANDBOX) += sandbox_wdt.o
>>>>>>>>  obj-$(CONFIG_WDT_ARMADA_37XX) += armada-37xx-wdt.o
>>>>>>>>  obj-$(CONFIG_WDT_ASPEED) += ast_wdt.o
>>>>>>>> diff --git a/include/asm-generic/global_data.h
>>>>>>>> b/include/asm-generic/global_data.h index
>>>>>>>> 02a3ed6838..7c2220643b 100644 ---
>>>>>>>> a/include/asm-generic/global_data.h +++
>>>>>>>> b/include/asm-generic/global_data.h @@ -137,7 +137,7 @@
>>>>>>>> typedef struct global_data { #if
>>>>>>>> defined(CONFIG_TRANSLATION_OFFSET) fdt_addr_t
>>>>>>>> translation_offset;	/* optional translation offset */
>>>>>>>> #endif -#if defined(CONFIG_WDT)
>>>>>>>> +#if CONFIG_IS_ENABLED(WDT)
>>>>>>>>  	struct udevice *watchdog_dev;
>>>>>>>>  #endif
>>>>>>>>  } gd_t;
>>>>>>>> diff --git a/include/wdt.h b/include/wdt.h
>>>>>>>> index aa77d3e9b4..5bcff24ab3 100644
>>>>>>>> --- a/include/wdt.h
>>>>>>>> +++ b/include/wdt.h
>>>>>>>> @@ -106,7 +106,7 @@ struct wdt_ops {
>>>>>>>>  	int (*expire_now)(struct udevice *dev, ulong flags);
>>>>>>>>  };
>>>>>>>>  
>>>>>>>> -#if defined(CONFIG_WDT)
>>>>>>>> +#if CONFIG_IS_ENABLED(WDT)
>>>>>>>>  #ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS
>>>>>>>>  #define CONFIG_WATCHDOG_TIMEOUT_MSECS	(60 * 1000)
>>>>>>>>  #endif        
>>>>>>>
>>>>>>> Tested-by: Lukasz Majewski <lukma at denx.de>
>>>>>>>
>>>>>>> Test HW: display5 i.MX6Q device 
>>>>>>>       
>>>>>>
>>>>>> Unfortunately this series causes build break when run on
>>>>>> Travis-CI for some Atmel/Microchip:
>>>>>>
>>>>>> arm:  +   picosam9g45
>>>>>> +lib/built-in.o: In function `udelay':
>>>>>> +lib/time.c:167: undefined reference to `watchdog_reset'
>>>>>> +drivers/built-in.o: In function `atmel_serial_getc':
>>>>>> +drivers/serial/atmel_usart.c:103: undefined reference to
>>>>>>        `watchdog_reset' +make[2]: *** [spl/u-boot-spl] Error 1
>>>>>> +make[1]: *** [spl/u-boot-spl] Error 2
>>>>>> +make: *** [sub-make] Error 2
>>>>>>    14   38    1 /53     axm
>>>>>> boards.cfg is up to date. Nothing to do.
>>>>>> Summary of current source for 53 boards (2 threads, 1 job per
>>>>>> thread)
>>>>>>
>>>>>>
>>>>>> u-boot master SHA1: e1a2ed7180adeefb6164239a18249dca5701319d
>>>>>>    
>>>>>
>>>>> The fix is under Travis-CI testing:
>>>>> https://travis-ci.org/lmajewski/u-boot-dfu/builds/542118403
>>>>>
>>>>> I will post patches when it finishes.    
>>>>
>>>> What's the fix ?
>>>>  
>>>
>>> https://github.com/lmajewski/u-boot-dfu/commit/98c95790fee02296023a2906811092a1868f622d
>>>
>>> I've explicitly enabled SPL_WDT=y for affected boards.  
>>
>> I don't think that's the right approach, it should be possible to do
>> without explicit board config modification. I'm building another take,
>> since some boards still failed to build.
>>
> 
> Just to be clear - if it works with imply - I'm for this solution.
> 
> However, if it breaks, then we may need to find another solution (as
> for example shown here:
> https://github.com/lmajewski/u-boot-dfu/commit/98c95790fee02296023a2906811092a1868f622d
>  - the number of affected boards is small - just 3 of them).

I would very much prefer to make it work without defconfig
modifications. This has to be possible, since it worked before the split.

-- 
Best regards,
Marek Vasut