Hi Tim Ouyang and Leo Liang !

Any comments on this ?

Thanks !

On Fri, 2026-09-25 at 11:38 +0000, Yao Zi wrote:
> On Fri, Sep 25, 2026 at 01:19:15PM +0300, Nikita Shubin wrote:
> > Hi Yao Zi!
> > 
> > A shy ping on this patch.
> > 
> > Thanks !
> 
> Sorry for the delay.
> 
> > On Fri, 2026-09-04 at 08:20 +0200, Michal Simek wrote:
> > > 
> > > 
> > > On 9/4/26 08:07, Nikita Shubin wrote:
> > > > If riscv_cpu_setup() is skipped we end up with riscv_init_ipi()
> > > > not
> > > > being called.
> > > > 
> > > > But we still use riscv_send_ipi() in spl_invoke_opensbi(), so
> > > > even
> > > > if
> > > > CONFIG_PHASE_CPU not set we still need init IPI and clear
> > > > pending
> > > > IPIs
> > > > on harts.
> > > > 
> > > > Fixes: d8810e1d4f82 ("riscv: Skip riscv_cpu_setup() when CPU
> > > > driver
> > > > is disabled")
> > > > Signed-off-by: Nikita Shubin <[email protected]>
> > > > ---
> > > >   arch/riscv/cpu/cpu.c | 3 ++-
> > > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > > > index 3bec7c7cb6d..a65e6951384 100644
> > > > --- a/arch/riscv/cpu/cpu.c
> > > > +++ b/arch/riscv/cpu/cpu.c
> > > > @@ -639,7 +639,7 @@ int riscv_cpu_setup(void)
> > > >         struct udevice *dev;
> > > >   
> > > >         if (!CONFIG_IS_ENABLED(CPU))
> > > > -               return 0;
> > > > +               goto init_riscv_ipi;
> > > >   
> > > >         uclass_find_first_device(UCLASS_CPU, &dev);
> > > >         if (!dev) {
> > > > @@ -687,6 +687,7 @@ int riscv_cpu_setup(void)
> > > >   #endif
> > > >         }
> > > >   
> > > > +init_riscv_ipi:
> > > >   #if CONFIG_IS_ENABLED(SMP)
> > > >         ret = riscv_init_ipi();
> > > >         if (ret)
> > > > 
> > > > ---
> > > > base-commit: cc557af4553382f6f50e3ed62b9577054e7bc54f
> > > > change-id: 20260904-fix_skipped_ipi_init-aa5575f55bc4
> > > > 
> > > > Best regards,
> > > 
> > > There are 2 other approaches how to do it in the code but it is
> > > up to
> > > maintainer 
> > > to decide. No issue from my side to solve it via goto. Obviously
> > > one
> > > big if 
> > > (CONFIG_IS_ENABLED(CPU)) is another one. Or move it to own
> > > function.
> > > 
> > > Anyway
> > > Acked-by: Michal Simek <[email protected]>
> 
> I think it's likely a bad idea to have IPI stuff in
> riscv_cpu_setup(),
> but this fix itself is okay. I should have noticed it when reviewing.
> 
> Reviewed-by: Yao Zi <[email protected]>
> 
> However, I'm RISC-V's reviewer instead of maintainer, so I couldn't
> pick
> the patch for you though.
> 
> > > Thanks,
> > > Michal
> 
> Best regards,
> Yao Zi

Reply via email to