[U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them

In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de --- arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif

On 11/28/19 9:56 AM, 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/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de
Reviewed-by: Marek Vasut marex@denx.de

On Thu, Nov 28, 2019 at 9:57 AM Claudius Heine ch@denx.de wrote:
In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET
Would it work to do this: obj-$(CONFIG_$(SPL_TPL_)SYSRESET) += reset.o
that would be nicer than ifndef, I think.
Regards, Simon
obj-y += reset.o endif
-- 2.21.0

On 28/11/2019 10.27, Simon Goldschmidt wrote:
On Thu, Nov 28, 2019 at 9:57 AM Claudius Heine ch@denx.de wrote:
In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET
Would it work to do this: obj-$(CONFIG_$(SPL_TPL_)SYSRESET) += reset.o
that would be nicer than ifndef, I think.
Yes it would, but it didn't seem to work.
With:
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9416369aad..913bb21eaf 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_(SPL_TPL_)SYSRESET -obj-y += reset.o -endif +obj-$(CONFIG_$(SPL_TPL_)SYSRESET) += reset.o
obj-y += cache.o obj-$(CONFIG_SYS_ARM_CACHE_CP15) += cache-cp15.o
I get with CONFIG_SYSRESET:
arm-linux-gnu-ld.bfd: drivers/built-in.o: in function `do_reset': drivers/sysreset/sysreset-uclass.c:113: multiple definition of `do_reset'; arch/arm/lib/built-in.o:arch/arm/lib/reset.c:30: first defined here make: *** [Makefile:1667: u-boot] Error 1
And without CONFIG_SYSRESET:
arm-linux-gnu-ld.bfd: lib/built-in.o: in function `efi_reset_system_boottime': lib/efi_loader/efi_runtime.c:165: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: lib/built-in.o: in function `panic_finish': lib/panic.c:26: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: arch/arm/mach-imx/built-in.o: in function `do_boot_mode': arch/arm/mach-imx/cmd_bmode.c:76: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: cmd/built-in.o:(.u_boot_list_2_cmd_2_reset+0xc): undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `jumptable_init': common/exports.c:30: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `print_resetinfo': common/board_f.c:167: undefined reference to `sysreset_get_status' arm-linux-gnu-ld.bfd: common/built-in.o: in function `do_bootm_states': common/bootm.c:633: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `run_usb_dnl_gadget': common/dfu.c:90: undefined reference to `do_reset' make: *** [Makefile:1667: u-boot] Error 1

On Thu, Nov 28, 2019 at 11:52 AM Claudius Heine ch@denx.de wrote:
On 28/11/2019 10.27, Simon Goldschmidt wrote:
On Thu, Nov 28, 2019 at 9:57 AM Claudius Heine ch@denx.de wrote:
In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET
Would it work to do this: obj-$(CONFIG_$(SPL_TPL_)SYSRESET) += reset.o
that would be nicer than ifndef, I think.
Yes it would, but it didn't seem to work.
OK, thanks for trying. Given the results below, it's fine for me, so:
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
With:
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9416369aad..913bb21eaf 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_(SPL_TPL_)SYSRESET -obj-y += reset.o -endif +obj-$(CONFIG_$(SPL_TPL_)SYSRESET) += reset.o
obj-y += cache.o obj-$(CONFIG_SYS_ARM_CACHE_CP15) += cache-cp15.o
I get with CONFIG_SYSRESET:
arm-linux-gnu-ld.bfd: drivers/built-in.o: in function `do_reset': drivers/sysreset/sysreset-uclass.c:113: multiple definition of `do_reset'; arch/arm/lib/built-in.o:arch/arm/lib/reset.c:30: first defined here make: *** [Makefile:1667: u-boot] Error 1
And without CONFIG_SYSRESET:
arm-linux-gnu-ld.bfd: lib/built-in.o: in function `efi_reset_system_boottime': lib/efi_loader/efi_runtime.c:165: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: lib/built-in.o: in function `panic_finish': lib/panic.c:26: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: arch/arm/mach-imx/built-in.o: in function `do_boot_mode': arch/arm/mach-imx/cmd_bmode.c:76: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: cmd/built-in.o:(.u_boot_list_2_cmd_2_reset+0xc): undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `jumptable_init': common/exports.c:30: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `print_resetinfo': common/board_f.c:167: undefined reference to `sysreset_get_status' arm-linux-gnu-ld.bfd: common/built-in.o: in function `do_bootm_states': common/bootm.c:633: undefined reference to `do_reset' arm-linux-gnu-ld.bfd: common/built-in.o: in function `run_usb_dnl_gadget': common/dfu.c:90: undefined reference to `do_reset' make: *** [Makefile:1667: u-boot] Error 1

On Thu, Nov 28, 2019 at 09:56:47AM +0100, 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/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de Reviewed-by: Marek Vasut marex@denx.de Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif
This needs to be updated and something reworked as it breaks imx8mp_evk imx8mn_ddr4_evk imx8mm_evk platforms that have since been added: board/freescale/imx8mm_evk/built-in.o: In function `do_reset': build/../board/freescale/imx8mm_evk/spl.c:164: multiple definition of `do_reset' and similar failures.

Hi,
On 10/01/2020 22.48, Tom Rini wrote:
On Thu, Nov 28, 2019 at 09:56:47AM +0100, 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/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de Reviewed-by: Marek Vasut marex@denx.de Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif
This needs to be updated and something reworked as it breaks imx8mp_evk imx8mn_ddr4_evk imx8mm_evk platforms that have since been added: board/freescale/imx8mm_evk/built-in.o: In function `do_reset': build/../board/freescale/imx8mm_evk/spl.c:164: multiple definition of `do_reset' and similar failures.
It seems the imx8mm_evk and imx8mn_evk are the first platforms that implement a 'do_reset' within its board files.
That means we then no longer have just two binary options where the 'do_reset' implementation originates from. Before that platform we only had the ARM cpu reset and the sysreset driver.
That means if 'CONFIG_$(SPL_TPL_)SYSRESET == n' we cannot automatically use the ARM platform reset.
We will probably need additional kconfig options to express this situation.
The question is, should we do that, or rather investigate why those platforms need their own implementation?
Is it not possible to use the sysreset or arm reset driver there?
regards, Claudius

On 10/01/2020 22.48, Tom Rini wrote:
On Thu, Nov 28, 2019 at 09:56:47AM +0100, 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/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de Reviewed-by: Marek Vasut marex@denx.de Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif
This needs to be updated and something reworked as it breaks imx8mp_evk imx8mn_ddr4_evk imx8mm_evk platforms that have since been added: board/freescale/imx8mm_evk/built-in.o: In function `do_reset': build/../board/freescale/imx8mm_evk/spl.c:164: multiple definition of `do_reset' and similar failures.
It seems the imx8mm_evk and imx8mn_evk are the first platforms that implement a 'do_reset' within its board files.
That means we then no longer have just two binary options where the 'do_reset' implementation originates from. Before that platform we only had the ARM cpu reset and the sysreset driver.
That means if 'CONFIG_$(SPL_TPL_)SYSRESET == n' we cannot automatically use the ARM platform reset.
We will probably need additional kconfig options to express this situation.
The question is, should we do that, or rather investigate why those platforms need their own implementation?
Is it not possible to use the sysreset or arm reset driver there?
Or PCSI via ATF?

Hi Peter,
On 14/01/2020 11.18, Peter Robinson wrote:
On 10/01/2020 22.48, Tom Rini wrote:
On Thu, Nov 28, 2019 at 09:56:47AM +0100, 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/TPL.
'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set
Signed-off-by: Claudius Heine ch@denx.de Reviewed-by: Marek Vasut marex@denx.de Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 9de9a9acee..7bf2c077ba 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -56,7 +56,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif
This needs to be updated and something reworked as it breaks imx8mp_evk imx8mn_ddr4_evk imx8mm_evk platforms that have since been added: board/freescale/imx8mm_evk/built-in.o: In function `do_reset': build/../board/freescale/imx8mm_evk/spl.c:164: multiple definition of `do_reset' and similar failures.
It seems the imx8mm_evk and imx8mn_evk are the first platforms that implement a 'do_reset' within its board files.
That means we then no longer have just two binary options where the 'do_reset' implementation originates from. Before that platform we only had the ARM cpu reset and the sysreset driver.
That means if 'CONFIG_$(SPL_TPL_)SYSRESET == n' we cannot automatically use the ARM platform reset.
We will probably need additional kconfig options to express this situation.
The question is, should we do that, or rather investigate why those platforms need their own implementation?
Is it not possible to use the sysreset or arm reset driver there?
Or PCSI via ATF?
Isn't that a sysreset driver?
Claudius

Hi,
I have only tested compiling, but if the reset in the SPL on i.MX8MM and i.MX8MN still works with this patch applied, then we don't need board specific 'do_reset' function and special configurations flags for this case.
I currently don't have access to the hardware to test this.
regards, Claudius
-- >8 -- Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
Instead of implementing a custom reset function for the SPL, it can directly use the sysreset DM driver.
Signed-off-by: Claudius Heine ch@denx.de --- arch/arm/mach-imx/imx8m/soc.c | 19 ------------------- board/freescale/imx8mm_evk/spl.c | 9 --------- board/freescale/imx8mn_evk/spl.c | 9 --------- configs/imx8mm_evk_defconfig | 1 + configs/imx8mn_ddr4_evk_defconfig | 1 + 5 files changed, 2 insertions(+), 37 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 5ce5a180e8..519108d4c9 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -378,22 +378,3 @@ int ft_system_setup(void *blob, bd_t *bd) return 0; } #endif - -#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET) -void reset_cpu(ulong addr) -{ - struct watchdog_regs *wdog = (struct watchdog_regs *)addr; - - if (!addr) - wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR; - - /* Clear WDA to trigger WDOG_B immediately */ - writew((WCR_WDE | WCR_SRS), &wdog->wcr); - - while (1) { - /* - * spin for .5 seconds before reset - */ - } -} -#endif diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c index 2d08f9a563..e8dec452ea 100644 --- a/board/freescale/imx8mm_evk/spl.c +++ b/board/freescale/imx8mm_evk/spl.c @@ -159,12 +159,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0); } - -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{ - puts ("resetting ...\n"); - - reset_cpu(WDOG1_BASE_ADDR); - - return 0; -} diff --git a/board/freescale/imx8mn_evk/spl.c b/board/freescale/imx8mn_evk/spl.c index cbde9f6b3c..0c70fbb155 100644 --- a/board/freescale/imx8mn_evk/spl.c +++ b/board/freescale/imx8mn_evk/spl.c @@ -112,12 +112,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0); } - -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{ - puts("resetting ...\n"); - - reset_cpu(WDOG1_BASE_ADDR); - - return 0; -} diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig index 87560ef989..c07f4104f9 100644 --- a/configs/imx8mm_evk_defconfig +++ b/configs/imx8mm_evk_defconfig @@ -83,5 +83,6 @@ CONFIG_DM_REGULATOR_FIXED=y CONFIG_DM_REGULATOR_GPIO=y CONFIG_MXC_UART=y CONFIG_SYSRESET=y +CONFIG_SPL_SYSRESET=y CONFIG_SYSRESET_PSCI=y CONFIG_DM_THERMAL=y diff --git a/configs/imx8mn_ddr4_evk_defconfig b/configs/imx8mn_ddr4_evk_defconfig index 50b03d0763..c55998da4c 100644 --- a/configs/imx8mn_ddr4_evk_defconfig +++ b/configs/imx8mn_ddr4_evk_defconfig @@ -75,5 +75,6 @@ CONFIG_DM_REGULATOR_FIXED=y CONFIG_DM_REGULATOR_GPIO=y CONFIG_MXC_UART=y CONFIG_SYSRESET=y +CONFIG_SPL_SYSRESET=y CONFIG_SYSRESET_PSCI=y CONFIG_DM_THERMAL=y

On Thu, 16 Jan 2020 at 03:50, Claudius Heine ch@denx.de wrote:
Hi,
I have only tested compiling, but if the reset in the SPL on i.MX8MM and i.MX8MN still works with this patch applied, then we don't need board specific 'do_reset' function and special configurations flags for this case.
I currently don't have access to the hardware to test this.
regards, Claudius
-- >8 -- Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
Instead of implementing a custom reset function for the SPL, it can directly use the sysreset DM driver.
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/mach-imx/imx8m/soc.c | 19 ------------------- board/freescale/imx8mm_evk/spl.c | 9 --------- board/freescale/imx8mn_evk/spl.c | 9 --------- configs/imx8mm_evk_defconfig | 1 + configs/imx8mn_ddr4_evk_defconfig | 1 + 5 files changed, 2 insertions(+), 37 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
NAK, this will not work on i.MX8MM/N.
Currently sysreset psci is enabled, however psci will not work for SPL, because SPL boots before BL31.
Regards, Peng.
Hi,
I have only tested compiling, but if the reset in the SPL on i.MX8MM and i.MX8MN still works with this patch applied, then we don't need board specific 'do_reset' function and special configurations flags for this case.
I currently don't have access to the hardware to test this.
regards, Claudius
-- >8 -- Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
Instead of implementing a custom reset function for the SPL, it can directly use the sysreset DM driver.
Signed-off-by: Claudius Heine ch@denx.de
arch/arm/mach-imx/imx8m/soc.c | 19 ------------------- board/freescale/imx8mm_evk/spl.c | 9 --------- board/freescale/imx8mn_evk/spl.c | 9 --------- configs/imx8mm_evk_defconfig | 1 + configs/imx8mn_ddr4_evk_defconfig | 1 + 5 files changed, 2 insertions(+), 37 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 5ce5a180e8..519108d4c9 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -378,22 +378,3 @@ int ft_system_setup(void *blob, bd_t *bd) return 0; } #endif
-#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET) -void reset_cpu(ulong addr) -{
struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
if (!addr)
wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
/* Clear WDA to trigger WDOG_B immediately */
writew((WCR_WDE | WCR_SRS), &wdog->wcr);
while (1) {
/*
* spin for .5 seconds before reset
*/
}
-} -#endif diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c index 2d08f9a563..e8dec452ea 100644 --- a/board/freescale/imx8mm_evk/spl.c +++ b/board/freescale/imx8mm_evk/spl.c @@ -159,12 +159,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0); }
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{
- puts ("resetting ...\n");
- reset_cpu(WDOG1_BASE_ADDR);
- return 0;
-} diff --git a/board/freescale/imx8mn_evk/spl.c b/board/freescale/imx8mn_evk/spl.c index cbde9f6b3c..0c70fbb155 100644 --- a/board/freescale/imx8mn_evk/spl.c +++ b/board/freescale/imx8mn_evk/spl.c @@ -112,12 +112,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0); }
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) -{
- puts("resetting ...\n");
- reset_cpu(WDOG1_BASE_ADDR);
- return 0;
-} diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig index 87560ef989..c07f4104f9 100644 --- a/configs/imx8mm_evk_defconfig +++ b/configs/imx8mm_evk_defconfig @@ -83,5 +83,6 @@ CONFIG_DM_REGULATOR_FIXED=y CONFIG_DM_REGULATOR_GPIO=y CONFIG_MXC_UART=y CONFIG_SYSRESET=y +CONFIG_SPL_SYSRESET=y CONFIG_SYSRESET_PSCI=y CONFIG_DM_THERMAL=y diff --git a/configs/imx8mn_ddr4_evk_defconfig b/configs/imx8mn_ddr4_evk_defconfig index 50b03d0763..c55998da4c 100644 --- a/configs/imx8mn_ddr4_evk_defconfig +++ b/configs/imx8mn_ddr4_evk_defconfig @@ -75,5 +75,6 @@ CONFIG_DM_REGULATOR_FIXED=y CONFIG_DM_REGULATOR_GPIO=y CONFIG_MXC_UART=y CONFIG_SYSRESET=y +CONFIG_SPL_SYSRESET=y CONFIG_SYSRESET_PSCI=y CONFIG_DM_THERMAL=y -- 2.24.1

On 1/16/20 3:21 AM, Peng Fan wrote:
Hello Peng,
Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
NAK, this will not work on i.MX8MM/N.
Currently sysreset psci is enabled, however psci will not work for SPL, because SPL boots before BL31.
Thank you for the constructive feedback.
So basically , what we need is a real sysreset driver for iMX8MM , which can work with bare-bones hardware interface, that is, poking some register, correct ?
So, either you or Claudius needs to implement a driver in drivers/sysreset, which will bind in SPL and if needed, do some writel() into some registers to reset the board, correct ?
Because, I believe we can both agree that dumping such ad-hoc code which implements reset in board code is not the right way, esp. nowadays with all the DM/DT stuff in.

Hi Marek,
Subject: Re: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
On 1/16/20 3:21 AM, Peng Fan wrote:
Hello Peng,
Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
NAK, this will not work on i.MX8MM/N.
Currently sysreset psci is enabled, however psci will not work for SPL, because SPL boots before BL31.
Thank you for the constructive feedback.
So basically , what we need is a real sysreset driver for iMX8MM , which can work with bare-bones hardware interface, that is, poking some register, correct ?
Yes.
So, either you or Claudius needs to implement a driver in drivers/sysreset, which will bind in SPL and if needed, do some writel() into some registers to reset the board, correct ?
Yes.
Because, I believe we can both agree that dumping such ad-hoc code which implements reset in board code is not the right way, esp. nowadays with all the DM/DT stuff in.
Alought we still have ocram space, but our SPL is huge now, 100KB+
Regards, Peng.

