diff mbox series

[U-Boot,2/9] riscv: Add a SYSCON driver for Andestech's PLIC

Message ID 20190319090750.8923-3-uboot@andestech.com
State Superseded
Delegated to: Andes
Headers show
Series AE350 SMP support RISC-V | expand

Commit Message

Andes March 19, 2019, 9:07 a.m. UTC
From: Rick Chen <rick@andestech.com>

The Platform-Level Interrupt Controller(PLIC)
block holds memory-mapped claim and pending registers
associated with software interrupt.It is required
for handling IPI.

Signed-off-by: Rick Chen <rick@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
---
 arch/riscv/Kconfig                   |  9 ++++
 arch/riscv/include/asm/global_data.h |  3 ++
 arch/riscv/include/asm/syscon.h      |  2 +-
 arch/riscv/lib/Makefile              |  1 +
 arch/riscv/lib/nds_plic.c            | 84 ++++++++++++++++++++++++++++++++++++
 5 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/lib/nds_plic.c

Comments

Bin Meng March 20, 2019, 7:22 a.m. UTC | #1
Hi Rick,

On Tue, Mar 19, 2019 at 5:12 PM Andes <uboot@andestech.com> wrote:
>
> From: Rick Chen <rick@andestech.com>
>
> The Platform-Level Interrupt Controller(PLIC)
> block holds memory-mapped claim and pending registers
> associated with software interrupt.It is required

nits: need one space after interrupt.

> for handling IPI.
>
> Signed-off-by: Rick Chen <rick@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> ---
>  arch/riscv/Kconfig                   |  9 ++++
>  arch/riscv/include/asm/global_data.h |  3 ++
>  arch/riscv/include/asm/syscon.h      |  2 +-
>  arch/riscv/lib/Makefile              |  1 +
>  arch/riscv/lib/nds_plic.c            | 84 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 98 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/lib/nds_plic.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 3a4470d..fef11dd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig

Probably it makes more sense to put this to arch/riscv/cpu/ax25/Kconfig?

> @@ -109,6 +109,15 @@ config SIFIVE_CLINT
>           The SiFive CLINT block holds memory-mapped control and status registers
>           associated with software and timer interrupts.
>
> +config NDS_PLIC

I am not sure if it is appropriate to call this "NDS_PLIC". Shouldn't
it be "ANDES_PLIC", because ANDES_ and SIFIVE_ are vendor prefixes.

> +       bool
> +       depends on RISCV_MMODE
> +       select REGMAP
> +       select SYSCON
> +       help
> +         The Andes PLIC block holds memory-mapped claim and pending registers
> +         associated with software interrupt.
> +
>  config RISCV_RDTIME
>         bool
>         default y if RISCV_SMODE
> diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> index 80e3165..15867f5 100644
> --- a/arch/riscv/include/asm/global_data.h
> +++ b/arch/riscv/include/asm/global_data.h
> @@ -18,6 +18,9 @@ struct arch_global_data {
>  #ifdef CONFIG_SIFIVE_CLINT
>         void __iomem *clint;    /* clint base address */
>  #endif
> +#ifdef CONFIG_NDS_PLIC
> +       void __iomem *plic;     /* plic base address */
> +#endif
>  #ifdef CONFIG_SMP
>         struct ipi_data ipi[CONFIG_NR_CPUS];
>  #endif
> diff --git a/arch/riscv/include/asm/syscon.h b/arch/riscv/include/asm/syscon.h
> index d311ee6..0229989 100644
> --- a/arch/riscv/include/asm/syscon.h
> +++ b/arch/riscv/include/asm/syscon.h
> @@ -9,11 +9,11 @@
>  /*
>   * System controllers in a RISC-V system
>   *
> - * So far only SiFive's Core Local Interruptor (CLINT) is defined.
>   */
>  enum {
>         RISCV_NONE,
>         RISCV_SYSCON_CLINT,     /* Core Local Interruptor (CLINT) */
> +       RISCV_SYSCON_PLIC,      /* Platform Level Interrup Controller (PLIC) */

typo: Interrupt

>  };
>
>  #endif /* _ASM_SYSCON_H */
> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> index 35dbf64..8187c2b 100644
> --- a/arch/riscv/lib/Makefile
> +++ b/arch/riscv/lib/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_CMD_GO) += boot.o
>  obj-y  += cache.o
>  obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
>  obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> +obj-$(CONFIG_NDS_PLIC) += nds_plic.o
>  obj-y  += interrupts.o
>  obj-y  += reset.o
>  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c

And move this driver to arch/riscv/cpu/ax25 since it's only available
in AX25 CPUs?

