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

[Claudius Heine]

On 27/11/2019 16.12, Marek Vasut wrote:
> 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 ?

Not sure what you mean... sysreset implements do_reset:

https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/sysreset/sysreset-uclass.c#L112

But the SPL does not have sysreset in this case, so it needs something
different.

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch at denx.de

           PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
                             Keyserver: hkp://pool.sks-keyservers.net