On 1/17/20 3:33 AM, Peng Fan wrote:
Hi Marek,
Hi,
Subject: Re: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
On 1/16/20 3:21 AM, Peng Fan wrote:
Hello Peng,
Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
NAK, this will not work on i.MX8MM/N.
Currently sysreset psci is enabled, however psci will not work for SPL, because SPL boots before BL31.
Thank you for the constructive feedback.
So basically , what we need is a real sysreset driver for iMX8MM , which can work with bare-bones hardware interface, that is, poking some register, correct ?
Yes.
So, either you or Claudius needs to implement a driver in drivers/sysreset, which will bind in SPL and if needed, do some writel() into some registers to reset the board, correct ?
Yes.
Because, I believe we can both agree that dumping such ad-hoc code which implements reset in board code is not the right way, esp. nowadays with all the DM/DT stuff in.
Alought we still have ocram space, but our SPL is huge now, 100KB+
How so ?
How much does DM sysreset add to that ?

Subject: Re: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
On 1/17/20 3:33 AM, Peng Fan wrote:
Hi Marek,
Hi,
Subject: Re: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
On 1/16/20 3:21 AM, Peng Fan wrote:
Hello Peng,
Subject: [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
NAK, this will not work on i.MX8MM/N.
Currently sysreset psci is enabled, however psci will not work for SPL, because SPL boots before BL31.
Thank you for the constructive feedback.
So basically , what we need is a real sysreset driver for iMX8MM , which can work with bare-bones hardware interface, that is, poking some register, correct ?
Yes.
So, either you or Claudius needs to implement a driver in drivers/sysreset, which will bind in SPL and if needed, do some writel() into some registers to reset the board, correct ?
Yes.
Because, I believe we can both agree that dumping such ad-hoc code which implements reset in board code is not the right way, esp. nowadays with all the DM/DT stuff in.
Alought we still have ocram space, but our SPL is huge now, 100KB+
How so ?
How much does DM sysreset add to that ?
currently I am a bit hesitated about DM SPL, but it let us not to maintain two drivers for one module. Just some complaining words:) I am fine to add DM sysreset currently. Not sure how much it will add after DM usb.
Regards, Peng.