> new file mode 100644
> index 0000000..563da7d
> --- /dev/null
> +++ b/arch/riscv/lib/nds_plic.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019, Rick Chen <rick@andestech.com>
> + *
> + * U-Boot syscon driver for Andes's Platform Level Interrupt Controller (PLIC).
> + * The PLIC block holds memory-mapped claim and pending registers
> + * associated with software interrupt.
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include <asm/syscon.h>
> +
> +/* pending register */
> +#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + (hart) * 8)
> +/* enable register */
> +#define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
> +/* claim register */
> +#define CLAIM_REG(base, hart)  ((ulong)(base) + 0x200004 + (hart) * 0x1000)
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define PLIC_BASE_GET(void)                                            \
> +       do {                                                            \
> +               long *ret;                                              \
> +                                                                       \
> +               if (!gd->arch.plic) {                                   \
> +                       ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \
> +                       if (IS_ERR(ret))                                \
> +                               return PTR_ERR(ret);                    \
> +                       gd->arch.plic = ret;                            \
> +               }                                                       \
> +       } while (0)
> +
> +int plic_init(int harts)

Can we make this function be automatically called in PLIC_BASE_GET()?

> +{
> +       int i;
> +       int en = 0x80808080;

Can we use some macros for this?

> +
> +       PLIC_BASE_GET();
> +       for(i=0;i<harts;i++)

nits: should have various spaces like i = 0;

> +       {
> +               en = en >> i;
> +               writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i));
> +       }
> +
> +       return 0;
> +}
> +
> +int riscv_send_ipi(int hart)
> +{
> +       PLIC_BASE_GET();
> +
> +       writel((0x80>>hart), (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));

macro for 0x80?

> +
> +       return 0;
> +}
> +
> +int riscv_clear_ipi(int hart)
> +{
> +       u32 source_id;
> +
> +       PLIC_BASE_GET();
> +
> +       source_id = readl((void __iomem *)CLAIM_REG(gd->arch.plic, hart));
> +       writel(source_id, (void __iomem *)CLAIM_REG(gd->arch.plic, hart));
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id nds_plic_ids[] = {
> +       { .compatible = "riscv,plic1", .data = RISCV_SYSCON_PLIC },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(nds_plic) = {
> +       .name           = "nds_plic",
> +       .id             = UCLASS_SYSCON,
> +       .of_match       = nds_plic_ids,
> +       .flags          = DM_FLAG_PRE_RELOC,
> +};
> --

Regards,
Bin
Rick Chen March 21, 2019, 7:04 a.m. UTC | #2
Hi Bin

Bin Meng <bmeng.cn@gmail.com> 於 2019年3月20日 週三 下午3:22寫道:
>
> Hi Rick,
>
> On Tue, Mar 19, 2019 at 5:12 PM Andes <uboot@andestech.com> wrote:
> >
> > From: Rick Chen <rick@andestech.com>
> >
> > The Platform-Level Interrupt Controller(PLIC)
> > block holds memory-mapped claim and pending registers
> > associated with software interrupt.It is required
>
> nits: need one space after interrupt.

OK
I will add space.

>
> > for handling IPI.
> >
> > Signed-off-by: Rick Chen <rick@andestech.com>
> > Cc: Greentime Hu <greentime@andestech.com>
> > ---
> >  arch/riscv/Kconfig                   |  9 ++++
> >  arch/riscv/include/asm/global_data.h |  3 ++
> >  arch/riscv/include/asm/syscon.h      |  2 +-
> >  arch/riscv/lib/Makefile              |  1 +
> >  arch/riscv/lib/nds_plic.c            | 84 ++++++++++++++++++++++++++++++++++++
> >  5 files changed, 98 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/riscv/lib/nds_plic.c
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 3a4470d..fef11dd 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
>
> Probably it makes more sense to put this to arch/riscv/cpu/ax25/Kconfig?

I just refer to SIFIVE_CLINT. It also not make sense to place here, right ?

>
> > @@ -109,6 +109,15 @@ config SIFIVE_CLINT
> >           The SiFive CLINT block holds memory-mapped control and status registers
> >           associated with software and timer interrupts.
> >
> > +config NDS_PLIC
>
> I am not sure if it is appropriate to call this "NDS_PLIC". Shouldn't
> it be "ANDES_PLIC", because ANDES_ and SIFIVE_ are vendor prefixes.

OK
I will use ANDES_PLIC to replace NDS_PLIC.

>
> > +       bool
> > +       depends on RISCV_MMODE
> > +       select REGMAP
> > +       select SYSCON
> > +       help
> > +         The Andes PLIC block holds memory-mapped claim and pending registers
> > +         associated with software interrupt.
> > +
> >  config RISCV_RDTIME
> >         bool
> >         default y if RISCV_SMODE
> > diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> > index 80e3165..15867f5 100644
> > --- a/arch/riscv/include/asm/global_data.h
> > +++ b/arch/riscv/include/asm/global_data.h
> > @@ -18,6 +18,9 @@ struct arch_global_data {
> >  #ifdef CONFIG_SIFIVE_CLINT
> >         void __iomem *clint;    /* clint base address */
> >  #endif
> > +#ifdef CONFIG_NDS_PLIC
> > +       void __iomem *plic;     /* plic base address */
> > +#endif
> >  #ifdef CONFIG_SMP
> >         struct ipi_data ipi[CONFIG_NR_CPUS];
> >  #endif
> > diff --git a/arch/riscv/include/asm/syscon.h b/arch/riscv/include/asm/syscon.h
> > index d311ee6..0229989 100644
> > --- a/arch/riscv/include/asm/syscon.h
> > +++ b/arch/riscv/include/asm/syscon.h
> > @@ -9,11 +9,11 @@
> >  /*
> >   * System controllers in a RISC-V system
> >   *
> > - * So far only SiFive's Core Local Interruptor (CLINT) is defined.
> >   */
> >  enum {
> >         RISCV_NONE,
> >         RISCV_SYSCON_CLINT,     /* Core Local Interruptor (CLINT) */
> > +       RISCV_SYSCON_PLIC,      /* Platform Level Interrup Controller (PLIC) */
>
> typo: Interrupt

OK
I will correct it.

>
> >  };
> >
> >  #endif /* _ASM_SYSCON_H */
> > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > index 35dbf64..8187c2b 100644
> > --- a/arch/riscv/lib/Makefile
> > +++ b/arch/riscv/lib/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_CMD_GO) += boot.o
> >  obj-y  += cache.o
> >  obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> >  obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> > +obj-$(CONFIG_NDS_PLIC) += nds_plic.o
> >  obj-y  += interrupts.o
> >  obj-y  += reset.o
> >  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> > diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c
>
> And move this driver to arch/riscv/cpu/ax25 since it's only available
> in AX25 CPUs?

Same as sifive_clint.c . Shall it also move away from /lib ?

>
> > new file mode 100644
> > index 0000000..563da7d
> > --- /dev/null
> > +++ b/arch/riscv/lib/nds_plic.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019, Rick Chen <rick@andestech.com>
> > + *
> > + * U-Boot syscon driver for Andes's Platform Level Interrupt Controller (PLIC).
> > + * The PLIC block holds memory-mapped claim and pending registers
> > + * associated with software interrupt.
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <regmap.h>
> > +#include <syscon.h>
> > +#include <asm/io.h>
> > +#include <asm/syscon.h>
> > +
> > +/* pending register */
> > +#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + (hart) * 8)
> > +/* enable register */
> > +#define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
> > +/* claim register */
> > +#define CLAIM_REG(base, hart)  ((ulong)(base) + 0x200004 + (hart) * 0x1000)
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define PLIC_BASE_GET(void)                                            \
> > +       do {                                                            \
> > +               long *ret;                                              \
> > +                                                                       \
> > +               if (!gd->arch.plic) {                                   \
> > +                       ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \
> > +                       if (IS_ERR(ret))                                \
> > +                               return PTR_ERR(ret);                    \
> > +                       gd->arch.plic = ret;                            \
> > +               }                                                       \
> > +       } while (0)
> > +
> > +int plic_init(int harts)
>
> Can we make this function be automatically called in PLIC_BASE_GET()?

OK
I will move it in PLIC_BASE_GET()

>
> > +{
> > +       int i;
> > +       int en = 0x80808080;
>
> Can we use some macros for this?

OK
I will use macro to represent it.

>
> > +
> > +       PLIC_BASE_GET();
> > +       for(i=0;i<harts;i++)
>
> nits: should have various spaces like i = 0;

OK

>
> > +       {
> > +               en = en >> i;
> > +               writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i));
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +int riscv_send_ipi(int hart)
> > +{
> > +       PLIC_BASE_GET();
> > +
> > +       writel((0x80>>hart), (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));
>
> macro for 0x80?

OK
I will use macro to represent it.

Thanks for review.

Rick

>
> > +
> > +       return 0;
> > +}
> > +
> > +int riscv_clear_ipi(int hart)
> > +{
> > +       u32 source_id;
> > +
> > +       PLIC_BASE_GET();
> > +
> > +       source_id = readl((void __iomem *)CLAIM_REG(gd->arch.plic, hart));
> > +       writel(source_id, (void __iomem *)CLAIM_REG(gd->arch.plic, hart));
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct udevice_id nds_plic_ids[] = {
> > +       { .compatible = "riscv,plic1", .data = RISCV_SYSCON_PLIC },
> > +       { }
> > +};
> > +
> > +U_BOOT_DRIVER(nds_plic) = {
> > +       .name           = "nds_plic",
> > +       .id             = UCLASS_SYSCON,
> > +       .of_match       = nds_plic_ids,
> > +       .flags          = DM_FLAG_PRE_RELOC,
> > +};
> > --
>
> Regards,
> Bin
Bin Meng March 21, 2019, 7:32 a.m. UTC | #3
Hi Rick,

On Thu, Mar 21, 2019 at 3:04 PM Rick Chen <rickchen36@gmail.com> wrote:
>
> Hi Bin
>
> Bin Meng <bmeng.cn@gmail.com> 於 2019年3月20日 週三 下午3:22寫道:
> >
> > Hi Rick,
> >
> > On Tue, Mar 19, 2019 at 5:12 PM Andes <uboot@andestech.com> wrote:
> > >
> > > From: Rick Chen <rick@andestech.com>
> > >
> > > The Platform-Level Interrupt Controller(PLIC)
> > > block holds memory-mapped claim and pending registers
> > > associated with software interrupt.It is required
> >
> > nits: need one space after interrupt.
>
> OK
> I will add space.
>
> >
> > > for handling IPI.
> > >
> > > Signed-off-by: Rick Chen <rick@andestech.com>
> > > Cc: Greentime Hu <greentime@andestech.com>
> > > ---
> > >  arch/riscv/Kconfig                   |  9 ++++
> > >  arch/riscv/include/asm/global_data.h |  3 ++
> > >  arch/riscv/include/asm/syscon.h      |  2 +-
> > >  arch/riscv/lib/Makefile              |  1 +
> > >  arch/riscv/lib/nds_plic.c            | 84 ++++++++++++++++++++++++++++++++++++
> > >  5 files changed, 98 insertions(+), 1 deletion(-)
> > >  create mode 100644 arch/riscv/lib/nds_plic.c
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 3a4470d..fef11dd 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> >
> > Probably it makes more sense to put this to arch/riscv/cpu/ax25/Kconfig?
>
> I just refer to SIFIVE_CLINT. It also not make sense to place here, right ?

Maybe, but since the cpu directory is renamed to 'generic', I am not
sure moving to that directory is a good idea.

>
> >
> > > @@ -109,6 +109,15 @@ config SIFIVE_CLINT
> > >           The SiFive CLINT block holds memory-mapped control and status registers
> > >           associated with software and timer interrupts.
> > >
> > > +config NDS_PLIC
> >
> > I am not sure if it is appropriate to call this "NDS_PLIC". Shouldn't
> > it be "ANDES_PLIC", because ANDES_ and SIFIVE_ are vendor prefixes.
>
> OK
> I will use ANDES_PLIC to replace NDS_PLIC.
>
> >
> > > +       bool
> > > +       depends on RISCV_MMODE
> > > +       select REGMAP
> > > +       select SYSCON
> > > +       help
> > > +         The Andes PLIC block holds memory-mapped claim and pending registers
> > > +         associated with software interrupt.
> > > +
> > >  config RISCV_RDTIME
> > >         bool
> > >         default y if RISCV_SMODE
> > > diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> > > index 80e3165..15867f5 100644
> > > --- a/arch/riscv/include/asm/global_data.h
> > > +++ b/arch/riscv/include/asm/global_data.h
> > > @@ -18,6 +18,9 @@ struct arch_global_data {
> > >  #ifdef CONFIG_SIFIVE_CLINT
> > >         void __iomem *clint;    /* clint base address */
> > >  #endif
> > > +#ifdef CONFIG_NDS_PLIC
> > > +       void __iomem *plic;     /* plic base address */
> > > +#endif
> > >  #ifdef CONFIG_SMP
> > >         struct ipi_data ipi[CONFIG_NR_CPUS];
> > >  #endif
> > > diff --git a/arch/riscv/include/asm/syscon.h b/arch/riscv/include/asm/syscon.h
> > > index d311ee6..0229989 100644
> > > --- a/arch/riscv/include/asm/syscon.h
> > > +++ b/arch/riscv/include/asm/syscon.h
> > > @@ -9,11 +9,11 @@
> > >  /*
> > >   * System controllers in a RISC-V system
> > >   *
> > > - * So far only SiFive's Core Local Interruptor (CLINT) is defined.
> > >   */
> > >  enum {
> > >         RISCV_NONE,
> > >         RISCV_SYSCON_CLINT,     /* Core Local Interruptor (CLINT) */
> > > +       RISCV_SYSCON_PLIC,      /* Platform Level Interrup Controller (PLIC) */
> >
> > typo: Interrupt
>
> OK
> I will correct it.
>
> >
> > >  };
> > >
> > >  #endif /* _ASM_SYSCON_H */
> > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > > index 35dbf64..8187c2b 100644
> > > --- a/arch/riscv/lib/Makefile
> > > +++ b/arch/riscv/lib/Makefile
> > > @@ -11,6 +11,7 @@ obj-$(CONFIG_CMD_GO) += boot.o
> > >  obj-y  += cache.o
> > >  obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> > >  obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> > > +obj-$(CONFIG_NDS_PLIC) += nds_plic.o
> > >  obj-y  += interrupts.o
> > >  obj-y  += reset.o
> > >  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> > > diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c
> >
> > And move this driver to arch/riscv/cpu/ax25 since it's only available
> > in AX25 CPUs?
>
> Same as sifive_clint.c . Shall it also move away from /lib ?
>

I agree, but see comments above :)

> >
> > > new file mode 100644
> > > index 0000000..563da7d
> > > --- /dev/null
> > > +++ b/arch/riscv/lib/nds_plic.c
> > > @@ -0,0 +1,84 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2019, Rick Chen <rick@andestech.com>
> > > + *
> > > + * U-Boot syscon driver for Andes's Platform Level Interrupt Controller (PLIC).
> > > + * The PLIC block holds memory-mapped claim and pending registers
> > > + * associated with software interrupt.
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <dm.h>
> > > +#include <regmap.h>
> > > +#include <syscon.h>
> > > +#include <asm/io.h>
> > > +#include <asm/syscon.h>
> > > +
> > > +/* pending register */
> > > +#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + (hart) * 8)
> > > +/* enable register */
> > > +#define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
> > > +/* claim register */
> > > +#define CLAIM_REG(base, hart)  ((ulong)(base) + 0x200004 + (hart) * 0x1000)
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +#define PLIC_BASE_GET(void)                                            \
> > > +       do {                                                            \
> > > +               long *ret;                                              \
> > > +                                                                       \
> > > +               if (!gd->arch.plic) {                                   \
> > > +                       ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \
> > > +                       if (IS_ERR(ret))                                \
> > > +                               return PTR_ERR(ret);                    \
> > > +                       gd->arch.plic = ret;                            \
> > > +               }                                                       \
> > > +       } while (0)
> > > +
> > > +int plic_init(int harts)
> >
> > Can we make this function be automatically called in PLIC_BASE_GET()?
>
> OK
> I will move it in PLIC_BASE_GET()
>
> >
> > > +{
> > > +       int i;
> > > +       int en = 0x80808080;
> >
> > Can we use some macros for this?
>
> OK
> I will use macro to represent it.
>
> >
> > > +
> > > +       PLIC_BASE_GET();
> > > +       for(i=0;i<harts;i++)
> >
> > nits: should have various spaces like i = 0;
>
> OK
>
> >
> > > +       {
> > > +               en = en >> i;
> > > +               writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i));
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +int riscv_send_ipi(int hart)
> > > +{
> > > +       PLIC_BASE_GET();
> > > +
> > > +       writel((0x80>>hart), (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));
> >
> > macro for 0x80?
>
> OK
> I will use macro to represent it.
>
> Thanks for review.
>

Regards,
Bin
Rick Chen March 21, 2019, 8:39 a.m. UTC | #4
Bin Meng <bmeng.cn@gmail.com> 於 2019年3月21日 週四 下午3:32寫道:
>
> Hi Rick,
>
> On Thu, Mar 21, 2019 at 3:04 PM Rick Chen <rickchen36@gmail.com> wrote:
> >
> > Hi Bin
> >
> > Bin Meng <bmeng.cn@gmail.com> 於 2019年3月20日 週三 下午3:22寫道:
> > >
> > > Hi Rick,
> > >
> > > On Tue, Mar 19, 2019 at 5:12 PM Andes <uboot@andestech.com> wrote:
> > > >
> > > > From: Rick Chen <rick@andestech.com>
> > > >
> > > > The Platform-Level Interrupt Controller(PLIC)
> > > > block holds memory-mapped claim and pending registers
> > > > associated with software interrupt.It is required
> > >
> > > nits: need one space after interrupt.
> >
> > OK
> > I will add space.
> >
> > >
> > > > for handling IPI.
> > > >
> > > > Signed-off-by: Rick Chen <rick@andestech.com>
> > > > Cc: Greentime Hu <greentime@andestech.com>
> > > > ---
> > > >  arch/riscv/Kconfig                   |  9 ++++
> > > >  arch/riscv/include/asm/global_data.h |  3 ++
> > > >  arch/riscv/include/asm/syscon.h      |  2 +-
> > > >  arch/riscv/lib/Makefile              |  1 +
> > > >  arch/riscv/lib/nds_plic.c            | 84 ++++++++++++++++++++++++++++++++++++
> > > >  5 files changed, 98 insertions(+), 1 deletion(-)
> > > >  create mode 100644 arch/riscv/lib/nds_plic.c
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index 3a4470d..fef11dd 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > >
> > > Probably it makes more sense to put this to arch/riscv/cpu/ax25/Kconfig?
> >
> > I just refer to SIFIVE_CLINT. It also not make sense to place here, right ?
>
> Maybe, but since the cpu directory is renamed to 'generic', I am not
> sure moving to that directory is a good idea.

Maybe I will still put it in /arch/riscv/Kconfig.
And we can move them to the place they belong individually together if
there have better place to accommodate to SIFIVE_CLINT.
How do you think ?

Rick

>
> >
> > >
> > > > @@ -109,6 +109,15 @@ config SIFIVE_CLINT
> > > >           The SiFive CLINT block holds memory-mapped control and status registers
> > > >           associated with software and timer interrupts.
> > > >
> > > > +config NDS_PLIC
> > >
> > > I am not sure if it is appropriate to call this "NDS_PLIC". Shouldn't
> > > it be "ANDES_PLIC", because ANDES_ and SIFIVE_ are vendor prefixes.
> >
> > OK
> > I will use ANDES_PLIC to replace NDS_PLIC.
> >
> > >
> > > > +       bool
> > > > +       depends on RISCV_MMODE
> > > > +       select REGMAP
> > > > +       select SYSCON
> > > > +       help
> > > > +         The Andes PLIC block holds memory-mapped claim and pending registers
> > > > +         associated with software interrupt.
> > > > +
> > > >  config RISCV_RDTIME
> > > >         bool
> > > >         default y if RISCV_SMODE
> > > > diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
> > > > index 80e3165..15867f5 100644
> > > > --- a/arch/riscv/include/asm/global_data.h
> > > > +++ b/arch/riscv/include/asm/global_data.h
> > > > @@ -18,6 +18,9 @@ struct arch_global_data {
> > > >  #ifdef CONFIG_SIFIVE_CLINT
> > > >         void __iomem *clint;    /* clint base address */
> > > >  #endif
> > > > +#ifdef CONFIG_NDS_PLIC
> > > > +       void __iomem *plic;     /* plic base address */
> > > > +#endif
> > > >  #ifdef CONFIG_SMP
> > > >         struct ipi_data ipi[CONFIG_NR_CPUS];
> > > >  #endif
> > > > diff --git a/arch/riscv/include/asm/syscon.h b/arch/riscv/include/asm/syscon.h
> > > > index d311ee6..0229989 100644
> > > > --- a/arch/riscv/include/asm/syscon.h
> > > > +++ b/arch/riscv/include/asm/syscon.h
> > > > @@ -9,11 +9,11 @@
> > > >  /*
> > > >   * System controllers in a RISC-V system
> > > >   *
> > > > - * So far only SiFive's Core Local Interruptor (CLINT) is defined.
> > > >   */
> > > >  enum {
> > > >         RISCV_NONE,
> > > >         RISCV_SYSCON_CLINT,     /* Core Local Interruptor (CLINT) */
> > > > +       RISCV_SYSCON_PLIC,      /* Platform Level Interrup Controller (PLIC) */
> > >
> > > typo: Interrupt
> >
> > OK
> > I will correct it.
> >
> > >
> > > >  };
> > > >
> > > >  #endif /* _ASM_SYSCON_H */
> > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
> > > > index 35dbf64..8187c2b 100644
> > > > --- a/arch/riscv/lib/Makefile
> > > > +++ b/arch/riscv/lib/Makefile
> > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_CMD_GO) += boot.o
> > > >  obj-y  += cache.o
> > > >  obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> > > >  obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> > > > +obj-$(CONFIG_NDS_PLIC) += nds_plic.o
> > > >  obj-y  += interrupts.o
> > > >  obj-y  += reset.o
> > > >  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> > > > diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c
> > >
> > > And move this driver to arch/riscv/cpu/ax25 since it's only available
> > > in AX25 CPUs?
> >
> > Same as sifive_clint.c . Shall it also move away from /lib ?
> >
>
> I agree, but see comments above :)
>
> > >
> > > > new file mode 100644
> > > > index 0000000..563da7d
> > > > --- /dev/null
> > > > +++ b/arch/riscv/lib/nds_plic.c
> > > > @@ -0,0 +1,84 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright (C) 2019, Rick Chen <rick@andestech.com>
> > > > + *
> > > > + * U-Boot syscon driver for Andes's Platform Level Interrupt Controller (PLIC).
> > > > + * The PLIC block holds memory-mapped claim and pending registers
> > > > + * associated with software interrupt.
> > > > + */
> > > > +
> > > > +#include <common.h>
> > > > +#include <dm.h>
> > > > +#include <regmap.h>
> > > > +#include <syscon.h>
> > > > +#include <asm/io.h>
> > > > +#include <asm/syscon.h>
> > > > +
> > > > +/* pending register */
> > > > +#define PENDING_REG(base, hart)        ((ulong)(base) + 0x1000 + (hart) * 8)
> > > > +/* enable register */
> > > > +#define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
> > > > +/* claim register */
> > > > +#define CLAIM_REG(base, hart)  ((ulong)(base) + 0x200004 + (hart) * 0x1000)
> > > > +
> > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > +
> > > > +#define PLIC_BASE_GET(void)                                            \
> > > > +       do {                                                            \
> > > > +               long *ret;                                              \
> > > > +                                                                       \
> > > > +               if (!gd->arch.plic) {                                   \
> > > > +                       ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \
> > > > +                       if (IS_ERR(ret))                                \
> > > > +                               return PTR_ERR(ret);                    \
> > > > +                       gd->arch.plic = ret;                            \
> > > > +               }                                                       \
> > > > +       } while (0)
> > > > +
> > > > +int plic_init(int harts)
> > >
> > > Can we make this function be automatically called in PLIC_BASE_GET()?
> >
> > OK
> > I will move it in PLIC_BASE_GET()
> >
> > >
> > > > +{
> > > > +       int i;
> > > > +       int en = 0x80808080;
> > >
> > > Can we use some macros for this?
> >
> > OK
> > I will use macro to represent it.
> >
> > >
> > > > +
> > > > +       PLIC_BASE_GET();
> > > > +       for(i=0;i<harts;i++)
> > >
> > > nits: should have various spaces like i = 0;
> >
> > OK
> >
> > >
> > > > +       {
> > > > +               en = en >> i;
> > > > +               writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i));
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +int riscv_send_ipi(int hart)
> > > > +{
> > > > +       PLIC_BASE_GET();
> > > > +
> > > > +       writel((0x80>>hart), (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));
> > >
> > > macro for 0x80?
> >
> > OK
> > I will use macro to represent it.
> >
> > Thanks for review.
> >
>
> Regards,
> Bin
Troy Benjegerdes March 21, 2019, 4:24 p.m. UTC | #5
> > > >
> > > > Probably it makes more sense to put this to arch/riscv/cpu/ax25/Kconfig?
> > >
> > > I just refer to SIFIVE_CLINT. It also not make sense to place here, right ?
> >
> > Maybe, but since the cpu directory is renamed to 'generic', I am not
> > sure moving to that directory is a good idea.
> 
> Maybe I will still put it in /arch/riscv/Kconfig.
> And we can move them to the place they belong individually together if
> there have better place to accommodate to SIFIVE_CLINT.
> How do you think ?
> 



> > > > > +++ b/arch/riscv/lib/Makefile
> > > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_CMD_GO) += boot.o
> > > > >  obj-y  += cache.o
> > > > >  obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> > > > >  obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> > > > > +obj-$(CONFIG_NDS_PLIC) += nds_plic.o
> > > > >  obj-y  += interrupts.o
> > > > >  obj-y  += reset.o
> > > > >  obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> > > > > diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c
> > > >
> > > > And move this driver to arch/riscv/cpu/ax25 since it's only available
> > > > in AX25 CPUs?
> > >
> > > Same as sifive_clint.c . Shall it also move away from /lib ?
> > >
> >
> > I agree, but see comments above :)


It seems to me there might be a usefull distinction between potentially 
generic things like the Sifive/Berkely/Rocket-chip clint [1] and other
vendor implementations which do not have necessarily have publicly 
reviewable hardware implementations.

[1] https://github.com/sifive/rocket-chip/blob/master/src/main/scala/devices/tilelink/CLINT.scala
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 3a4470d..fef11dd 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -109,6 +109,15 @@  config SIFIVE_CLINT
 	  The SiFive CLINT block holds memory-mapped control and status registers
 	  associated with software and timer interrupts.
 
+config NDS_PLIC
+	bool
+	depends on RISCV_MMODE
+	select REGMAP
+	select SYSCON
+	help
+	  The Andes PLIC block holds memory-mapped claim and pending registers
+	  associated with software interrupt.
+
 config RISCV_RDTIME
 	bool
 	default y if RISCV_SMODE
diff --git a/arch/riscv/include/asm/global_data.h b/arch/riscv/include/asm/global_data.h
index 80e3165..15867f5 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -18,6 +18,9 @@  struct arch_global_data {
 #ifdef CONFIG_SIFIVE_CLINT
 	void __iomem *clint;	/* clint base address */
 #endif
+#ifdef CONFIG_NDS_PLIC
+	void __iomem *plic;	/* plic base address */
+#endif
 #ifdef CONFIG_SMP
 	struct ipi_data ipi[CONFIG_NR_CPUS];
 #endif
diff --git a/arch/riscv/include/asm/syscon.h b/arch/riscv/include/asm/syscon.h
index d311ee6..0229989 100644
--- a/arch/riscv/include/asm/syscon.h
+++ b/arch/riscv/include/asm/syscon.h
@@ -9,11 +9,11 @@ 
 /*
  * System controllers in a RISC-V system
  *
- * So far only SiFive's Core Local Interruptor (CLINT) is defined.
  */
 enum {
 	RISCV_NONE,
 	RISCV_SYSCON_CLINT,	/* Core Local Interruptor (CLINT) */
+	RISCV_SYSCON_PLIC,	/* Platform Level Interrup Controller (PLIC) */
 };
 
 #endif /* _ASM_SYSCON_H */
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index 35dbf64..8187c2b 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -11,6 +11,7 @@  obj-$(CONFIG_CMD_GO) += boot.o
 obj-y	+= cache.o
 obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
 obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
+obj-$(CONFIG_NDS_PLIC) += nds_plic.o
 obj-y	+= interrupts.o
 obj-y	+= reset.o
 obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
diff --git a/arch/riscv/lib/nds_plic.c b/arch/riscv/lib/nds_plic.c
new file mode 100644
index 0000000..563da7d
--- /dev/null
+++ b/arch/riscv/lib/nds_plic.c
@@ -0,0 +1,84 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019, Rick Chen <rick@andestech.com>
+ *
+ * U-Boot syscon driver for Andes's Platform Level Interrupt Controller (PLIC).
+ * The PLIC block holds memory-mapped claim and pending registers
+ * associated with software interrupt.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/io.h>
+#include <asm/syscon.h>
+
+/* pending register */
+#define PENDING_REG(base, hart)	((ulong)(base) + 0x1000 + (hart) * 8)
+/* enable register */
+#define ENABLE_REG(base, hart)	((ulong)(base) + 0x2000 + (hart) * 0x80)
+/* claim register */
+#define CLAIM_REG(base, hart)	((ulong)(base) + 0x200004 + (hart) * 0x1000)
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define PLIC_BASE_GET(void)						\
+	do {								\
+		long *ret;						\
+									\
+		if (!gd->arch.plic) {					\
+			ret = syscon_get_first_range(RISCV_SYSCON_PLIC); \
+			if (IS_ERR(ret))				\
+				return PTR_ERR(ret);			\
+			gd->arch.plic = ret;				\
+		}							\
+	} while (0)
+
+int plic_init(int harts)
+{
+	int i;
+	int en = 0x80808080;
+
+	PLIC_BASE_GET();
+	for(i=0;i<harts;i++)
+	{
+		en = en >> i;
+		writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i));
+	}
+
+	return 0;
+}
+
+int riscv_send_ipi(int hart)
+{
+	PLIC_BASE_GET();
+
+	writel((0x80>>hart), (void __iomem *)PENDING_REG(gd->arch.plic, gd->arch.boot_hart));
+
+	return 0;
+}
+
+int riscv_clear_ipi(int hart)
+{
+	u32 source_id;
+
+	PLIC_BASE_GET();
+
+	source_id = readl((void __iomem *)CLAIM_REG(gd->arch.plic, hart));
+	writel(source_id, (void __iomem *)CLAIM_REG(gd->arch.plic, hart));
+
+	return 0;
+}
+
+static const struct udevice_id nds_plic_ids[] = {
+	{ .compatible = "riscv,plic1", .data = RISCV_SYSCON_PLIC },
+	{ }
+};
+
+U_BOOT_DRIVER(nds_plic) = {
+	.name		= "nds_plic",
+	.id		= UCLASS_SYSCON,
+	.of_match	= nds_plic_ids,
+	.flags		= DM_FLAG_PRE_RELOC,
+};