On 27/11/2019 16.21, Marek Vasut wrote:
On 11/27/19 4:17 PM, Claudius Heine wrote:
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@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-u...
But the SPL does not have sysreset in this case, so it needs something different.
Oh, so you need CONFIG_$(SPL_TPL_)SYSRESET then ?
Well that would probably not enough. I would also need settings for the watchdog, because the SPL does not have DM support, so while u-boot uses CONFIG_WATCHDOG the SPL uses CONFIG_HW_WATCHDOG.
Easier that changing all this is something like this in the board header file (as I described in the commit description):
#if defined(CONFIG_SPL_BUILD) #undef CONFIG_WDT #undef CONFIG_WATCHDOG #undef CONFIG_SYSRESET #define CONFIG_HW_WATCHDOG #endif
In case of imx6, that way the SPL uses the hw_watchdog_reset from the imx watchdog driver instead of the 'watchdog_reset'.
'watchdog_reset' is not available since that is implemented in wdt-uclass.c and CONFIG_SPL_WDT depends on SPL_DM.