| Message ID | 20191128085648.30716-1-ch@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them | expand |
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
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
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(-)