Re: [PATCH 13/34] sunxi: spl: add support for Allwinner A523 watchdog

[Andre Przywara <andre.przywara@arm.com>]

On Sun, 23 Mar 2025 13:15:46 +0100
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi,

> Dne nedelja, 23. marec 2025 ob 12:35:23 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> > From: Jernej Skrabec <jernej.skrabec@gmail.com>
> > 
> > The watchdog in the Allwinner A523 SoC differs a bit from the one in the
> > previous SoCs: it lives in a separate register frame, so no longer
> > inside some timer device, and it manages to shuffle around some
> > registers a bit.
> > 
> > Provide a new struct describing the register layout, and adjust the
> > address calculation in the SPL code accoringly. This is guarded by the
> > MACH_SUN55I_A523 Kconfig variable.  
> 
> This one is missing SoB, apparently by me :)

Yes, I don't think it's appropriate to add your Signed-off-by, so if
you could give it here.

> 
> > ---
> >  arch/arm/include/asm/arch-sunxi/watchdog.h | 12 ++++++++++++
> >  arch/arm/mach-sunxi/board.c                |  6 ++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/arch-sunxi/watchdog.h b/arch/arm/include/asm/arch-sunxi/watchdog.h
> > index 38e2ef2aca3..96d5725141e 100644
> > --- a/arch/arm/include/asm/arch-sunxi/watchdog.h
> > +++ b/arch/arm/include/asm/arch-sunxi/watchdog.h
> > @@ -26,6 +26,18 @@ struct sunxi_wdog {
> >  	u32 res[2];
> >  };
> >  
> > +#elif defined(CONFIG_MACH_SUN55I_A523)
> > +
> > +struct sunxi_wdog {
> > +	u32 irq_en;		/* 0x00 */
> > +	u32 irq_sta;		/* 0x04 */
> > +	u32 srst;		/* 0x08 */
> > +	u32 ctl;		/* 0x0c */
> > +	u32 cfg;		/* 0x10 */
> > +	u32 mode;		/* 0x14 */
> > +	u32 ocfg;		/* 0x18 */
> > +};  
> 
> Anyway, since you announced battle against C structures for register
> layout, it seems contraproductive to add it here. What do you think?

Well, I am all for it, but I guess that's another refactor patch first,
so please be my guest. I didn't deem this important enough to address
this now (it's not blocking as for the clocks), and this series is
already long enough, so I was just happy to take this working patch.
For development I really like having a working "reset" command ;-)

Cheers,
Andre

> 
> Best regards,
> Jernej
> 
> > +
> >  #else
> >  
> >  #define WDT_CFG_RESET		(0x1)
> > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > index 701899ee4b2..89aea61e8e8 100644
> > --- a/arch/arm/mach-sunxi/board.c
> > +++ b/arch/arm/mach-sunxi/board.c
> > @@ -495,6 +495,12 @@ void reset_cpu(void)
> >  		/* sun5i sometimes gets stuck without this */
> >  		writel(WDT_MODE_RESET_EN | WDT_MODE_EN, &wdog->mode);
> >  	}
> > +#elif defined(CONFIG_MACH_SUN55I_A523)
> > +	static const struct sunxi_wdog *wdog =
> > +		(struct sunxi_wdog *)SUNXI_TIMER_BASE;
> > +
> > +	writel(WDT_CTRL_KEY | WDT_CTRL_RESTART, &wdog->srst);
> > +	while (1) { }
> >  #elif defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_SUN50I_GEN_H6) || defined(CONFIG_SUNXI_GEN_NCAT2)
> >  #if defined(CONFIG_MACH_SUN50I_H6)
> >  	/* WDOG is broken for some H6 rev. use the R_WDOG instead */
> >   
> 
> 
> 
> 
>