[PATCH 3/4] Revert "lpc32xx: cpu: add support for soft reset"

Harald Seiler hws at denx.de
Thu Dec 17 11:09:06 CET 2020


Hello Sylvain,

On Tue, 2020-12-15 at 17:29 +0000, Sylvain Lemieux wrote:
> Hi,
> 
> This functionality (soft vs hard reset) is used in multiple LPC32xx
> products with our custom hardware.
> 
> If this support is remove from upstream, we will have to maintain this
> patch locally (out of tree).

My intention with this series is of course not to break existing and
useful functionality.  Can you elaborate how you are making use of this
differentiation in its current form?  Does some board code call
reset_cpu() directly, with different parameter values?

I would argue that at this time, the proper way to support both soft-reset
and hard-reset is via a sysreset driver, see
`drivers/sysreset/sysreset_psci.c` for a simple example.  As I laid out in
the cover letter of this series, the `addr` parameter to reset_cpu() does
not really fit this purpose.

Regards,
-- 
Harald

> Sylvain Lemieux
> 
> -----Original Message-----
> From: Harald Seiler <hws at denx.de> 
> Sent: Tuesday, December 15, 2020 10:48 AM
> To: u-boot at lists.denx.de
> Cc: Harald Seiler <hws at denx.de>; Tom Rini <trini at konsulko.com>; Simon Glass <sjg at chromium.org>; Sylvain Lemieux <sylvain.lemieux at jci.com>
> Subject: [PATCH 3/4] Revert "lpc32xx: cpu: add support for soft reset"
> 
> This reverts commit 576007aec9a4a5f4f3dd1f690fb26a8c05ceb75f.
> 
> The paramter passed to reset_cpu() no longer holds a meaning as all
> call-sites now pass the value 0.  Thus, branching on it is essentially
> dead code and will just confuse future readers.
> 
> Revert soft-reset support and just always perform a hard-reset for now.
> This is a preparation for removal of the reset_cpu() parameter across
> the entire tree in a later patch.
> 
> Fixes: 576007aec9a4 ("lpc32xx: cpu: add support for soft reset")
> Cc: Sylvain Lemieux <slemieux at tycoint.com>
> Signed-off-by: Harald Seiler <hws at denx.de>
> ---
>  arch/arm/mach-lpc32xx/cpu.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/mach-lpc32xx/cpu.c b/arch/arm/mach-lpc32xx/cpu.c index 32af6206056b..7378192a33c2 100644
> --- a/arch/arm/mach-lpc32xx/cpu.c
> +++ b/arch/arm/mach-lpc32xx/cpu.c
> @@ -22,23 +22,12 @@ void reset_cpu(ulong addr)
>         /* Enable watchdog clock */
>         setbits_le32(&clk->timclk_ctrl, CLK_TIMCLK_WATCHDOG);
>  
> -       /* To be compatible with the original U-Boot code:
> -        * addr: - 0: perform hard reset.
> -        *       - !=0: perform a soft reset; i.e. "RESOUT_N" not asserted). */
> -       if (addr == 0) {
> -               /* Reset pulse length is 13005 peripheral clock frames */
> -               writel(13000, &wdt->pulse);
> +       /* Reset pulse length is 13005 peripheral clock frames */
> +       writel(13000, &wdt->pulse);
>  
> -               /* Force WDOG_RESET2 and RESOUT_N signal active */
> -               writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1
> -                      | WDTIM_MCTRL_M_RES2, &wdt->mctrl);
> -       } else {
> -               /* Force match output active */
> -               writel(0x01, &wdt->emr);
> -
> -               /* Internal reset on match output (no pulse on "RESOUT_N") */
> -               writel(WDTIM_MCTRL_M_RES1, &wdt->mctrl);
> -       }
> +       /* Force WDOG_RESET2 and RESOUT_N signal active */
> +       writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1 | WDTIM_MCTRL_M_RES2,
> +              &wdt->mctrl);
>  
>         while (1)
>                 /* NOP */;
> --
> 2.26.2
> 

-- 
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



More information about the U-Boot mailing list