Re: [PATCH 3/6] ARM: imx: Add weak default reset_cpu()

[Marek Vasut]

On 8/8/23 21:17, Fabio Estevam wrote:

Hi Marek,

On Mon, Jun 26, 2023 at 5:53 AM Marek Vasut  wrote:


Add weak default reset_cpu() implementation needed by e.g. panic().

Signed-off-by: Marek Vasut 
---
Cc: "NXP i.MX U-Boot Team" 
Cc: Fabio Estevam 
Cc: Stefano Babic 
---
  arch/arm/mach-imx/cpu.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index 702cfc33275..488638c9058 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -510,3 +510,7 @@ char nxp_board_rev_string(void)
 return (*rev + nxp_board_rev() - 1);
  }
  #endif
+
+__weak void reset_cpu(void)
+{
+}


This patch causes the 'reset' command to not reset the imx7d-sdb and
smegw01 boards anymore.

If I do the change below, the board resets again:

diff --git a/drivers/watchdog/imx_watchdog.c b/drivers/watchdog/imx_watchdog.c
index 894158b304a7..709e437c8bd0 100644
--- a/drivers/watchdog/imx_watchdog.c
+++ b/drivers/watchdog/imx_watchdog.c
@@ -44,7 +44,7 @@ static void imx_watchdog_expire_now(struct
watchdog_regs *wdog, bool ext_reset)

  #if !defined(CONFIG_IMX_WATCHDOG) || \
  (defined(CONFIG_IMX_WATCHDOG) && !CONFIG_IS_ENABLED(WDT))
-void __attribute__((weak)) reset_cpu(void)
+void reset_cpu(void)
  {
   struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;

Is this a correct fix?


No, this will likely fail on other machines which will now define two 
"strong" reset_cpu() functions. Drivers should really not provide any 
reset_cpu() implementation at all (except maybe for sysreset drivers), 
all that reset_cpu() should be in arch/ .


I think the proper fix is to use drivers/sysreset/sysreset_watchdog.c to 
expire that imx watchdog and cause a reset.

Re: [PATCH 3/6] ARM: imx: Add weak default reset_cpu()

[Fabio Estevam]

Hi Marek,

On Mon, Jun 26, 2023 at 5:53 AM Marek Vasut  wrote:
>
> Add weak default reset_cpu() implementation needed by e.g. panic().
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: "NXP i.MX U-Boot Team" 
> Cc: Fabio Estevam 
> Cc: Stefano Babic 
> ---
>  arch/arm/mach-imx/cpu.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
> index 702cfc33275..488638c9058 100644
> --- a/arch/arm/mach-imx/cpu.c
> +++ b/arch/arm/mach-imx/cpu.c
> @@ -510,3 +510,7 @@ char nxp_board_rev_string(void)
> return (*rev + nxp_board_rev() - 1);
>  }
>  #endif
> +
> +__weak void reset_cpu(void)
> +{
> +}

This patch causes the 'reset' command to not reset the imx7d-sdb and
smegw01 boards anymore.

If I do the change below, the board resets again:

diff --git a/drivers/watchdog/imx_watchdog.c b/drivers/watchdog/imx_watchdog.c
index 894158b304a7..709e437c8bd0 100644
--- a/drivers/watchdog/imx_watchdog.c
+++ b/drivers/watchdog/imx_watchdog.c
@@ -44,7 +44,7 @@ static void imx_watchdog_expire_now(struct
watchdog_regs *wdog, bool ext_reset)

 #if !defined(CONFIG_IMX_WATCHDOG) || \
 (defined(CONFIG_IMX_WATCHDOG) && !CONFIG_IS_ENABLED(WDT))
-void __attribute__((weak)) reset_cpu(void)
+void reset_cpu(void)
 {
  struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;

Is this a correct fix?

Thanks

[PATCH 3/6] ARM: imx: Add weak default reset_cpu()

[Marek Vasut]

Add weak default reset_cpu() implementation needed by e.g. panic().

Signed-off-by: Marek Vasut 
---
Cc: "NXP i.MX U-Boot Team" 
Cc: Fabio Estevam 
Cc: Stefano Babic 
---
 arch/arm/mach-imx/cpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index 702cfc33275..488638c9058 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -510,3 +510,7 @@ char nxp_board_rev_string(void)
return (*rev + nxp_board_rev() - 1);
 }
 #endif
+
+__weak void reset_cpu(void)
+{
+}
-- 
2.40.1

Re: [PATCH 3/6] ARM: imx: Add weak default reset_cpu()

[Tom Rini]

On Fri, Jun 23, 2023 at 09:56:27PM +0200, Marek Vasut wrote:

> Add weak default reset_cpu() implementation needed by e.g. panic().
> 
> Signed-off-by: Marek Vasut 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature