Re: [PATCH 2/3] sunxi: spl: initialise timer before clocks

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

On Mon, 11 Aug 2025 16:52:25 +0100
Andre Przywara <andre.przywara@arm.com> wrote:

Hi,

> On Mon, 11 Aug 2025 17:34:47 +0200
> Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> 
> > Dne sobota, 2. avgust 2025 ob 01:49:17 Srednjeevropski poletni čas je Andre Przywara napisal(a):  
> > > Recent changes in the H6 clock code added delay() calls into the SPL clock
> > > setup routine, which requires the timers to work. When compiling for
> > > AArch64, we are always using the Arm Generic Timer (aka. arch timer),
> > > which does not require further setup, hence having an empty timer_init()
> > > routine.
> > > However for 32-bit SoCs we use the Allwinner timers, which require some
> > > setup routine, and hence we need timer_init() to be called before
> > > clock_init().
> > > 
> > > Swap the order of the two calls, to be more robust when compiling the H6
> > > clock code for AArch32 or when using the Allwinner timers for whatever
> > > reason.
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  arch/arm/mach-sunxi/board.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> > > index fb4837c2082..432b1c10f92 100644
> > > --- a/arch/arm/mach-sunxi/board.c
> > > +++ b/arch/arm/mach-sunxi/board.c
> > > @@ -476,8 +476,8 @@ void board_init_f(ulong dummy)
> > >  	/* Enable non-secure access to some peripherals */
> > >  	tzpc_init();
> > >  
> > > -	clock_init();
> > >  	timer_init();
> > > +	clock_init();    
> > 
> > I contemplated similar change in past. It works fine for 64-bit architectures,
> > but I'm unsure for 32-bit. If you take a look at A83t clock code, it uses
> > sdelay() exactly because timer is not initialized at that time.
> > 
> > So, are you sure that this change has no unwanted side effects?  
> 
> No, I am not ;-)
> I was about to test this on some more boards. But I think with the
> change in that direction, the only issue would be if the timer_init() code
> relies on the clocks being set up already. And since timer_init() is only
> *one* rather small function, just doing MMIO accesses, I think the risk is
> quite low.
> 
> Thanks for having a look anyway, I will report back how the test goes.

Alright, I booted this patch on:
A10, A20, A33, V40, A80, A83T, T113s3, F1C100s, H3
(/me puts the mini-USB cables back into the shelf now)

For the A83T specifically, as a test, I replaced the sdelay() calls with
udelay() ones, and it hangs with mainline, but works with this patch.

So unless someone speaks up now, I am tempted to merge this patch.

Cheers,
Andre