Re: [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled

[Yao Zi <me@ziyao.cc>]

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