| Message ID | 0d875f98e9d81b68265ea26c379dae8ce5b1f5a9.1777550269.git.michal.simek@amd.com |
|---|---|
| State | New |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: Skip riscv_cpu_setup() when CPU driver is disabled | expand |
On Thu, Apr 30, 2026 at 01:57:51PM +0200, Michal Simek wrote: > Building on commit c64fc632a86a ("riscv: cpu: Use CONFIG_IS_ENABLED(CPU) > instead of plain ifdef"), add an early return in riscv_cpu_setup() when > CONFIG_CPU is not enabled. This allows platforms to save code space in > SPL by disabling CONFIG_SPL_CPU. > > The compiler's dead-code elimination combined with --gc-sections > removes the unreachable code and all associated static data, > achieving significant size reduction without preprocessor guards: > > spl/u-boot-spl:all -4332 spl/u-boot-spl:rodata -2872 > spl/u-boot-spl:text -1460 > > Signed-off-by: Michal Simek <michal.simek@amd.com> > --- > > arch/riscv/cpu/cpu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > index bbadd0c9a469..3bec7c7cb6d0 100644 > --- a/arch/riscv/cpu/cpu.c > +++ b/arch/riscv/cpu/cpu.c > @@ -638,6 +638,9 @@ int riscv_cpu_setup(void) > const char *isa, **exts; > struct udevice *dev; > > + if (!CONFIG_IS_ENABLED(CPU)) > + return 0; Should we return zero here, or -ENOENT like the original behavior when no UCLASS_CPU device is found? Otherwise the patch looks good to me. Reviewed-by: Yao Zi <me@ziyao.cc> > + > uclass_find_first_device(UCLASS_CPU, &dev); > if (!dev) { > debug("unable to find the RISC-V cpu device\n"); > -- > 2.43.0 > > base-commit: ab9f3cb4de54c65bde394e31469c75ec89b18d5b
On 5/1/26 10:54, Yao Zi wrote: > On Thu, Apr 30, 2026 at 01:57:51PM +0200, Michal Simek wrote: >> Building on commit c64fc632a86a ("riscv: cpu: Use CONFIG_IS_ENABLED(CPU) >> instead of plain ifdef"), add an early return in riscv_cpu_setup() when >> CONFIG_CPU is not enabled. This allows platforms to save code space in >> SPL by disabling CONFIG_SPL_CPU. >> >> The compiler's dead-code elimination combined with --gc-sections >> removes the unreachable code and all associated static data, >> achieving significant size reduction without preprocessor guards: >> >> spl/u-boot-spl:all -4332 spl/u-boot-spl:rodata -2872 >> spl/u-boot-spl:text -1460 >> >> Signed-off-by: Michal Simek <michal.simek@amd.com> >> --- >> >> arch/riscv/cpu/cpu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c >> index bbadd0c9a469..3bec7c7cb6d0 100644 >> --- a/arch/riscv/cpu/cpu.c >> +++ b/arch/riscv/cpu/cpu.c >> @@ -638,6 +638,9 @@ int riscv_cpu_setup(void) >> const char *isa, **exts; >> struct udevice *dev; >> >> + if (!CONFIG_IS_ENABLED(CPU)) >> + return 0; > > Should we return zero here, or -ENOENT like the original behavior when > no UCLASS_CPU device is found? if you put there error value then when CONFIG_CPU is disable you get boot hang which is likely what you don't want to see right? initcall_run_f(): initcall initf_dm() failed ### ERROR ### Please RESET the board ### Thanks, Michal
On Mon, May 04, 2026 at 11:00:17AM +0200, Michal Simek wrote: > > > On 5/1/26 10:54, Yao Zi wrote: > > On Thu, Apr 30, 2026 at 01:57:51PM +0200, Michal Simek wrote: > > > Building on commit c64fc632a86a ("riscv: cpu: Use CONFIG_IS_ENABLED(CPU) > > > instead of plain ifdef"), add an early return in riscv_cpu_setup() when > > > CONFIG_CPU is not enabled. This allows platforms to save code space in > > > SPL by disabling CONFIG_SPL_CPU. > > > > > > The compiler's dead-code elimination combined with --gc-sections > > > removes the unreachable code and all associated static data, > > > achieving significant size reduction without preprocessor guards: > > > > > > spl/u-boot-spl:all -4332 spl/u-boot-spl:rodata -2872 > > > spl/u-boot-spl:text -1460 > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com> > > > --- > > > > > > arch/riscv/cpu/cpu.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > > > index bbadd0c9a469..3bec7c7cb6d0 100644 > > > --- a/arch/riscv/cpu/cpu.c > > > +++ b/arch/riscv/cpu/cpu.c > > > @@ -638,6 +638,9 @@ int riscv_cpu_setup(void) > > > const char *isa, **exts; > > > struct udevice *dev; > > > + if (!CONFIG_IS_ENABLED(CPU)) > > > + return 0; > > > > Should we return zero here, or -ENOENT like the original behavior when > > no UCLASS_CPU device is found? My bad, the original return value when CONFIG_CPU is disabled should be -ENODEV instead of -ENOENT. But in case that CONFIG_CPU is disabled, -EOPNOTSUPP might be a better outcome. I don't have a strong opinion on this. > if you put there error value then when CONFIG_CPU is disable you get boot > hang which is likely what you don't want to see right? > > initcall_run_f(): initcall initf_dm() failed > ### ERROR ### Please RESET the board ### This should be the original behavior, i.e. U-Boot built with CONFIG_CPU=n and CONFIG_EVENT=y is broken without this patch, too, right? int riscv_cpu_setup(void) { int ret = -ENODEV, ext_count, i; const char *isa, **exts; struct udevice *dev; uclass_find_first_device(UCLASS_CPU, &dev); if (!dev) { debug("unable to find the RISC-V cpu device\n"); return ret; } ... } EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, riscv_cpu_setup); When CONFIG_CPU is disabled, the call to uclass_find_first_device() would always find no device, and riscv_cpu_setup() would return with -ENODEV, causing the call to event_notify_null() in dm_init_and_scan() to fail. We could fix this by guarding the instantiation of EVENT_SPY_SIMPLE() within #if CONFIG_IS_ENABLED(CPU). I consider returning with error a useful behavior, since there are raw calls to this function (board/[starfive,thead]/*/spl.c), in which cases a correct return value suggests something goes wrong, thus might help debugging, for example, if someone turns off CONFIG_CPU but doesn't expect the side effects. Anyway, whether you agree on the return value or not, it should be mentioned in the commit message that U-Boot built with CONFIG_CPU=n and COFNIG_EVENT=y is broken without the patch. > Thanks, > Michal Best regards, Yao Zi
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index bbadd0c9a469..3bec7c7cb6d0 100644 --- a/arch/riscv/cpu/cpu.c +++ b/arch/riscv/cpu/cpu.c @@ -638,6 +638,9 @@ int riscv_cpu_setup(void) const char *isa, **exts; struct udevice *dev; + if (!CONFIG_IS_ENABLED(CPU)) + return 0; + uclass_find_first_device(UCLASS_CPU, &dev); if (!dev) { debug("unable to find the RISC-V cpu device\n");
Building on commit c64fc632a86a ("riscv: cpu: Use CONFIG_IS_ENABLED(CPU) instead of plain ifdef"), add an early return in riscv_cpu_setup() when CONFIG_CPU is not enabled. This allows platforms to save code space in SPL by disabling CONFIG_SPL_CPU. The compiler's dead-code elimination combined with --gc-sections removes the unreachable code and all associated static data, achieving significant size reduction without preprocessor guards: spl/u-boot-spl:all -4332 spl/u-boot-spl:rodata -2872 spl/u-boot-spl:text -1460 Signed-off-by: Michal Simek <michal.simek@amd.com> --- arch/riscv/cpu/cpu.c | 3 +++ 1 file changed, 3 insertions(+)