[PATCH v3] riscv: Make SiFive HiFive Unleashed board boot again

[Bin Meng]

On Mon, Jul 20, 2020 at 1:06 PM Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 7/20/20 12:33 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng at windriver.com>
> >
> > Commit 40686c394e53 ("riscv: Clean up IPI initialization code")
> > caused U-Boot failed to boot on SiFive HiFive Unleashed board.
> >
> > The codes inside arch_cpu_init_dm() may call U-Boot timer APIs
> > before the call to riscv_init_ipi(). At that time the timer register
> > base (e.g.: the SiFive CLINT device in this case) is unknown yet.
> >
> > It might be the name riscv_init_ipi() that misleads people to only
> > consider it is related to IPI, but in fact the timer capability is
> > provided by the same SiFive CLINT device that provides the IPI.
> > Timer capability is needed for both UP and SMP.
> >
> > Considering that the original refactor does have benefits, that it
> > makes the IPI code more similar to U-boot initialization idioms.
> > It also removes some quite ugly macros. Let's do the minimal revert
> > instead of a complete revert, plus a fixes to arch_cpu_init_dm() to
> > consider the SPL case.
> >
> > Fixes: 40686c394e53 ("riscv: Clean up IPI initialization code")
> > Signed-off-by: Bin Meng <bin.meng at windriver.com>
> > ---
> >
> > Changes in v3:
> > - Simply call riscv_init_ipi() in clint timer functions to avoid
> >   some duplications
> >
> > Changes in v2:
> > - Do the minimal partial revert instead of a complete revert, enough
> >   to make HiFive Unleashed board boot again.
> >
> >  arch/riscv/cpu/cpu.c          |  2 +-
> >  arch/riscv/lib/sifive_clint.c | 16 ++++++++++++----
> >  common/spl/spl_opensbi.c      |  5 -----
> >  3 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > index bbd6c15..bfa2d4a 100644
> > --- a/arch/riscv/cpu/cpu.c
> > +++ b/arch/riscv/cpu/cpu.c
> > @@ -107,7 +107,7 @@ int arch_cpu_init_dm(void)
> >  #endif
> >       }
> >
> > -#ifdef CONFIG_SMP
> > +#if CONFIG_IS_ENABLED(SMP)
> >       ret = riscv_init_ipi();
> >       if (ret)
> >               return ret;
> > diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
> > index 78fc6c8..b9a2c64 100644
> > --- a/arch/riscv/lib/sifive_clint.c
> > +++ b/arch/riscv/lib/sifive_clint.c
> > @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
> >
> >  int riscv_get_time(u64 *time)
> >  {
> > +     /* ensure timer register base has a sane value */
> > +     riscv_init_ipi();
> > +
> >       *time = readq((void __iomem *)MTIME_REG(gd->arch.clint));
> >
> >       return 0;
> > @@ -33,6 +36,9 @@ int riscv_get_time(u64 *time)
> >
> >  int riscv_set_timecmp(int hart, u64 cmp)
> >  {
> > +     /* ensure timer register base has a sane value */
> > +     riscv_init_ipi();
> > +
> >       writeq(cmp, (void __iomem *)MTIMECMP_REG(gd->arch.clint, hart));
> >
> >       return 0;
> > @@ -40,11 +46,13 @@ int riscv_set_timecmp(int hart, u64 cmp)
> >
> >  int riscv_init_ipi(void)
> >  {
> > -     long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT);
> > +     if (!gd->arch.clint) {
> > +             long *ret = syscon_get_first_range(RISCV_SYSCON_CLINT);
> >
> > -     if (IS_ERR(ret))
> > -             return PTR_ERR(ret);
> > -     gd->arch.clint = ret;
> > +             if (IS_ERR(ret))
> > +                     return PTR_ERR(ret);
> > +             gd->arch.clint = ret;
> > +     }
> >
> >       return 0;
> >  }
> > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
> > index 3440bc0..14f335f 100644
> > --- a/common/spl/spl_opensbi.c
> > +++ b/common/spl/spl_opensbi.c
> > @@ -79,11 +79,6 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
> >       invalidate_icache_all();
> >
> >  #ifdef CONFIG_SPL_SMP
> > -     /* Initialize the IPI before we use it */
> > -     ret = riscv_init_ipi();
> > -     if (ret)
> > -             hang();
> > -
> >       /*
> >        * Start OpenSBI on all secondary harts and wait for acknowledgment.
> >        *
> >
>
> Reviwed-by: Sean Anderson <seanga2 at gmail.com>

There is a typo in the tag. I will include the correct tag in v4.

Regards,
Bin