[U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file

[Marek Vasut]

On 11/27/19 4:09 PM, Claudius Heine wrote:
> Hi Marek,
> 
> On 27/11/2019 15.47, Marek Vasut wrote:
>> On 11/27/19 3:20 PM, Claudius Heine wrote:
>>> In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available
>>> anywere, even if SYSRESET is disabled for SPL in the board specific header
>>> file like this:
>>>
>>>     #if defined(CONFIG_SPL_BUILD)
>>>     #undef CONFIG_WDT
>>>     #undef CONFIG_WATCHDOG
>>>     #undef CONFIG_SYSRESET
>>>     #define CONFIG_HW_WATCHDOG
>>>     #endif
>>>
>>> 'do_reset' is called from SPL for instance from the panic handler in case
>>> SPL_USB_SDP is enabled and PANIC_HANG is not set.
>>>
>>> Setting PANIC_HANG would solve this issue, but it also changes the behavior
>>> in case a panic occurs.
>>>
>>> Signed-off-by: Claudius Heine <ch at denx.de>
>>> ---
>>>  arch/arm/lib/Makefile | 2 --
>>>  arch/arm/lib/reset.c  | 2 ++
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
>>> index 9de9a9acee..763eb4498f 100644
>>> --- a/arch/arm/lib/Makefile
>>> +++ b/arch/arm/lib/Makefile
>>> @@ -56,9 +56,7 @@ obj-y	+= interrupts_64.o
>>>  else
>>>  obj-y	+= interrupts.o
>>>  endif
>>> -ifndef CONFIG_SYSRESET
>>>  obj-y	+= reset.o
>>> -endif
>>>  
>>>  obj-y	+= cache.o
>>>  obj-$(CONFIG_SYS_ARM_CACHE_CP15)	+= cache-cp15.o
>>> diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
>>> index f3ea116e87..11e680be1d 100644
>>> --- a/arch/arm/lib/reset.c
>>> +++ b/arch/arm/lib/reset.c
>>> @@ -22,6 +22,7 @@
>>>  
>>>  #include <common.h>
>>>  
>>> +#if !defined(CONFIG_SYSRESET)
>>>  __weak void reset_misc(void)
>>>  {
>>>  }
>>> @@ -40,3 +41,4 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>>  	/*NOTREACHED*/
>>>  	return 0;
>>>  }
>>> +#endif
>>
>> Does this mean there's now one huge ifdef around the entire source file?
>> That's odd.
> 
> Right. Other suggestions?
> 
> Maybe having 'do_reset' here as a weak instead, so that sysreset can
> overwrite it? But then the other definitions in arch/*/lib/reset.c
> should probably be the same for consistency sake?
> 
> I tried with this patch not to change anything in case SYSRESET is
> enabled too much and since the file isn't too large I thought that would
> be ok for now.

What if sysreset implemented do_reset ? Wouldn't that solve the issue ?