Hi Yao, On 2026-07-01T11:17:53, Yao Zi <[email protected]> wrote: > LoongArch: CPU assembly routines > > start.S for initialisation, smp_secondary routine for > a spin-table like interface for secondary cpus. > > Signed-off-by: Jiaxun Yang <[email protected]> > Signed-off-by: Yao Zi <[email protected]> > > arch/loongarch/cpu/Makefile | 4 + > arch/loongarch/cpu/cpu.c | 27 ++++++ > arch/loongarch/cpu/smp_secondary.S | 55 ++++++++++++ > arch/loongarch/cpu/start.S | 170 > +++++++++++++++++++++++++++++++++++++ > 4 files changed, 256 insertions(+)
> diff --git a/arch/loongarch/cpu/cpu.c b/arch/loongarch/cpu/cpu.c > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018, Bin Meng <[email protected]> > + */ Good to keep that copyright (if that is where the code was derived) but you could add your own too. > diff --git a/arch/loongarch/cpu/cpu.c b/arch/loongarch/cpu/cpu.c > @@ -0,0 +1,27 @@ > +#include <command.h> > +#include <cpu.h> > +#include <cpu_func.h> > +#include <dm.h> > +#include <dm/lists.h> > +#include <event.h> > +#include <hang.h> > +#include <init.h> > +#include <log.h> > +#include <asm/system.h> > +#include <dm/uclass-internal.h> > +#include <linux/bitops.h> Most of these are unused - only printf() and hang() are referenced. Please prune to the minimum required. > diff --git a/arch/loongarch/cpu/cpu.c b/arch/loongarch/cpu/cpu.c > @@ -0,0 +1,27 @@ > +#if !CONFIG_IS_ENABLED(SYSRESET) > +void reset_cpu(void) > +{ > + printf("resetting ...\n"); > + > + printf("reset not supported yet\n"); > + hang(); > +} > +#endif Printing 'resetting ...' immediately followed by 'reset not supported yet' is confusing. Please drop the first message. > diff --git a/arch/loongarch/cpu/start.S b/arch/loongarch/cpu/start.S > @@ -0,0 +1,170 @@ > +call_board_init_f_0: > + /* find top of reserve space */ > + PTR_LI t1, 1 > + PTR_SLL t1, t1, CONFIG_STACK_SIZE_SHIFT > + PTR_SUB a0, t0, t1 /* t1 -> size of all CPU stacks > */ The comment is misleading - t1 holds a single stack's size, not the combined size of all CPU stacks. Please match arch/riscv/cpu/start.S ('top of stack of first CPU'). > diff --git a/arch/loongarch/cpu/start.S b/arch/loongarch/cpu/start.S > @@ -0,0 +1,170 @@ > + move a0, zero /* a0 <-- boot_flags = 0 */ > + la.pcrel t5, board_init_f > + jirl ra, t5, 0 /* jump to board_init_f() */ > + > + move sp, s0 s0 is never assigned before this point, so if board_init_f() ever returned here sp would be zero. Today this is dead code because jump_to_copy() reaches relocate_code() via a normal C call, but the instruction is misleading. Please remove it, or if you intend to mirror the RISC-V flow, add the missing 'move s0, a0' after board_init_f_alloc_reserve() and a comment explaining what s0 holds. > diff --git a/arch/loongarch/cpu/start.S b/arch/loongarch/cpu/start.S > @@ -0,0 +1,170 @@ > + csrrd t0, LOONGARCH_CSR_CPUID > + andi t0, t0, CSR_CPUID_COREID > + LONG_LI t1, BOOTCORE_ID > + bne t1, t0, secondary_core_loop Stray extra tab after bne > diff --git a/arch/loongarch/cpu/smp_secondary.S > b/arch/loongarch/cpu/smp_secondary.S > @@ -0,0 +1,55 @@ > +1: > + /* Query spin table */ > + idle 0 > + nop > + iocsrrd.w t0, t1 > + beqz t0, 1b Is the nop after idle 0 required by the ISA (to guarantee the wake-up completes before the load) or just padding? A one-line comment would help. Regards, Simon
