[U-Boot] [PATCH 4/8] riscv: andes_plic: Fix some wrong configurations

[Rick Chen]

Hi Bin

>
> Hi Rick,
>
> On Fri, Oct 25, 2019 at 2:18 PM Andes <uboot at andestech.com> wrote:
> >
> > From: Rick Chen <rick at andestech.com>
> >
> > It will work fine due to hart 0 always will be main
> > hart coincidentally. When develop SPL flow, I try to
> > force other harts to be main hart. And it will go
> > wrong in sending IPI flow. So fix it.
>
> Fix what? Does this commit contain 2 fixes, or just 1 fix?

Yes, it include two fixs. But they will cause one negative result
that only hart 0 can send ipi to other harts.

>
> >
> > Having this fix, any hart can be main hart in U-Boot SPL
> > theoretically, but it still fail somewhere. After dig in
> > and found there is an assumption that hart 0 shall be
> > main hart in OpenSbi.
>
> So does this mean there is a bug in OpenSBI too?

I am not sure if it is a bug. Maybe it is a compatible issue.
There is a limitation that only hart 0 can be main hart in OpenSBI.
But any hart can be main hart in U-Boot.

In general case, hart 0 will be main and it is fine when U-Boot jump ot OpenSBI.
But if we force hart 1 to be main hart, when hart 0 jump to OPenSBI from U-Boot,
It will do relocation flow in OpenSBI which willcorrupt U-Boot SPL,
but hart 0 still in U-Boot SPL.

>
> >
> > After some work-arounds, it can pass the verifications
> > that any hart can be main hart and boots U-Boot SPL ->
> > OpenSbi -> U-Boot proper -> Linux Kernel successfully.
> >
>
> It's a bit hard for me to understand what was described here in the
> commit message. Maybe you need rewrite something.

OK. I will rewrite this commit message.

>
> > Signed-off-by: Rick Chen <rick at andestech.com>
> > Cc: KC Lin <kclin at andestech.com>
> > Cc: Alan Kao <alankao at andestech.com>
> > ---
> >  arch/riscv/lib/andes_plic.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c
> > index 28568e4..42394b9 100644
> > --- a/arch/riscv/lib/andes_plic.c
> > +++ b/arch/riscv/lib/andes_plic.c
> > @@ -19,7 +19,7 @@
> >  #include <cpu.h>
> >
> >  /* pending register */
> > -#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + (hart) * 8)
> > +#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + ((hart) / 4) * 4)
> >  /* enable register */
> >  #define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
> >  /* claim register */
> > @@ -46,7 +46,7 @@ static int init_plic(void);
> >
> >  static int enable_ipi(int hart)
> >  {
> > -       int en;
> > +       unsigned int en;
>
> Is this for some compiler warning fix?

No, it is not a warning fix. It is a bug fix.
I hope en can be 0x0000000080808080 instead of 0xffffffff80808080,
or it will cause IPI sending errors.

Thanks
Rick

>
> >
> >         en = ENABLE_HART_IPI >> hart;
> >         writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, hart));
> > @@ -94,10 +94,13 @@ static int init_plic(void)
> >
> >  int riscv_send_ipi(int hart)
> >  {
> > +       unsigned int ipi;
> > +
> >         PLIC_BASE_GET();
> >
> > -       writel(SEND_IPI_TO_HART(hart),
> > -              (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));
> > +       ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart));
> > +       writel(ipi, (void __iomem *)PENDING_REG(gd->arch.plic,
> > +                               gd->arch.boot_hart));
> >
> >         return 0;
> >  }
> > --
>
> Regards,
> Bin