* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
@ 2019-11-28 8:56 Claudius Heine
2019-11-28 9:22 ` Marek Vasut
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Claudius Heine @ 2019-11-28 8:56 UTC (permalink / raw)
To: u-boot
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
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2019-11-28 8:56 [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Claudius Heine
@ 2019-11-28 9:22 ` Marek Vasut
2019-11-28 9:27 ` Simon Goldschmidt
2020-01-10 21:48 ` Tom Rini
2 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2019-11-28 9:22 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2019-11-28 8:56 [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Claudius Heine
2019-11-28 9:22 ` Marek Vasut
@ 2019-11-28 9:27 ` Simon Goldschmidt
2019-11-28 10:52 ` Claudius Heine
2020-01-10 21:48 ` Tom Rini
2 siblings, 1 reply; 17+ messages in thread
From: Simon Goldschmidt @ 2019-11-28 9:27 UTC (permalink / raw)
To: u-boot
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2019-11-28 9:27 ` Simon Goldschmidt
@ 2019-11-28 10:52 ` Claudius Heine
2019-11-28 11:31 ` Simon Goldschmidt
0 siblings, 1 reply; 17+ messages in thread
From: Claudius Heine @ 2019-11-28 10:52 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2019-11-28 10:52 ` Claudius Heine
@ 2019-11-28 11:31 ` Simon Goldschmidt
0 siblings, 0 replies; 17+ messages in thread
From: Simon Goldschmidt @ 2019-11-28 11:31 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2019-11-28 8:56 [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Claudius Heine
2019-11-28 9:22 ` Marek Vasut
2019-11-28 9:27 ` Simon Goldschmidt
@ 2020-01-10 21:48 ` Tom Rini
2020-01-14 9:59 ` Claudius Heine
2 siblings, 1 reply; 17+ messages in thread
From: Tom Rini @ 2020-01-10 21:48 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200110/16f033ae/attachment.sig>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-01-10 21:48 ` Tom Rini
@ 2020-01-14 9:59 ` Claudius Heine
2020-01-14 10:18 ` Peter Robinson
2020-01-15 14:49 ` [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL Claudius Heine
0 siblings, 2 replies; 17+ messages in thread
From: Claudius Heine @ 2020-01-14 9:59 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-01-14 9:59 ` Claudius Heine
@ 2020-01-14 10:18 ` Peter Robinson
2020-01-14 12:02 ` Claudius Heine
2020-01-15 14:49 ` [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL Claudius Heine
1 sibling, 1 reply; 17+ messages in thread
From: Peter Robinson @ 2020-01-14 10:18 UTC (permalink / raw)
To: u-boot
> 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?
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-01-14 10:18 ` Peter Robinson
@ 2020-01-14 12:02 ` Claudius Heine
0 siblings, 0 replies; 17+ messages in thread
From: Claudius Heine @ 2020-01-14 12:02 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-14 9:59 ` Claudius Heine
2020-01-14 10:18 ` Peter Robinson
@ 2020-01-15 14:49 ` Claudius Heine
2020-01-16 0:10 ` Simon Glass
2020-01-16 2:21 ` Peng Fan
1 sibling, 2 replies; 17+ messages in thread
From: Claudius Heine @ 2020-01-15 14:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-15 14:49 ` [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL Claudius Heine
@ 2020-01-16 0:10 ` Simon Glass
2020-01-16 2:21 ` Peng Fan
1 sibling, 0 replies; 17+ messages in thread
From: Simon Glass @ 2020-01-16 0:10 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-15 14:49 ` [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL Claudius Heine
2020-01-16 0:10 ` Simon Glass
@ 2020-01-16 2:21 ` Peng Fan
2020-01-16 9:27 ` Marek Vasut
1 sibling, 1 reply; 17+ messages in thread
From: Peng Fan @ 2020-01-16 2:21 UTC (permalink / raw)
To: u-boot
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-16 2:21 ` Peng Fan
@ 2020-01-16 9:27 ` Marek Vasut
2020-01-17 2:33 ` Peng Fan
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2020-01-16 9:27 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-16 9:27 ` Marek Vasut
@ 2020-01-17 2:33 ` Peng Fan
2020-01-17 9:32 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Peng Fan @ 2020-01-17 2:33 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-17 2:33 ` Peng Fan
@ 2020-01-17 9:32 ` Marek Vasut
2020-01-19 7:48 ` Peng Fan
0 siblings, 1 reply; 17+ messages in thread
From: Marek Vasut @ 2020-01-17 9:32 UTC (permalink / raw)
To: u-boot
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 ?
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-17 9:32 ` Marek Vasut
@ 2020-01-19 7:48 ` Peng Fan
2020-01-19 14:10 ` Marek Vasut
0 siblings, 1 reply; 17+ messages in thread
From: Peng Fan @ 2020-01-19 7:48 UTC (permalink / raw)
To: u-boot
> 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL
2020-01-19 7:48 ` Peng Fan
@ 2020-01-19 14:10 ` Marek Vasut
0 siblings, 0 replies; 17+ messages in thread
From: Marek Vasut @ 2020-01-19 14:10 UTC (permalink / raw)
To: u-boot
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.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-01-19 14:10 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28 8:56 [U-Boot] [PATCH] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Claudius Heine
2019-11-28 9:22 ` Marek Vasut
2019-11-28 9:27 ` Simon Goldschmidt
2019-11-28 10:52 ` Claudius Heine
2019-11-28 11:31 ` Simon Goldschmidt
2020-01-10 21:48 ` Tom Rini
2020-01-14 9:59 ` Claudius Heine
2020-01-14 10:18 ` Peter Robinson
2020-01-14 12:02 ` Claudius Heine
2020-01-15 14:49 ` [RFC PATCH] imx: imx8mm-evk/imx8mn-evk: enable sysreset driver for SPL Claudius Heine
2020-01-16 0:10 ` Simon Glass
2020-01-16 2:21 ` Peng Fan
2020-01-16 9:27 ` Marek Vasut
2020-01-17 2:33 ` Peng Fan
2020-01-17 9:32 ` Marek Vasut
2020-01-19 7:48 ` Peng Fan
2020-01-19 14:10 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.