Hi Rick,
On Tue, Jul 21, 2020 at 4:30 PM Leo Liang <ycli...@andestech.com> wrote:
>
> On Sun, Jul 19, 2020 at 11:17:07PM -0700, Bin Meng wrote:
> > From: Bin Meng <bin.m...@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.m...@windriver.com>
> > Reviewed-by: Sean Anderson <sean...@gmail.com>
> > ---
> >
> > Changes in v4:
> > - Include Sean's "Reviewed-by" tag
> >
> > 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.
> > *
> > --
> > 2.7.4
> >
>
> Tested-by: Leo Liang <ycli...@andestech.com>
Could we get this applied soon, since it's broken in u-boot/master?
Regards,
Bin
