Hi Bin

From: Bin Meng [mailto:bmeng.cn@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 ycliang@andestech.com wrote:

...

On Sun, Jul 19, 2020 at 11:17:07PM -0700, Bin Meng wrote:

...

From: Bin Meng bin.meng@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@windriver.com Reviewed-by: Sean Anderson seanga2@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 ycliang@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