On 1/19/20 8:48 AM, Peng Fan wrote: [...]
Because, I believe we can both agree that dumping such ad-hoc code which implements reset in board code is not the right way, esp. nowadays with all the DM/DT stuff in.
Alought we still have ocram space, but our SPL is huge now, 100KB+
How so ?
How much does DM sysreset add to that ?
currently I am a bit hesitated about DM SPL, but it let us not to maintain two drivers for one module. Just some complaining words:) I am fine to add DM sysreset currently. Not sure how much it will add after DM usb.
I have two things to say about this. 1) I share your concern about DM SPL, we have a massive size problem. Thus far, we don't have a solution. Patches welcome. One option might be to optimize out frameworks which only have one driver instance in SPL, like MMC_TINY does and have subsystem API directly call that one driver instance. 2) Hacking ad-hoc drivers in board/ files is broken design, so we need a solution, which does not do that. If we want to disregard DM sysreset in SPL for this board, we can only do that based on hard numbers, that is, measure the size impact and determine if that's an issue. If so, we need another solution. Otherwise, we enable DM sysreset and go back to solving 1) as a separate topic.
participants (7)
-
Claudius Heine
-
Marek Vasut
-
Peng Fan
-
Peter Robinson
-
Simon Glass
-
Simon Goldschmidt
-
Tom Rini