All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled
@ 2026-04-30 11:57 Michal Simek
  2026-05-01  8:54 ` Yao Zi
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Simek @ 2026-04-30 11:57 UTC (permalink / raw)
  To: u-boot, git; +Cc: Leo, Rick Chen, Tom Rini, Yao Zi

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;
+
 	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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled
  2026-04-30 11:57 [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled Michal Simek
@ 2026-05-01  8:54 ` Yao Zi
  2026-05-04  9:00   ` Michal Simek
  0 siblings, 1 reply; 4+ messages in thread
From: Yao Zi @ 2026-05-01  8:54 UTC (permalink / raw)
  To: Michal Simek, u-boot, git; +Cc: Leo, Rick Chen, Tom Rini

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled
  2026-05-01  8:54 ` Yao Zi
@ 2026-05-04  9:00   ` Michal Simek
  2026-05-04 20:16     ` Yao Zi
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Simek @ 2026-05-04  9:00 UTC (permalink / raw)
  To: Yao Zi, u-boot, git; +Cc: Leo, Rick Chen, Tom Rini



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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled
  2026-05-04  9:00   ` Michal Simek
@ 2026-05-04 20:16     ` Yao Zi
  0 siblings, 0 replies; 4+ messages in thread
From: Yao Zi @ 2026-05-04 20:16 UTC (permalink / raw)
  To: Michal Simek, u-boot, git; +Cc: Leo, Rick Chen, Tom Rini

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-04 20:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 11:57 [PATCH] riscv: Skip riscv_cpu_setup() when CPU driver is disabled Michal Simek
2026-05-01  8:54 ` Yao Zi
2026-05-04  9:00   ` Michal Simek
2026-05-04 20:16     ` Yao Zi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.