Hi Bin > From: Bin Meng [mailto:bmeng...@gmail.com] > Sent: Friday, July 24, 2020 11:26 AM > To: Leo Yu-Chi Liang(梁育齊); Rick Jian-Zhi Chen(陳建志) > Cc: U-Boot Mailing List > Subject: Re: [PATCH v4] riscv: Make SiFive HiFive Unleashed board boot again > > 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?
OK. I am preparing to send a PR later today. Thanks, Rick > > Regards, > Bin
