Hi Lukas >Auer, Lukas <lukas.a...@aisec.fraunhofer.de> 於 2019年4月1日 週一 下午5:08寫道： > > Hi Rick, > > On Mon, 2019-04-01 at 16:24 +0800, Andes wrote: > > From: Rick Chen <r...@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 <r...@andestech.com> > > Cc: Greentime Hu <greent...@andestech.com> > > --- > > V3: > > - Rename plic_init() as enable_ipi(). > > - Declase as static. > > - Remove PLIC_BASE_GET() from enable_ipi(). > > > > Take a look at patman [1], it makes it really easy to handle different > versions of a patch series. :)
OK Thanks :) > > [1]: https://github.com/u-boot/u-boot/blob/master/tools/patman/README > > > 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/andes_plic.c | 110 > > +++++++++++++++++++++++++++++++++++ > > 5 files changed, 124 insertions(+), 1 deletion(-) > > create mode 100644 arch/riscv/lib/andes_plic.c > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 3a4470d..511768b 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 ANDES_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..b867910 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_ANDES_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..c1b4b86 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 > > * > > nit: can you also drop the empty comment line above? OK > > > - * 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 Interrupt Controller (PLIC) > > */ > > }; > > > > #endif /* _ASM_SYSCON_H */ > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > index 35dbf64..1bf554b 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_ANDES_PLIC) += andes_plic.o > > obj-y += interrupts.o > > obj-y += reset.o > > obj-$(CONFIG_SBI_IPI) += sbi_ipi.o > > diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c > > new file mode 100644 > > index 0000000..67ab561 > > --- /dev/null > > +++ b/arch/riscv/lib/andes_plic.c > > @@ -0,0 +1,110 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2019, Rick Chen <r...@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 <dm/device-internal.h> > > +#include <dm/lists.h> > > +#include <dm/uclass-internal.h> > > +#include <regmap.h> > > +#include <syscon.h> > > +#include <asm/io.h> > > +#include <asm/syscon.h> > > +#include <cpu.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) > > + > > +#define ENABLE_HART_IPI (0x80808080) > > +#define SEND_IPI_TO_HART(hart) (0x80>>hart) > > + > > +DECLARE_GLOBAL_DATA_PTR; > > +static int init_plic(void); > > + > > +#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; \ > > + init_plic(); \ > > + } \ > > + } while (0) > > + > > +static int enable_ipi(int harts) > > +{ > > + int i; > > + int en = ENABLE_HART_IPI; > > + > > + for (i = 0; i < harts ;i++) > > + { > > + en = en >> i; > > + writel(en, (void __iomem *)ENABLE_REG(gd->arch.plic, i)); > > + } > > + > > + return 0; > > +} > > + > > +static int init_plic(void) > > +{ > > + struct udevice *dev; > > + int ret; > > + > > + ret = uclass_find_first_device(UCLASS_CPU, &dev); > > + if (ret) > > + return ret; > > + > > + if (ret == 0 && dev != NULL) { > > + ret = cpu_get_count(dev); > > You should check first, if cpu_get_count() has returned an error before > using its return value. OK I will check the return value from cpu_get_count(). > > > + enable_ipi(ret); > > + return 0; > > + } > > + > > + return -ENODEV; > > +} > > + > > +int riscv_send_ipi(int hart) > > +{ > > + PLIC_BASE_GET(); > > + > > + writel(SEND_IPI_TO_HART(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 andes_plic_ids[] = { > > + { .compatible = "riscv,plic1", .data = RISCV_SYSCON_PLIC }, > > Would a compatible string of "andes,plic1" be more suitable? I got and sync the dts from our kernel team. If I shall change the string, I need to discuss with them. Some of them are not in office this week. So I prefer not change in this series. > > > + { } > > +}; > > + > > +U_BOOT_DRIVER(nds_plic) = { > > nit: andes_plic OK > + .name = "andes_plic", > > + .id = UCLASS_SYSCON, > > + .of_match = andes_plic_ids, > > + .flags = DM_FLAG_PRE_RELOC, > > +}; > > There are a couple of checkpatch issues in this patch. I have copied > the relevant ones below. > I will check patchs in the next patchsets Thanks Rick > CHECK: spaces preferred around that '>>' (ctx:VxV) > #135: FILE: arch/riscv/lib/andes_plic.c:29: > +#define SEND_IPI_TO_HART(hart) (0x80>>hart) > > ERROR: that open brace { should be on the previous line > #158: FILE: arch/riscv/lib/andes_plic.c:52: > + for (i = 0; i < harts ;i++) > + { > > ERROR: space required after that ';' (ctx:WxV) > #158: FILE: arch/riscv/lib/andes_plic.c:52: > + for (i = 0; i < harts ;i++) > > CHECK: Comparison to NULL could be written "dev" > #176: FILE: arch/riscv/lib/andes_plic.c:70: > + if (ret == 0 && dev != NULL) { > > WARNING: line over 80 characters > #189: FILE: arch/riscv/lib/andes_plic.c:83: > + writel(SEND_IPI_TO_HART(hart), (void __iomem *)PENDING_REG(gd- > >arch.plic, gd->arch.boot_hart)); > > Thanks, > Lukas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
