* [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
@ 2020-03-04 14:23 Harald Seiler
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Harald Seiler @ 2020-03-04 14:23 UTC (permalink / raw)
To: u-boot
Hello,
continuing on the discussion around Claudius' patch for fixing reset in SPL [1]
we have taken a closer look at the issue. To quickly summarize the situation:
The original patch was to enable the generic ARM implementation of
`do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break
compilation for some boards which define their own `do_reset` in
`board/*/spl.c`.
To be more specific, the following 4 boards have this custom `do_reset`:
- toradex/verdin-imx8mm
- freescale/imx8mn_evk
- freescale/imx8mm_evk
- freescale/imx8mp_evk
I hope we can all agree that `do_reset` is not at all meant to be implemented
in board files. From looking at the related code for imx8m, it feels like this
was just a workaround hack to archieve the same thing which Claudius has fixed.
So this patch series reverts the addition of `do_reset` implementations in
imx8m board files and instead switches to using the proper fix provided by
Claudius.
Additionally, the custom do_reset implementations were passing an address
(WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in the
entire U-Boot tree where this happens. Instead, all other implementations
simply ignore the parameter and 0 is passed by callers. It looks a lot like
this is a legacy left-over which makes me think that using it for a (hard-coded)
watchdog address is not a good idea as it breaks convention with the rest of
U-Boot.
[1]: https://patchwork.ozlabs.org/patch/1201959
Claudius Heine (3):
ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for
them
imx: imx8m*: Remove do_reset from board files
imx: imx8m: Don't use the addr parameter of reset_cpu
arch/arm/lib/Makefile | 2 +-
arch/arm/mach-imx/imx8m/soc.c | 5 +----
board/freescale/imx8mm_evk/spl.c | 9 ---------
board/freescale/imx8mn_evk/spl.c | 9 ---------
board/freescale/imx8mp_evk/spl.c | 9 ---------
board/toradex/verdin-imx8mm/spl.c | 9 ---------
6 files changed, 2 insertions(+), 41 deletions(-)
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: hws at denx.de
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-03-04 14:23 [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
@ 2020-03-04 14:23 ` Harald Seiler
2020-03-04 14:26 ` Marek Vasut
2020-05-01 16:31 ` sbabic at denx.de
2020-03-04 14:23 ` [PATCH 2/3] imx: imx8m*: Remove do_reset from board files Harald Seiler
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Harald Seiler @ 2020-03-04 14:23 UTC (permalink / raw)
To: u-boot
From: Claudius Heine <ch@denx.de>
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 8482f5446c5c..b839aa7a5096 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -57,7 +57,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.24.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] imx: imx8m*: Remove do_reset from board files
2020-03-04 14:23 [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
@ 2020-03-04 14:23 ` Harald Seiler
2020-03-04 14:29 ` Marek Vasut
2020-05-01 16:33 ` sbabic at denx.de
2020-03-04 14:23 ` [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu Harald Seiler
2020-04-23 11:18 ` [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
3 siblings, 2 replies; 13+ messages in thread
From: Harald Seiler @ 2020-03-04 14:23 UTC (permalink / raw)
To: u-boot
From: Claudius Heine <ch@denx.de>
Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL
instead. It is very close to what is done here, anyway, and plays
more nicely with the rest of U-Boot than adding a custom `do_reset`
implementation into board files.
`do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the
addr parameter while the boards are passing WDOG1_BASE_ADDR. This is
ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by
default if 0 is passed in.
Co-Authored-by: Harald Seiler <hws@denx.de>
Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Harald Seiler <hws@denx.de>
---
board/freescale/imx8mm_evk/spl.c | 9 ---------
board/freescale/imx8mn_evk/spl.c | 9 ---------
board/freescale/imx8mp_evk/spl.c | 9 ---------
board/toradex/verdin-imx8mm/spl.c | 9 ---------
4 files changed, 36 deletions(-)
diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c
index 5d17f397cb68..4d34622465b3 100644
--- a/board/freescale/imx8mm_evk/spl.c
+++ b/board/freescale/imx8mm_evk/spl.c
@@ -161,12 +161,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 7aed14c52b68..45417b24464d 100644
--- a/board/freescale/imx8mn_evk/spl.c
+++ b/board/freescale/imx8mn_evk/spl.c
@@ -114,12 +114,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/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c
index 0b20668e2b30..39c1dae684ac 100644
--- a/board/freescale/imx8mp_evk/spl.c
+++ b/board/freescale/imx8mp_evk/spl.c
@@ -149,12 +149,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/toradex/verdin-imx8mm/spl.c b/board/toradex/verdin-imx8mm/spl.c
index a5dc54082054..dc5bd84f332e 100644
--- a/board/toradex/verdin-imx8mm/spl.c
+++ b/board/toradex/verdin-imx8mm/spl.c
@@ -169,12 +169,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;
-}
--
2.24.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
2020-03-04 14:23 [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
2020-03-04 14:23 ` [PATCH 2/3] imx: imx8m*: Remove do_reset from board files Harald Seiler
@ 2020-03-04 14:23 ` Harald Seiler
2020-03-04 14:30 ` Marek Vasut
2020-05-01 16:32 ` sbabic at denx.de
2020-04-23 11:18 ` [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
3 siblings, 2 replies; 13+ messages in thread
From: Harald Seiler @ 2020-03-04 14:23 UTC (permalink / raw)
To: u-boot
From: Claudius Heine <ch@denx.de>
imx8m has the only implementation of `reset_cpu` which does not ignore
the addr parameter and instead gives it some meaning as the base address
of watchdog registers. This breaks convention with the rest of U-Boot
where the parameter is ignored and callers are passing in 0.
Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
Co-Authored-by: Harald Seiler <hws@denx.de>
Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Harald Seiler <hws@denx.de>
---
arch/arm/mach-imx/imx8m/soc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 7fcbd53f3020..2d3afc61a452 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
#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;
+ struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
/* Clear WDA to trigger WDOG_B immediately */
writew((WCR_WDE | WCR_SRS), &wdog->wcr);
--
2.24.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
@ 2020-03-04 14:26 ` Marek Vasut
2020-05-01 16:31 ` sbabic at denx.de
1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2020-03-04 14:26 UTC (permalink / raw)
To: u-boot
On 3/4/20 3:23 PM, Harald Seiler wrote:
> From: Claudius Heine <ch@denx.de>
>
> 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] 13+ messages in thread
* [PATCH 2/3] imx: imx8m*: Remove do_reset from board files
2020-03-04 14:23 ` [PATCH 2/3] imx: imx8m*: Remove do_reset from board files Harald Seiler
@ 2020-03-04 14:29 ` Marek Vasut
2020-05-01 16:33 ` sbabic at denx.de
1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2020-03-04 14:29 UTC (permalink / raw)
To: u-boot
On 3/4/20 3:23 PM, Harald Seiler wrote:
> From: Claudius Heine <ch@denx.de>
>
> Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL
> instead. It is very close to what is done here, anyway, and plays
> more nicely with the rest of U-Boot than adding a custom `do_reset`
> implementation into board files.
>
> `do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the
> addr parameter while the boards are passing WDOG1_BASE_ADDR. This is
> ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by
> default if 0 is passed in.
>
> Co-Authored-by: Harald Seiler <hws@denx.de>
> Signed-off-by: Claudius Heine <ch@denx.de>
> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
> board/freescale/imx8mm_evk/spl.c | 9 ---------
> board/freescale/imx8mn_evk/spl.c | 9 ---------
> board/freescale/imx8mp_evk/spl.c | 9 ---------
> board/toradex/verdin-imx8mm/spl.c | 9 ---------
> 4 files changed, 36 deletions(-)
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
2020-03-04 14:23 ` [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu Harald Seiler
@ 2020-03-04 14:30 ` Marek Vasut
2020-03-04 14:35 ` Harald Seiler
2020-05-01 16:32 ` sbabic at denx.de
1 sibling, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2020-03-04 14:30 UTC (permalink / raw)
To: u-boot
On 3/4/20 3:23 PM, Harald Seiler wrote:
> From: Claudius Heine <ch@denx.de>
>
> imx8m has the only implementation of `reset_cpu` which does not ignore
> the addr parameter and instead gives it some meaning as the base address
> of watchdog registers. This breaks convention with the rest of U-Boot
> where the parameter is ignored and callers are passing in 0.
>
> Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> Co-Authored-by: Harald Seiler <hws@denx.de>
> Signed-off-by: Claudius Heine <ch@denx.de>
> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
> arch/arm/mach-imx/imx8m/soc.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 7fcbd53f3020..2d3afc61a452 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> #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;
> + struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
>
> /* Clear WDA to trigger WDOG_B immediately */
> writew((WCR_WDE | WCR_SRS), &wdog->wcr);
Can't we remove the param altogether ?
Otherwise
Reviewed-by: Marek Vasut <marex@denx.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
2020-03-04 14:30 ` Marek Vasut
@ 2020-03-04 14:35 ` Harald Seiler
0 siblings, 0 replies; 13+ messages in thread
From: Harald Seiler @ 2020-03-04 14:35 UTC (permalink / raw)
To: u-boot
Hello Marek,
On Wed, 2020-03-04 at 15:30 +0100, Marek Vasut wrote:
> On 3/4/20 3:23 PM, Harald Seiler wrote:
> > From: Claudius Heine <ch@denx.de>
> >
> > imx8m has the only implementation of `reset_cpu` which does not ignore
> > the addr parameter and instead gives it some meaning as the base address
> > of watchdog registers. This breaks convention with the rest of U-Boot
> > where the parameter is ignored and callers are passing in 0.
> >
> > Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> > Co-Authored-by: Harald Seiler <hws@denx.de>
> > Signed-off-by: Claudius Heine <ch@denx.de>
> > Signed-off-by: Harald Seiler <hws@denx.de>
> > ---
> > arch/arm/mach-imx/imx8m/soc.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> > index 7fcbd53f3020..2d3afc61a452 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> > #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;
> > + struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> >
> > /* Clear WDA to trigger WDOG_B immediately */
> > writew((WCR_WDE | WCR_SRS), &wdog->wcr);
>
> Can't we remove the param altogether ?
Yes, that is what I would suggest as well. Although I'd do that as
a separate follow up because it is not ARM nor i.MX specific.
> Otherwise
> Reviewed-by: Marek Vasut <marex@denx.de>
--
Harald
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
2020-03-04 14:23 [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
` (2 preceding siblings ...)
2020-03-04 14:23 ` [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu Harald Seiler
@ 2020-04-23 11:18 ` Harald Seiler
2020-04-23 11:23 ` Stefano Babic
3 siblings, 1 reply; 13+ messages in thread
From: Harald Seiler @ 2020-04-23 11:18 UTC (permalink / raw)
To: u-boot
Hello,
On Wed, 2020-03-04 at 15:23 +0100, Harald Seiler wrote:
> Hello,
>
> continuing on the discussion around Claudius' patch for fixing reset in SPL [1]
> we have taken a closer look at the issue. To quickly summarize the situation:
>
> The original patch was to enable the generic ARM implementation of
> `do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break
> compilation for some boards which define their own `do_reset` in
> `board/*/spl.c`.
>
> To be more specific, the following 4 boards have this custom `do_reset`:
>
> - toradex/verdin-imx8mm
> - freescale/imx8mn_evk
> - freescale/imx8mm_evk
> - freescale/imx8mp_evk
>
> I hope we can all agree that `do_reset` is not at all meant to be implemented
> in board files. From looking at the related code for imx8m, it feels like this
> was just a workaround hack to archieve the same thing which Claudius has fixed.
> So this patch series reverts the addition of `do_reset` implementations in
> imx8m board files and instead switches to using the proper fix provided by
> Claudius.
>
>
> Additionally, the custom do_reset implementations were passing an address
> (WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in the
> entire U-Boot tree where this happens. Instead, all other implementations
> simply ignore the parameter and 0 is passed by callers. It looks a lot like
> this is a legacy left-over which makes me think that using it for a (hard-coded)
> watchdog address is not a good idea as it breaks convention with the rest of
> U-Boot.
>
> [1]: https://patchwork.ozlabs.org/patch/1201959
>
> Claudius Heine (3):
> ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for
> them
> imx: imx8m*: Remove do_reset from board files
> imx: imx8m: Don't use the addr parameter of reset_cpu
>
> arch/arm/lib/Makefile | 2 +-
> arch/arm/mach-imx/imx8m/soc.c | 5 +----
> board/freescale/imx8mm_evk/spl.c | 9 ---------
> board/freescale/imx8mn_evk/spl.c | 9 ---------
> board/freescale/imx8mp_evk/spl.c | 9 ---------
> board/toradex/verdin-imx8mm/spl.c | 9 ---------
> 6 files changed, 2 insertions(+), 41 deletions(-)
>
Quick question, what is the situation with this series? Is there anything
which needs to be addressed?
Without it, CONFIG_SPL_USB_SDP_SUPPORT is broken on the imx6q hardware
I am working with and I guess the same issue might exist on many other
boards as well (The USB stack needs a do_reset() implementation in SPL).
Regards,
--
Harald
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
2020-04-23 11:18 ` [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
@ 2020-04-23 11:23 ` Stefano Babic
0 siblings, 0 replies; 13+ messages in thread
From: Stefano Babic @ 2020-04-23 11:23 UTC (permalink / raw)
To: u-boot
On 23.04.20 13:18, Harald Seiler wrote:
> Hello,
>
> On Wed, 2020-03-04 at 15:23 +0100, Harald Seiler wrote:
>> Hello,
>>
>> continuing on the discussion around Claudius' patch for fixing reset in SPL [1]
>> we have taken a closer look at the issue. To quickly summarize the situation:
>>
>> The original patch was to enable the generic ARM implementation of
>> `do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break
>> compilation for some boards which define their own `do_reset` in
>> `board/*/spl.c`.
>>
>> To be more specific, the following 4 boards have this custom `do_reset`:
>>
>> - toradex/verdin-imx8mm
>> - freescale/imx8mn_evk
>> - freescale/imx8mm_evk
>> - freescale/imx8mp_evk
>>
>> I hope we can all agree that `do_reset` is not at all meant to be implemented
>> in board files. From looking at the related code for imx8m, it feels like this
>> was just a workaround hack to archieve the same thing which Claudius has fixed.
>> So this patch series reverts the addition of `do_reset` implementations in
>> imx8m board files and instead switches to using the proper fix provided by
>> Claudius.
>>
>>
>> Additionally, the custom do_reset implementations were passing an address
>> (WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in the
>> entire U-Boot tree where this happens. Instead, all other implementations
>> simply ignore the parameter and 0 is passed by callers. It looks a lot like
>> this is a legacy left-over which makes me think that using it for a (hard-coded)
>> watchdog address is not a good idea as it breaks convention with the rest of
>> U-Boot.
>>
>> [1]: https://patchwork.ozlabs.org/patch/1201959
>>
>> Claudius Heine (3):
>> ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for
>> them
>> imx: imx8m*: Remove do_reset from board files
>> imx: imx8m: Don't use the addr parameter of reset_cpu
>>
>> arch/arm/lib/Makefile | 2 +-
>> arch/arm/mach-imx/imx8m/soc.c | 5 +----
>> board/freescale/imx8mm_evk/spl.c | 9 ---------
>> board/freescale/imx8mn_evk/spl.c | 9 ---------
>> board/freescale/imx8mp_evk/spl.c | 9 ---------
>> board/toradex/verdin-imx8mm/spl.c | 9 ---------
>> 6 files changed, 2 insertions(+), 41 deletions(-)
>>
>
> Quick question, what is the situation with this series? Is there anything
> which needs to be addressed?
Patches are fine IMHO - I will merge them soon.
Stefano
>
> Without it, CONFIG_SPL_USB_SDP_SUPPORT is broken on the imx6q hardware
> I am working with and I guess the same issue might exist on many other
> boards as well (The USB stack needs a do_reset() implementation in SPL).
>
> Regards,
>
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
2020-03-04 14:26 ` Marek Vasut
@ 2020-05-01 16:31 ` sbabic at denx.de
1 sibling, 0 replies; 13+ messages in thread
From: sbabic at denx.de @ 2020-05-01 16:31 UTC (permalink / raw)
To: u-boot
> From: Claudius Heine <ch@denx.de>
> 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>
Applied to u-boot-imx, master, thanks !
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
2020-03-04 14:23 ` [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu Harald Seiler
2020-03-04 14:30 ` Marek Vasut
@ 2020-05-01 16:32 ` sbabic at denx.de
1 sibling, 0 replies; 13+ messages in thread
From: sbabic at denx.de @ 2020-05-01 16:32 UTC (permalink / raw)
To: u-boot
> From: Claudius Heine <ch@denx.de>
> imx8m has the only implementation of `reset_cpu` which does not ignore
> the addr parameter and instead gives it some meaning as the base address
> of watchdog registers. This breaks convention with the rest of U-Boot
> where the parameter is ignored and callers are passing in 0.
> Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> Co-Authored-by: Harald Seiler <hws@denx.de>
> Signed-off-by: Claudius Heine <ch@denx.de>
> Signed-off-by: Harald Seiler <hws@denx.de>
> Reviewed-by: Marek Vasut <marex@denx.de>
Applied to u-boot-imx, master, thanks !
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] imx: imx8m*: Remove do_reset from board files
2020-03-04 14:23 ` [PATCH 2/3] imx: imx8m*: Remove do_reset from board files Harald Seiler
2020-03-04 14:29 ` Marek Vasut
@ 2020-05-01 16:33 ` sbabic at denx.de
1 sibling, 0 replies; 13+ messages in thread
From: sbabic at denx.de @ 2020-05-01 16:33 UTC (permalink / raw)
To: u-boot
> From: Claudius Heine <ch@denx.de>
> Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL
> instead. It is very close to what is done here, anyway, and plays
> more nicely with the rest of U-Boot than adding a custom `do_reset`
> implementation into board files.
> `do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the
> addr parameter while the boards are passing WDOG1_BASE_ADDR. This is
> ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by
> default if 0 is passed in.
> Co-Authored-by: Harald Seiler <hws@denx.de>
> Signed-off-by: Claudius Heine <ch@denx.de>
> Signed-off-by: Harald Seiler <hws@denx.de>
> Reviewed-by: Marek Vasut <marex@denx.de>
Applied to u-boot-imx, master, thanks !
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-05-01 16:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-04 14:23 [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
2020-03-04 14:23 ` [PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler
2020-03-04 14:26 ` Marek Vasut
2020-05-01 16:31 ` sbabic at denx.de
2020-03-04 14:23 ` [PATCH 2/3] imx: imx8m*: Remove do_reset from board files Harald Seiler
2020-03-04 14:29 ` Marek Vasut
2020-05-01 16:33 ` sbabic at denx.de
2020-03-04 14:23 ` [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu Harald Seiler
2020-03-04 14:30 ` Marek Vasut
2020-03-04 14:35 ` Harald Seiler
2020-05-01 16:32 ` sbabic at denx.de
2020-04-23 11:18 ` [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used Harald Seiler
2020-04-23 11:23 ` Stefano Babic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox