| Message ID | 20181203052743.29036-5-anup@brainfault.org |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | [U-Boot,v7,1/4] riscv: Add kconfig option to run U-Boot in S-mode | expand |
Hi Anup, On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > When running in S-mode, we can use rdtime and rdtimeh instructions > for reading timer ticks (just like Linux). The frequency of timer > ticks is passed by prior booting stages in "timebase-frequency" DT > property of the "/cpus" DT node. > > This patch provides a generic timer implementation for U-Boot > running in S-mode. For U-Boot running in M-mode, specific timer > drivers will have to be provided. > > Signed-off-by: Anup Patel <anup@brainfault.org> > --- > arch/Kconfig | 1 - > arch/riscv/Kconfig | 22 +++++++++++---- > arch/riscv/lib/Makefile | 1 + > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 78 insertions(+), 6 deletions(-) > create mode 100644 arch/riscv/lib/time.c > > diff --git a/arch/Kconfig b/arch/Kconfig > index 9fdd2f7e66..a4fcb3522d 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -72,7 +72,6 @@ config RISCV > imply BLK > imply CLK > imply MTD > - imply TIMER No, please don't do this. > imply CMD_DM > > config SANDBOX > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 732a357a99..20a060454b 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -44,6 +44,23 @@ config ARCH_RV64I > > endchoice > > +choice > + prompt "Run Mode" > + default RISCV_MMODE > + > +config RISCV_MMODE > + bool "Machine" > + select TIMER > + help > + Choose this option to build U-Boot for RISC-V M-Mode. > + > +config RISCV_SMODE > + bool "Supervisor" > + help > + Choose this option to build U-Boot for RISC-V S-Mode. > + > +endchoice > + The above changes deserve separate patch, or merge that in the 1st patch in the series. > config RISCV_ISA_C > bool "Emit compressed instructions" > default y > @@ -55,11 +72,6 @@ config RISCV_ISA_C > config RISCV_ISA_A > def_bool y > > -config RISCV_SMODE > - bool "Run in S-Mode" > - help > - Enable this option to build U-Boot for RISC-V S-Mode > - > config 32BIT > bool > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > index b58db89752..98aa6d40e7 100644 > --- a/arch/riscv/lib/Makefile > +++ b/arch/riscv/lib/Makefile > @@ -12,6 +12,7 @@ obj-y += cache.o > obj-y += interrupts.o > obj-y += reset.o > obj-y += setjmp.o > +obj-$(CONFIG_RISCV_SMODE) += time.o > > # For building EFI apps > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > new file mode 100644 > index 0000000000..077e568ca6 > --- /dev/null > +++ b/arch/riscv/lib/time.c > @@ -0,0 +1,60 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > + */ > + > +#include <common.h> > +#include <fdtdec.h> > + > +DECLARE_GLOBAL_DATA_PTR; > + > +static unsigned int tbclk; > + > +static void setup_tbclk(void) > +{ > + int cpus; > + > + if (!gd->fdt_blob || tbclk) > + return; > + > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > + if (cpus < 0) { > + debug("%s: Missing /cpus node\n", __func__); > + return; > + } > + > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > + "timebase-frequency", 1000000); > +} > + > +ulong notrace get_tbclk(void) > +{ > + setup_tbclk(); > + > + return tbclk; > +} > + > +#ifdef CONFIG_64BIT > +uint64_t notrace get_ticks(void) > +{ > + unsigned long n; > + > + __asm__ __volatile__ ( > + "rdtime %0" > + : "=r" (n)); > + return n; > +} > +#else > +uint64_t notrace get_ticks(void) > +{ > + uint32_t lo, hi, tmp; > + __asm__ __volatile__ ( > + "1:\n" > + "rdtimeh %0\n" > + "rdtime %1\n" > + "rdtimeh %2\n" > + "bne %0, %2, 1b" > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > + return ((uint64_t)hi << 32) | lo; > +} > +#endif > -- The proper way to implement timer driver is via DM. Could you please implement a DM timer driver for S-mode? The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. Regards, Bin
On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > for reading timer ticks (just like Linux). The frequency of timer > > ticks is passed by prior booting stages in "timebase-frequency" DT > > property of the "/cpus" DT node. > > > > This patch provides a generic timer implementation for U-Boot > > running in S-mode. For U-Boot running in M-mode, specific timer > > drivers will have to be provided. > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > --- > > arch/Kconfig | 1 - > > arch/riscv/Kconfig | 22 +++++++++++---- > > arch/riscv/lib/Makefile | 1 + > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 78 insertions(+), 6 deletions(-) > > create mode 100644 arch/riscv/lib/time.c > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 9fdd2f7e66..a4fcb3522d 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -72,7 +72,6 @@ config RISCV > > imply BLK > > imply CLK > > imply MTD > > - imply TIMER > > No, please don't do this. I am removing here but selecting it only for M-mode U-Boot. > > > imply CMD_DM > > > > config SANDBOX > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 732a357a99..20a060454b 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > endchoice > > > > +choice > > + prompt "Run Mode" > > + default RISCV_MMODE > > + > > +config RISCV_MMODE > > + bool "Machine" > > + select TIMER > > + help > > + Choose this option to build U-Boot for RISC-V M-Mode. > > + > > +config RISCV_SMODE > > + bool "Supervisor" > > + help > > + Choose this option to build U-Boot for RISC-V S-Mode. > > + > > +endchoice > > + > > The above changes deserve separate patch, or merge that in the 1st > patch in the series. Sure, I will put Kconfig changes as separate patch. > > > config RISCV_ISA_C > > bool "Emit compressed instructions" > > default y > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > config RISCV_ISA_A > > def_bool y > > > > -config RISCV_SMODE > > - bool "Run in S-Mode" > > - help > > - Enable this option to build U-Boot for RISC-V S-Mode > > - > > config 32BIT > > bool > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > index b58db89752..98aa6d40e7 100644 > > --- a/arch/riscv/lib/Makefile > > +++ b/arch/riscv/lib/Makefile > > @@ -12,6 +12,7 @@ obj-y += cache.o > > obj-y += interrupts.o > > obj-y += reset.o > > obj-y += setjmp.o > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > # For building EFI apps > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > new file mode 100644 > > index 0000000000..077e568ca6 > > --- /dev/null > > +++ b/arch/riscv/lib/time.c > > @@ -0,0 +1,60 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > + */ > > + > > +#include <common.h> > > +#include <fdtdec.h> > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +static unsigned int tbclk; > > + > > +static void setup_tbclk(void) > > +{ > > + int cpus; > > + > > + if (!gd->fdt_blob || tbclk) > > + return; > > + > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > + if (cpus < 0) { > > + debug("%s: Missing /cpus node\n", __func__); > > + return; > > + } > > + > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > + "timebase-frequency", 1000000); > > +} > > + > > +ulong notrace get_tbclk(void) > > +{ > > + setup_tbclk(); > > + > > + return tbclk; > > +} > > + > > +#ifdef CONFIG_64BIT > > +uint64_t notrace get_ticks(void) > > +{ > > + unsigned long n; > > + > > + __asm__ __volatile__ ( > > + "rdtime %0" > > + : "=r" (n)); > > + return n; > > +} > > +#else > > +uint64_t notrace get_ticks(void) > > +{ > > + uint32_t lo, hi, tmp; > > + __asm__ __volatile__ ( > > + "1:\n" > > + "rdtimeh %0\n" > > + "rdtime %1\n" > > + "rdtimeh %2\n" > > + "bne %0, %2, 1b" > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > + return ((uint64_t)hi << 32) | lo; > > +} > > +#endif > > -- > > The proper way to implement timer driver is via DM. Could you please > implement a DM timer driver for S-mode? > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. "rdtime" is an instruction. It's not an device for which we can implement DM driver. This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register. We cannot use DM driver here. Regards, Anup
Hi Anup, On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > for reading timer ticks (just like Linux). The frequency of timer > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > property of the "/cpus" DT node. > > > > > > This patch provides a generic timer implementation for U-Boot > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > drivers will have to be provided. > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > --- > > > arch/Kconfig | 1 - > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > arch/riscv/lib/Makefile | 1 + > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > create mode 100644 arch/riscv/lib/time.c > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > --- a/arch/Kconfig > > > +++ b/arch/Kconfig > > > @@ -72,7 +72,6 @@ config RISCV > > > imply BLK > > > imply CLK > > > imply MTD > > > - imply TIMER > > > > No, please don't do this. > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > imply CMD_DM > > > > > > config SANDBOX > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index 732a357a99..20a060454b 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > endchoice > > > > > > +choice > > > + prompt "Run Mode" > > > + default RISCV_MMODE > > > + > > > +config RISCV_MMODE > > > + bool "Machine" > > > + select TIMER > > > + help > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > + > > > +config RISCV_SMODE > > > + bool "Supervisor" > > > + help > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > + > > > +endchoice > > > + > > > > The above changes deserve separate patch, or merge that in the 1st > > patch in the series. > > Sure, I will put Kconfig changes as separate patch. > > > > > > config RISCV_ISA_C > > > bool "Emit compressed instructions" > > > default y > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > config RISCV_ISA_A > > > def_bool y > > > > > > -config RISCV_SMODE > > > - bool "Run in S-Mode" > > > - help > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > - > > > config 32BIT > > > bool > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > index b58db89752..98aa6d40e7 100644 > > > --- a/arch/riscv/lib/Makefile > > > +++ b/arch/riscv/lib/Makefile > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > obj-y += interrupts.o > > > obj-y += reset.o > > > obj-y += setjmp.o > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > # For building EFI apps > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > new file mode 100644 > > > index 0000000000..077e568ca6 > > > --- /dev/null > > > +++ b/arch/riscv/lib/time.c > > > @@ -0,0 +1,60 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > + */ > > > + > > > +#include <common.h> > > > +#include <fdtdec.h> > > > + > > > +DECLARE_GLOBAL_DATA_PTR; > > > + > > > +static unsigned int tbclk; > > > + > > > +static void setup_tbclk(void) > > > +{ > > > + int cpus; > > > + > > > + if (!gd->fdt_blob || tbclk) > > > + return; > > > + > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > + if (cpus < 0) { > > > + debug("%s: Missing /cpus node\n", __func__); > > > + return; > > > + } > > > + > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > + "timebase-frequency", 1000000); > > > +} > > > + > > > +ulong notrace get_tbclk(void) > > > +{ > > > + setup_tbclk(); > > > + > > > + return tbclk; > > > +} > > > + > > > +#ifdef CONFIG_64BIT > > > +uint64_t notrace get_ticks(void) > > > +{ > > > + unsigned long n; > > > + > > > + __asm__ __volatile__ ( > > > + "rdtime %0" > > > + : "=r" (n)); > > > + return n; > > > +} > > > +#else > > > +uint64_t notrace get_ticks(void) > > > +{ > > > + uint32_t lo, hi, tmp; > > > + __asm__ __volatile__ ( > > > + "1:\n" > > > + "rdtimeh %0\n" > > > + "rdtime %1\n" > > > + "rdtimeh %2\n" > > > + "bne %0, %2, 1b" > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > + return ((uint64_t)hi << 32) | lo; > > > +} > > > +#endif > > > -- > > > > The proper way to implement timer driver is via DM. Could you please > > implement a DM timer driver for S-mode? > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > "rdtime" is an instruction. It's not an device for which we can > implement DM driver. > Yes, I know. But that does not mean we cannot write a driver to do the paper work. > This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register. > > We cannot use DM driver here. > The issue is that eventually we will migrate all U-Boot device drivers to DM, which means get_tbclk() and get_ticks() you implemented will be broken someday in the future. Regards, Bin
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > property of the "/cpus" DT node. > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > drivers will have to be provided. > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > --- > > > > arch/Kconfig | 1 - > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > arch/riscv/lib/Makefile | 1 + > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > --- a/arch/Kconfig > > > > +++ b/arch/Kconfig > > > > @@ -72,7 +72,6 @@ config RISCV > > > > imply BLK > > > > imply CLK > > > > imply MTD > > > > - imply TIMER > > > > > > No, please don't do this. > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > imply CMD_DM > > > > > > > > config SANDBOX > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index 732a357a99..20a060454b 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > endchoice > > > > > > > > +choice > > > > + prompt "Run Mode" > > > > + default RISCV_MMODE > > > > + > > > > +config RISCV_MMODE > > > > + bool "Machine" > > > > + select TIMER > > > > + help > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > + > > > > +config RISCV_SMODE > > > > + bool "Supervisor" > > > > + help > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > + > > > > +endchoice > > > > + > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > patch in the series. > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > config RISCV_ISA_C > > > > bool "Emit compressed instructions" > > > > default y > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > config RISCV_ISA_A > > > > def_bool y > > > > > > > > -config RISCV_SMODE > > > > - bool "Run in S-Mode" > > > > - help > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > - > > > > config 32BIT > > > > bool > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > index b58db89752..98aa6d40e7 100644 > > > > --- a/arch/riscv/lib/Makefile > > > > +++ b/arch/riscv/lib/Makefile > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > obj-y += interrupts.o > > > > obj-y += reset.o > > > > obj-y += setjmp.o > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > # For building EFI apps > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > new file mode 100644 > > > > index 0000000000..077e568ca6 > > > > --- /dev/null > > > > +++ b/arch/riscv/lib/time.c > > > > @@ -0,0 +1,60 @@ > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > +/* > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > + */ > > > > + > > > > +#include <common.h> > > > > +#include <fdtdec.h> > > > > + > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > + > > > > +static unsigned int tbclk; > > > > + > > > > +static void setup_tbclk(void) > > > > +{ > > > > + int cpus; > > > > + > > > > + if (!gd->fdt_blob || tbclk) > > > > + return; > > > > + > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > + if (cpus < 0) { > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > + return; > > > > + } > > > > + > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > + "timebase-frequency", 1000000); > > > > +} > > > > + > > > > +ulong notrace get_tbclk(void) > > > > +{ > > > > + setup_tbclk(); > > > > + > > > > + return tbclk; > > > > +} > > > > + > > > > +#ifdef CONFIG_64BIT > > > > +uint64_t notrace get_ticks(void) > > > > +{ > > > > + unsigned long n; > > > > + > > > > + __asm__ __volatile__ ( > > > > + "rdtime %0" > > > > + : "=r" (n)); > > > > + return n; > > > > +} > > > > +#else > > > > +uint64_t notrace get_ticks(void) > > > > +{ > > > > + uint32_t lo, hi, tmp; > > > > + __asm__ __volatile__ ( > > > > + "1:\n" > > > > + "rdtimeh %0\n" > > > > + "rdtime %1\n" > > > > + "rdtimeh %2\n" > > > > + "bne %0, %2, 1b" > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > + return ((uint64_t)hi << 32) | lo; > > > > +} > > > > +#endif > > > > -- > > > > > > The proper way to implement timer driver is via DM. Could you please > > > implement a DM timer driver for S-mode? > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > "rdtime" is an instruction. It's not an device for which we can > > implement DM driver. > > > > Yes, I know. But that does not mean we cannot write a driver to do the > paper work. There is no DT node for "rdtime" so how should we probe the DM driver. > > > This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register. > > > > We cannot use DM driver here. > > > > The issue is that eventually we will migrate all U-Boot device drivers > to DM, which means get_tbclk() and get_ticks() you implemented will be > broken someday in the future. Yes, I agree we should have DM driver whenever possible but sometime there is no DT bindings for things like timer because it's part of CPU instruction or CSRs Regards, Anup
Hi Anup, On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > property of the "/cpus" DT node. > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > drivers will have to be provided. > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > --- > > > > > arch/Kconfig | 1 - > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > arch/riscv/lib/Makefile | 1 + > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > --- a/arch/Kconfig > > > > > +++ b/arch/Kconfig > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > imply BLK > > > > > imply CLK > > > > > imply MTD > > > > > - imply TIMER > > > > > > > > No, please don't do this. > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > config SANDBOX > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > index 732a357a99..20a060454b 100644 > > > > > --- a/arch/riscv/Kconfig > > > > > +++ b/arch/riscv/Kconfig > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > endchoice > > > > > > > > > > +choice > > > > > + prompt "Run Mode" > > > > > + default RISCV_MMODE > > > > > + > > > > > +config RISCV_MMODE > > > > > + bool "Machine" > > > > > + select TIMER > > > > > + help > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > + > > > > > +config RISCV_SMODE > > > > > + bool "Supervisor" > > > > > + help > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > + > > > > > +endchoice > > > > > + > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > patch in the series. > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > config RISCV_ISA_C > > > > > bool "Emit compressed instructions" > > > > > default y > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > config RISCV_ISA_A > > > > > def_bool y > > > > > > > > > > -config RISCV_SMODE > > > > > - bool "Run in S-Mode" > > > > > - help > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > - > > > > > config 32BIT > > > > > bool > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > index b58db89752..98aa6d40e7 100644 > > > > > --- a/arch/riscv/lib/Makefile > > > > > +++ b/arch/riscv/lib/Makefile > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > obj-y += interrupts.o > > > > > obj-y += reset.o > > > > > obj-y += setjmp.o > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > # For building EFI apps > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > new file mode 100644 > > > > > index 0000000000..077e568ca6 > > > > > --- /dev/null > > > > > +++ b/arch/riscv/lib/time.c > > > > > @@ -0,0 +1,60 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > +/* > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > + */ > > > > > + > > > > > +#include <common.h> > > > > > +#include <fdtdec.h> > > > > > + > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > + > > > > > +static unsigned int tbclk; > > > > > + > > > > > +static void setup_tbclk(void) > > > > > +{ > > > > > + int cpus; > > > > > + > > > > > + if (!gd->fdt_blob || tbclk) > > > > > + return; > > > > > + > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > + if (cpus < 0) { > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > + return; > > > > > + } > > > > > + > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > + "timebase-frequency", 1000000); > > > > > +} > > > > > + > > > > > +ulong notrace get_tbclk(void) > > > > > +{ > > > > > + setup_tbclk(); > > > > > + > > > > > + return tbclk; > > > > > +} > > > > > + > > > > > +#ifdef CONFIG_64BIT > > > > > +uint64_t notrace get_ticks(void) > > > > > +{ > > > > > + unsigned long n; > > > > > + > > > > > + __asm__ __volatile__ ( > > > > > + "rdtime %0" > > > > > + : "=r" (n)); > > > > > + return n; > > > > > +} > > > > > +#else > > > > > +uint64_t notrace get_ticks(void) > > > > > +{ > > > > > + uint32_t lo, hi, tmp; > > > > > + __asm__ __volatile__ ( > > > > > + "1:\n" > > > > > + "rdtimeh %0\n" > > > > > + "rdtime %1\n" > > > > > + "rdtimeh %2\n" > > > > > + "bne %0, %2, 1b" > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > +} > > > > > +#endif > > > > > -- > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > implement a DM timer driver for S-mode? > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > implement DM driver. > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > paper work. > > There is no DT node for "rdtime" so how should we probe the DM driver. > I think you can do some modification to create the S-mode timer driver based on the M-mode driver patch I provided. In the riscv_timer_probe(), do not require syscon driver to be loaded, ie: not calling syscon_get_by_driver_data(). In the riscv_timer_get_count(), implement that with rdtime for S-mode. > > > > > This is similar to how U-Boot ARM64 uses CNTPCT_EL0 register. > > > > > > We cannot use DM driver here. > > > > > > > The issue is that eventually we will migrate all U-Boot device drivers > > to DM, which means get_tbclk() and get_ticks() you implemented will be > > broken someday in the future. > > Yes, I agree we should have DM driver whenever possible but sometime > there is no DT bindings for things like timer because it's part of CPU > instruction or CSRs > Regards, Bin
On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > drivers will have to be provided. > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > --- > > > > > > arch/Kconfig | 1 - > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > --- a/arch/Kconfig > > > > > > +++ b/arch/Kconfig > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > imply BLK > > > > > > imply CLK > > > > > > imply MTD > > > > > > - imply TIMER > > > > > > > > > > No, please don't do this. > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > config SANDBOX > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > index 732a357a99..20a060454b 100644 > > > > > > --- a/arch/riscv/Kconfig > > > > > > +++ b/arch/riscv/Kconfig > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > endchoice > > > > > > > > > > > > +choice > > > > > > + prompt "Run Mode" > > > > > > + default RISCV_MMODE > > > > > > + > > > > > > +config RISCV_MMODE > > > > > > + bool "Machine" > > > > > > + select TIMER > > > > > > + help > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > + > > > > > > +config RISCV_SMODE > > > > > > + bool "Supervisor" > > > > > > + help > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > + > > > > > > +endchoice > > > > > > + > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > patch in the series. > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > bool "Emit compressed instructions" > > > > > > default y > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > config RISCV_ISA_A > > > > > > def_bool y > > > > > > > > > > > > -config RISCV_SMODE > > > > > > - bool "Run in S-Mode" > > > > > > - help > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > - > > > > > > config 32BIT > > > > > > bool > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > obj-y += interrupts.o > > > > > > obj-y += reset.o > > > > > > obj-y += setjmp.o > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > # For building EFI apps > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > new file mode 100644 > > > > > > index 0000000000..077e568ca6 > > > > > > --- /dev/null > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > @@ -0,0 +1,60 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > +/* > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > + */ > > > > > > + > > > > > > +#include <common.h> > > > > > > +#include <fdtdec.h> > > > > > > + > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > + > > > > > > +static unsigned int tbclk; > > > > > > + > > > > > > +static void setup_tbclk(void) > > > > > > +{ > > > > > > + int cpus; > > > > > > + > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > + return; > > > > > > + > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > + if (cpus < 0) { > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > + "timebase-frequency", 1000000); > > > > > > +} > > > > > > + > > > > > > +ulong notrace get_tbclk(void) > > > > > > +{ > > > > > > + setup_tbclk(); > > > > > > + > > > > > > + return tbclk; > > > > > > +} > > > > > > + > > > > > > +#ifdef CONFIG_64BIT > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + unsigned long n; > > > > > > + > > > > > > + __asm__ __volatile__ ( > > > > > > + "rdtime %0" > > > > > > + : "=r" (n)); > > > > > > + return n; > > > > > > +} > > > > > > +#else > > > > > > +uint64_t notrace get_ticks(void) > > > > > > +{ > > > > > > + uint32_t lo, hi, tmp; > > > > > > + __asm__ __volatile__ ( > > > > > > + "1:\n" > > > > > > + "rdtimeh %0\n" > > > > > > + "rdtime %1\n" > > > > > > + "rdtimeh %2\n" > > > > > > + "bne %0, %2, 1b" > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > +} > > > > > > +#endif > > > > > > -- > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > implement DM driver. > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > paper work. > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > I think you can do some modification to create the S-mode timer driver > based on the M-mode driver patch I provided. I think you missed my point. CLINT is SiFive specific. You have to rename your driver as clint_timer. The riscv_timer name is in-appropriate. We cannot use CLINT udevice for "rdtime" because: 1. SOC can implement "rdtime" in HW 2. SOC can implement some MMIO device (like CLINT) and emulate "rdtime" in runtime firmware. > > In the riscv_timer_probe(), do not require syscon driver to be loaded, > ie: not calling syscon_get_by_driver_data(). In the > riscv_timer_get_count(), implement that with rdtime for S-mode. There is no udevice for "rdtime" or time CSRs. CLINT is a SiFive specific device. Please rename your driver. Regards, Anup
Hi Anup, On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > --- > > > > > > > arch/Kconfig | 1 - > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > --- a/arch/Kconfig > > > > > > > +++ b/arch/Kconfig > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > imply BLK > > > > > > > imply CLK > > > > > > > imply MTD > > > > > > > - imply TIMER > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > config SANDBOX > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > +choice > > > > > > > + prompt "Run Mode" > > > > > > > + default RISCV_MMODE > > > > > > > + > > > > > > > +config RISCV_MMODE > > > > > > > + bool "Machine" > > > > > > > + select TIMER > > > > > > > + help > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > + > > > > > > > +config RISCV_SMODE > > > > > > > + bool "Supervisor" > > > > > > > + help > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > + > > > > > > > +endchoice > > > > > > > + > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > patch in the series. > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > bool "Emit compressed instructions" > > > > > > > default y > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > config RISCV_ISA_A > > > > > > > def_bool y > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > - bool "Run in S-Mode" > > > > > > > - help > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > - > > > > > > > config 32BIT > > > > > > > bool > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > obj-y += interrupts.o > > > > > > > obj-y += reset.o > > > > > > > obj-y += setjmp.o > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > # For building EFI apps > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > new file mode 100644 > > > > > > > index 0000000000..077e568ca6 > > > > > > > --- /dev/null > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > +/* > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > + */ > > > > > > > + > > > > > > > +#include <common.h> > > > > > > > +#include <fdtdec.h> > > > > > > > + > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > + > > > > > > > +static unsigned int tbclk; > > > > > > > + > > > > > > > +static void setup_tbclk(void) > > > > > > > +{ > > > > > > > + int cpus; > > > > > > > + > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > + return; > > > > > > > + > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > + if (cpus < 0) { > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > + return; > > > > > > > + } > > > > > > > + > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > + "timebase-frequency", 1000000); > > > > > > > +} > > > > > > > + > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > +{ > > > > > > > + setup_tbclk(); > > > > > > > + > > > > > > > + return tbclk; > > > > > > > +} > > > > > > > + > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > +{ > > > > > > > + unsigned long n; > > > > > > > + > > > > > > > + __asm__ __volatile__ ( > > > > > > > + "rdtime %0" > > > > > > > + : "=r" (n)); > > > > > > > + return n; > > > > > > > +} > > > > > > > +#else > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > +{ > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > + __asm__ __volatile__ ( > > > > > > > + "1:\n" > > > > > > > + "rdtimeh %0\n" > > > > > > > + "rdtime %1\n" > > > > > > > + "rdtimeh %2\n" > > > > > > > + "bne %0, %2, 1b" > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > +} > > > > > > > +#endif > > > > > > > -- > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > implement DM driver. > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > paper work. > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > I think you can do some modification to create the S-mode timer driver > > based on the M-mode driver patch I provided. > > I think you missed my point. > > CLINT is SiFive specific. You have to rename your driver as > clint_timer. The riscv_timer name is in-appropriate. > Yes, I am reconsidering this. Lukas had similar comments, and Rick confirmed that Andes's chip implemented different mtimer/IPI block from SiFive's. > We cannot use CLINT udevice for "rdtime" because: > 1. SOC can implement "rdtime" in HW > 2. SOC can implement some MMIO device (like CLINT) > and emulate "rdtime" in runtime firmware. > It looks you misunderstood things. I was not suggesting you use CLINT device for S-mode timer driver at all. I am not sure why you misunderstood it. Please read my comments carefully. > > > > In the riscv_timer_probe(), do not require syscon driver to be loaded, > > ie: not calling syscon_get_by_driver_data(). In the > > riscv_timer_get_count(), implement that with rdtime for S-mode. > > There is no udevice for "rdtime" or time CSRs. > > CLINT is a SiFive specific device. Please rename your driver. > Regards, Bin
On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > --- > > > > > > > > arch/Kconfig | 1 - > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > --- a/arch/Kconfig > > > > > > > > +++ b/arch/Kconfig > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > imply BLK > > > > > > > > imply CLK > > > > > > > > imply MTD > > > > > > > > - imply TIMER > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > +choice > > > > > > > > + prompt "Run Mode" > > > > > > > > + default RISCV_MMODE > > > > > > > > + > > > > > > > > +config RISCV_MMODE > > > > > > > > + bool "Machine" > > > > > > > > + select TIMER > > > > > > > > + help > > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > > + > > > > > > > > +config RISCV_SMODE > > > > > > > > + bool "Supervisor" > > > > > > > > + help > > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > > + > > > > > > > > +endchoice > > > > > > > > + > > > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > > patch in the series. > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > bool "Emit compressed instructions" > > > > > > > > default y > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > config RISCV_ISA_A > > > > > > > > def_bool y > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > - bool "Run in S-Mode" > > > > > > > > - help > > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > > - > > > > > > > > config 32BIT > > > > > > > > bool > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > obj-y += interrupts.o > > > > > > > > obj-y += reset.o > > > > > > > > obj-y += setjmp.o > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > new file mode 100644 > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > --- /dev/null > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > +/* > > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > > + */ > > > > > > > > + > > > > > > > > +#include <common.h> > > > > > > > > +#include <fdtdec.h> > > > > > > > > + > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > + > > > > > > > > +static unsigned int tbclk; > > > > > > > > + > > > > > > > > +static void setup_tbclk(void) > > > > > > > > +{ > > > > > > > > + int cpus; > > > > > > > > + > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > + return; > > > > > > > > + > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > + if (cpus < 0) { > > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > > + return; > > > > > > > > + } > > > > > > > > + > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > + "timebase-frequency", 1000000); > > > > > > > > +} > > > > > > > > + > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > +{ > > > > > > > > + setup_tbclk(); > > > > > > > > + > > > > > > > > + return tbclk; > > > > > > > > +} > > > > > > > > + > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > +{ > > > > > > > > + unsigned long n; > > > > > > > > + > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > + "rdtime %0" > > > > > > > > + : "=r" (n)); > > > > > > > > + return n; > > > > > > > > +} > > > > > > > > +#else > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > +{ > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > + "1:\n" > > > > > > > > + "rdtimeh %0\n" > > > > > > > > + "rdtime %1\n" > > > > > > > > + "rdtimeh %2\n" > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > +} > > > > > > > > +#endif > > > > > > > > -- > > > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > > implement DM driver. > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > > paper work. > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > > > > I think you can do some modification to create the S-mode timer driver > > > based on the M-mode driver patch I provided. > > > > I think you missed my point. > > > > CLINT is SiFive specific. You have to rename your driver as > > clint_timer. The riscv_timer name is in-appropriate. > > > > Yes, I am reconsidering this. Lukas had similar comments, and Rick > confirmed that Andes's chip implemented different mtimer/IPI block > from SiFive's. > > > We cannot use CLINT udevice for "rdtime" because: > > 1. SOC can implement "rdtime" in HW > > 2. SOC can implement some MMIO device (like CLINT) > > and emulate "rdtime" in runtime firmware. > > > > It looks you misunderstood things. I was not suggesting you use CLINT > device for S-mode timer driver at all. I am not sure why you > misunderstood it. Please read my comments carefully. > Can you explain how will the timer probed called if there is no matching device in DT? Regards, Anup
Hi Anup, On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > --- > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > imply BLK > > > > > > > > > imply CLK > > > > > > > > > imply MTD > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > + prompt "Run Mode" > > > > > > > > > + default RISCV_MMODE > > > > > > > > > + > > > > > > > > > +config RISCV_MMODE > > > > > > > > > + bool "Machine" > > > > > > > > > + select TIMER > > > > > > > > > + help > > > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > > > + > > > > > > > > > +config RISCV_SMODE > > > > > > > > > + bool "Supervisor" > > > > > > > > > + help > > > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > > > + > > > > > > > > > +endchoice > > > > > > > > > + > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > > > patch in the series. > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > default y > > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > > config RISCV_ISA_A > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > - help > > > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > > > - > > > > > > > > > config 32BIT > > > > > > > > > bool > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > > obj-y += interrupts.o > > > > > > > > > obj-y += reset.o > > > > > > > > > obj-y += setjmp.o > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > new file mode 100644 > > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > > --- /dev/null > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > +/* > > > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > > > + */ > > > > > > > > > + > > > > > > > > > +#include <common.h> > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > + > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > + > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > + > > > > > > > > > +static void setup_tbclk(void) > > > > > > > > > +{ > > > > > > > > > + int cpus; > > > > > > > > > + > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > + return; > > > > > > > > > + > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > + if (cpus < 0) { > > > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > > > + return; > > > > > > > > > + } > > > > > > > > > + > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > + "timebase-frequency", 1000000); > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > > +{ > > > > > > > > > + setup_tbclk(); > > > > > > > > > + > > > > > > > > > + return tbclk; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > +{ > > > > > > > > > + unsigned long n; > > > > > > > > > + > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > + "rdtime %0" > > > > > > > > > + : "=r" (n)); > > > > > > > > > + return n; > > > > > > > > > +} > > > > > > > > > +#else > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > +{ > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > + "1:\n" > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > + "rdtime %1\n" > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > > +} > > > > > > > > > +#endif > > > > > > > > > -- > > > > > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > > > implement DM driver. > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > > > paper work. > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > > > > > > > I think you can do some modification to create the S-mode timer driver > > > > based on the M-mode driver patch I provided. > > > > > > I think you missed my point. > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and Rick > > confirmed that Andes's chip implemented different mtimer/IPI block > > from SiFive's. > > > > > We cannot use CLINT udevice for "rdtime" because: > > > 1. SOC can implement "rdtime" in HW > > > 2. SOC can implement some MMIO device (like CLINT) > > > and emulate "rdtime" in runtime firmware. > > > > > > > It looks you misunderstood things. I was not suggesting you use CLINT > > device for S-mode timer driver at all. I am not sure why you > > misunderstood it. Please read my comments carefully. > > > > Can you explain how will the timer probed called if there is no > matching device in DT? > The timer driver is bound by the CPU driver. See http://patchwork.ozlabs.org/patch/996902/. Regards, Bin
On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > --- > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > imply BLK > > > > > > > > > > imply CLK > > > > > > > > > > imply MTD > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > + > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > + bool "Machine" > > > > > > > > > > + select TIMER > > > > > > > > > > + help > > > > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > > > > + > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > + help > > > > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > > > > + > > > > > > > > > > +endchoice > > > > > > > > > > + > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > > > > patch in the series. > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > default y > > > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > > > config RISCV_ISA_A > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > - help > > > > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > > > > - > > > > > > > > > > config 32BIT > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > > > obj-y += interrupts.o > > > > > > > > > > obj-y += reset.o > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > new file mode 100644 > > > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > > > --- /dev/null > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > +/* > > > > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > > > > + */ > > > > > > > > > > + > > > > > > > > > > +#include <common.h> > > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > > + > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > + > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > + > > > > > > > > > > +static void setup_tbclk(void) > > > > > > > > > > +{ > > > > > > > > > > + int cpus; > > > > > > > > > > + > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > + return; > > > > > > > > > > + > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > > > > + return; > > > > > > > > > > + } > > > > > > > > > > + > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > + "timebase-frequency", 1000000); > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > > > +{ > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > + > > > > > > > > > > + return tbclk; > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > +{ > > > > > > > > > > + unsigned long n; > > > > > > > > > > + > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > + "rdtime %0" > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > + return n; > > > > > > > > > > +} > > > > > > > > > > +#else > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > +{ > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > + "1:\n" > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > > > +} > > > > > > > > > > +#endif > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > > > > implement DM driver. > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > > > > paper work. > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode timer driver > > > > > based on the M-mode driver patch I provided. > > > > > > > > I think you missed my point. > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and Rick > > > confirmed that Andes's chip implemented different mtimer/IPI block > > > from SiFive's. > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > 1. SOC can implement "rdtime" in HW > > > > 2. SOC can implement some MMIO device (like CLINT) > > > > and emulate "rdtime" in runtime firmware. > > > > > > > > > > It looks you misunderstood things. I was not suggesting you use CLINT > > > device for S-mode timer driver at all. I am not sure why you > > > misunderstood it. Please read my comments carefully. > > > > > > > Can you explain how will the timer probed called if there is no > > matching device in DT? > > > > The timer driver is bound by the CPU driver. See > http://patchwork.ozlabs.org/patch/996902/. Makes sense now. There is no DT based probing. Instead, you are explicitly bind the timer driver. M-mode U-Boot will never use the generic riscv_timer based on "rdtime" instruction. We will mostly have SoC specific timer drivers for M-mode U-Boot. I think your CPU driver should only do the explicit device binding if CONFIG_RISCV_SMODE=y. There is inter-dependency here. My suggestion is, I can include you CPU driver patch and add DM driver for "rdtime" based upon that. Only If you are fine ?? Regards, Anup
Hi Anup, On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Anup, > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > --- > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > imply BLK > > > > > > > > > > > imply CLK > > > > > > > > > > > imply MTD > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > + > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > + select TIMER > > > > > > > > > > > + help > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > > > > > + > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > + help > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > > > > > + > > > > > > > > > > > +endchoice > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > > > > > patch in the series. > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > default y > > > > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > > > > config RISCV_ISA_A > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > - help > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > > > > > - > > > > > > > > > > > config 32BIT > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > > > > obj-y += interrupts.o > > > > > > > > > > > obj-y += reset.o > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > new file mode 100644 > > > > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > > > > --- /dev/null > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > +/* > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > > > > > + */ > > > > > > > > > > > + > > > > > > > > > > > +#include <common.h> > > > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > > > + > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > + > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > + > > > > > > > > > > > +static void setup_tbclk(void) > > > > > > > > > > > +{ > > > > > > > > > > > + int cpus; > > > > > > > > > > > + > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > + return; > > > > > > > > > > > + > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > > > > > + return; > > > > > > > > > > > + } > > > > > > > > > > > + > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > + "timebase-frequency", 1000000); > > > > > > > > > > > +} > > > > > > > > > > > + > > > > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > > > > +{ > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > + > > > > > > > > > > > + return tbclk; > > > > > > > > > > > +} > > > > > > > > > > > + > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > +{ > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > + > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > + return n; > > > > > > > > > > > +} > > > > > > > > > > > +#else > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > +{ > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > + "1:\n" > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > > > > +} > > > > > > > > > > > +#endif > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > > > > > implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > > > > > paper work. > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode timer driver > > > > > > based on the M-mode driver patch I provided. > > > > > > > > > > I think you missed my point. > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and Rick > > > > confirmed that Andes's chip implemented different mtimer/IPI block > > > > from SiFive's. > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > 1. SOC can implement "rdtime" in HW > > > > > 2. SOC can implement some MMIO device (like CLINT) > > > > > and emulate "rdtime" in runtime firmware. > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you use CLINT > > > > device for S-mode timer driver at all. I am not sure why you > > > > misunderstood it. Please read my comments carefully. > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > matching device in DT? > > > > > > > The timer driver is bound by the CPU driver. See > > http://patchwork.ozlabs.org/patch/996902/. > > Makes sense now. There is no DT based probing. Instead, you > are explicitly bind the timer driver. > > M-mode U-Boot will never use the generic riscv_timer based > on "rdtime" instruction. We will mostly have SoC specific timer > drivers for M-mode U-Boot. > Unfortunately yes, M-mode timer driver cannot be generic and that's what bothered me. It should have been architected clearly in the very beginning since the timer is supposed to deliver interrupts directly into [M|S]IP on the hart and allowing platform implementation to me is creating fragmented solutions. If there are other timers available on SoC device, I will definitely use them instead. > I think your CPU driver should only do the explicit device > binding if CONFIG_RISCV_SMODE=y. > Yep I haven't figured out a clean way to do the driver binding if there are different mtimer implementations, as the mtimer driver name has to be made known to the CPU driver. But I think the S-mode driver can be generic. > There is inter-dependency here. > > My suggestion is, I can include you CPU driver patch and > add DM driver for "rdtime" based upon that. Only If you are > fine ?? > I am fine with that. However there are some review comments that need be addressed in my v1 CPU/timer driver series so this should be fixed at first. Maybe we can just delay the S-mode timer driver support until later, after my series goes in. Regards, Bin
On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Anup, > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and rdtimeh instructions > > > > > > > > > > > > for reading timer ticks (just like Linux). The frequency of timer > > > > > > > > > > > > ticks is passed by prior booting stages in "timebase-frequency" DT > > > > > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer implementation for U-Boot > > > > > > > > > > > > running in S-mode. For U-Boot running in M-mode, specific timer > > > > > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > --- > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 deletions(-) > > > > > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > imply BLK > > > > > > > > > > > > imply CLK > > > > > > > > > > > > imply MTD > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > + > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > + help > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V M-Mode. > > > > > > > > > > > > + > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > + help > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V S-Mode. > > > > > > > > > > > > + > > > > > > > > > > > > +endchoice > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge that in the 1st > > > > > > > > > > > patch in the series. > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > default y > > > > > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > > > > > config RISCV_ISA_A > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > - help > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V S-Mode > > > > > > > > > > > > - > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile > > > > > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > > > > > obj-y += interrupts.o > > > > > > > > > > > > obj-y += reset.o > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > > > > > diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > new file mode 100644 > > > > > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > +/* > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> > > > > > > > > > > > > + */ > > > > > > > > > > > > + > > > > > > > > > > > > +#include <common.h> > > > > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > > > > + > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > + > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > + > > > > > > > > > > > > +static void setup_tbclk(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > + > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > + return; > > > > > > > > > > > > + > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", __func__); > > > > > > > > > > > > + return; > > > > > > > > > > > > + } > > > > > > > > > > > > + > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > + "timebase-frequency", 1000000); > > > > > > > > > > > > +} > > > > > > > > > > > > + > > > > > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > + > > > > > > > > > > > > + return tbclk; > > > > > > > > > > > > +} > > > > > > > > > > > > + > > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > + > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > + return n; > > > > > > > > > > > > +} > > > > > > > > > > > > +#else > > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > > > > > +} > > > > > > > > > > > > +#endif > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via DM. Could you please > > > > > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for which we can > > > > > > > > > > implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a driver to do the > > > > > > > > > paper work. > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM driver. > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode timer driver > > > > > > > based on the M-mode driver patch I provided. > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and Rick > > > > > confirmed that Andes's chip implemented different mtimer/IPI block > > > > > from SiFive's. > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > 1. SOC can implement "rdtime" in HW > > > > > > 2. SOC can implement some MMIO device (like CLINT) > > > > > > and emulate "rdtime" in runtime firmware. > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you use CLINT > > > > > device for S-mode timer driver at all. I am not sure why you > > > > > misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > matching device in DT? > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > http://patchwork.ozlabs.org/patch/996902/. > > > > Makes sense now. There is no DT based probing. Instead, you > > are explicitly bind the timer driver. > > > > M-mode U-Boot will never use the generic riscv_timer based > > on "rdtime" instruction. We will mostly have SoC specific timer > > drivers for M-mode U-Boot. > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > what bothered me. It should have been architected clearly in the very > beginning since the timer is supposed to deliver interrupts directly > into [M|S]IP on the hart and allowing platform implementation to me is > creating fragmented solutions. If there are other timers available on > SoC device, I will definitely use them instead. Even, I think timer CSRs should be clearly defined and accessible from both M and S modes. > > > I think your CPU driver should only do the explicit device > > binding if CONFIG_RISCV_SMODE=y. > > > > Yep I haven't figured out a clean way to do the driver binding if > there are different mtimer implementations, as the mtimer driver name > has to be made known to the CPU driver. But I think the S-mode driver > can be generic. > > > There is inter-dependency here. > > > > My suggestion is, I can include you CPU driver patch and > > add DM driver for "rdtime" based upon that. Only If you are > > fine ?? > > > > I am fine with that. However there are some review comments that need > be addressed in my v1 CPU/timer driver series so this should be fixed > at first. Maybe we can just delay the S-mode timer driver support > until later, after my series goes in. > I think first three patches of this series can go in without S-mode timer patch. Regards, Anup
Hi Bin, On Mon, 2018-12-03 at 16:45 +0800, Bin Meng wrote: > Hi Anup, > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> > wrote: > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> > > > wrote: > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > > > wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel < > > > > > anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng < > > > > > > bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel < > > > > > > > anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng < > > > > > > > > bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel < > > > > > > > > > anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng < > > > > > > > > > > bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel < > > > > > > > > > > > anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > rdtimeh instructions > > > > > > > > > > > > for reading timer ticks (just like Linux). The > > > > > > > > > > > > frequency of timer > > > > > > > > > > > > ticks is passed by prior booting stages in > > > > > > > > > > > > "timebase-frequency" DT > > > > > > > > > > > > property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > implementation for U-Boot > > > > > > > > > > > > running in S-mode. For U-Boot running in M- > > > > > > > > > > > > mode, specific timer > > > > > > > > > > > > drivers will have to be provided. > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > --- > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > > > > > > > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > deletions(-) > > > > > > > > > > > > create mode 100644 arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > > > > > > > > > > index 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > imply BLK > > > > > > > > > > > > imply CLK > > > > > > > > > > > > imply MTD > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode > > > > > > > > > > U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > b/arch/riscv/Kconfig > > > > > > > > > > > > index 732a357a99..20a060454b 100644 > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > + > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > + help > > > > > > > > > > > > + Choose this option to build U-Boot > > > > > > > > > > > > for RISC-V M-Mode. > > > > > > > > > > > > + > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > + help > > > > > > > > > > > > + Choose this option to build U-Boot > > > > > > > > > > > > for RISC-V S-Mode. > > > > > > > > > > > > + > > > > > > > > > > > > +endchoice > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or > > > > > > > > > > > merge that in the 1st > > > > > > > > > > > patch in the series. > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > default y > > > > > > > > > > > > @@ -55,11 +72,6 @@ config RISCV_ISA_C > > > > > > > > > > > > config RISCV_ISA_A > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > - help > > > > > > > > > > > > - Enable this option to build U-Boot > > > > > > > > > > > > for RISC-V S-Mode > > > > > > > > > > > > - > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > b/arch/riscv/lib/Makefile > > > > > > > > > > > > index b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o > > > > > > > > > > > > obj-y += interrupts.o > > > > > > > > > > > > obj-y += reset.o > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) > > > > > > > > > > > > diff --git a/arch/riscv/lib/time.c > > > > > > > > > > > > b/arch/riscv/lib/time.c > > > > > > > > > > > > new file mode 100644 > > > > > > > > > > > > index 0000000000..077e568ca6 > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > +/* > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel < > > > > > > > > > > > > anup@brainfault.org> > > > > > > > > > > > > + */ > > > > > > > > > > > > + > > > > > > > > > > > > +#include <common.h> > > > > > > > > > > > > +#include <fdtdec.h> > > > > > > > > > > > > + > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > + > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > + > > > > > > > > > > > > +static void setup_tbclk(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > + > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > + return; > > > > > > > > > > > > + > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, > > > > > > > > > > > > "/cpus"); > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > + debug("%s: Missing /cpus > > > > > > > > > > > > node\n", __func__); > > > > > > > > > > > > + return; > > > > > > > > > > > > + } > > > > > > > > > > > > + > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, > > > > > > > > > > > > cpus, > > > > > > > > > > > > + "timebase- > > > > > > > > > > > > frequency", 1000000); > > > > > > > > > > > > +} > > > > > > > > > > > > + > > > > > > > > > > > > +ulong notrace get_tbclk(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > + > > > > > > > > > > > > + return tbclk; > > > > > > > > > > > > +} > > > > > > > > > > > > + > > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > + > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > + return n; > > > > > > > > > > > > +} > > > > > > > > > > > > +#else > > > > > > > > > > > > +uint64_t notrace get_ticks(void) > > > > > > > > > > > > +{ > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" > > > > > > > > > > > > (tmp)); > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; > > > > > > > > > > > > +} > > > > > > > > > > > > +#endif > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > DM. Could you please > > > > > > > > > > > implement a DM timer driver for S-mode? > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > > > > > > > > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > which we can > > > > > > > > > > implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > driver to do the > > > > > > > > > paper work. > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe > > > > > > > > the DM driver. > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > timer driver > > > > > > > based on the M-mode driver patch I provided. > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > Rick > > > > > confirmed that Andes's chip implemented different mtimer/IPI > > > > > block > > > > > from SiFive's. > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > 1. SOC can implement "rdtime" in HW > > > > > > 2. SOC can implement some MMIO device (like CLINT) > > > > > > and emulate "rdtime" in runtime firmware. > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > use CLINT > > > > > device for S-mode timer driver at all. I am not sure why you > > > > > misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > matching device in DT? > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > http://patchwork.ozlabs.org/patch/996902/. > > > > Makes sense now. There is no DT based probing. Instead, you > > are explicitly bind the timer driver. > > > > M-mode U-Boot will never use the generic riscv_timer based > > on "rdtime" instruction. We will mostly have SoC specific timer > > drivers for M-mode U-Boot. > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > what bothered me. It should have been architected clearly in the very > beginning since the timer is supposed to deliver interrupts directly > into [M|S]IP on the hart and allowing platform implementation to me > is > creating fragmented solutions. If there are other timers available on > SoC device, I will definitely use them instead. > > > I think your CPU driver should only do the explicit device > > binding if CONFIG_RISCV_SMODE=y. > > > > Yep I haven't figured out a clean way to do the driver binding if > there are different mtimer implementations, as the mtimer driver name > has to be made known to the CPU driver. But I think the S-mode driver > can be generic. > How about something like the driver misc/tegra_car.c? So one misc driver for riscv,clint0, which explicitly binds the M-mode timer and the syscon driver. Thanks, Lukas > > There is inter-dependency here. > > > > My suggestion is, I can include you CPU driver patch and > > add DM driver for "rdtime" based upon that. Only If you are > > fine ?? > > > > I am fine with that. However there are some review comments that need > be addressed in my v1 CPU/timer driver series so this should be fixed > at first. Maybe we can just delay the S-mode timer driver support > until later, after my series goes in. > > Regards, > Bin
> > From: Anup Patel [mailto:anup@brainfault.org] > > Sent: Monday, December 03, 2018 6:30 PM > > To: Bin Meng > > Cc: Rick Jian-Zhi Chen(陳建志); Lukas Auer; Alexander Graf; Palmer Dabbelt; > > Atish Patra; Christoph Hellwig; U-Boot Mailing List > > Subject: Re: [PATCH v7 4/4] RISC-V: Add S-mode timer implementation > > > > On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Anup, > > > > > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> > > wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> > > wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> > > wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > > > rdtimeh instructions for reading timer ticks > > > > > > > > > > > > > > (just like Linux). The frequency of timer ticks > > > > > > > > > > > > > > is passed by prior booting stages in "timebase-frequency" > > DT property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > > > implementation for U-Boot running in S-mode. For > > > > > > > > > > > > > > U-Boot running in M-mode, specific timer drivers will have > > to be provided. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > > > deletions(-) create mode 100644 > > > > > > > > > > > > > > arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > > > > > > > > > > > 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > > > imply BLK > > > > > > > > > > > > > > imply CLK > > > > > > > > > > > > > > imply MTD > > > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > > > b/arch/riscv/Kconfig index > > > > > > > > > > > > > > 732a357a99..20a060454b 100644 > > > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > M-Mode. > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > S-Mode. > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +endchoice > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge > > > > > > > > > > > > > that in the 1st patch in the series. > > > > > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > > > default y @@ -55,11 +72,6 @@ config > > > > > > > > > > > > > > RISCV_ISA_C config RISCV_ISA_A > > > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V > > S-Mode > > > > > > > > > > > > > > - > > > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > b/arch/riscv/lib/Makefile index > > > > > > > > > > > > > > b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += > > > > > > > > > > > > > > interrupts.o obj-y += reset.o > > > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git > > > > > > > > > > > > > > a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > > > new file mode 100644 index > > > > > > > > > > > > > > 0000000000..077e568ca6 > > > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > > > +/* > > > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel > > > > > > > > > > > > > > +<anup@brainfault.org> */ > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +#include <common.h> #include <fdtdec.h> > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +static void setup_tbclk(void) { > > > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", > > __func__); > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +"timebase-frequency", 1000000); } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +ulong notrace get_tbclk(void) { > > > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + return tbclk; } > > > > > > > > > > > > > > + > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT uint64_t notrace > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > > > + > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > > > + return n; } #else uint64_t notrace > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; } > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > > > DM. Could you please implement a DM timer driver for > > S-mode? > > > > > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > > > which we can implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > > > driver to do the paper work. > > > > > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM > > driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > > > timer driver based on the M-mode driver patch I provided. > > > > > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > > > Rick confirmed that Andes's chip implemented different > > > > > > > mtimer/IPI block from SiFive's. > > > > > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > > > 1. SOC can implement "rdtime" in HW 2. SOC can implement > > > > > > > > some MMIO device (like CLINT) and emulate "rdtime" in > > > > > > > > runtime firmware. > > > > > > > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > > > use CLINT device for S-mode timer driver at all. I am not sure > > > > > > > why you misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > > > matching device in DT? > > > > > > > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > > > http://patchwork.ozlabs.org/patch/996902/. > > > > > > > > Makes sense now. There is no DT based probing. Instead, you are > > > > explicitly bind the timer driver. > > > > > > > > M-mode U-Boot will never use the generic riscv_timer based on > > > > "rdtime" instruction. We will mostly have SoC specific timer drivers > > > > for M-mode U-Boot. > > > > > > > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > > > what bothered me. It should have been architected clearly in the very > > > beginning since the timer is supposed to deliver interrupts directly > > > into [M|S]IP on the hart and allowing platform implementation to me is > > > creating fragmented solutions. If there are other timers available on > > > SoC device, I will definitely use them instead. > > > > Even, I think timer CSRs should be clearly defined and accessible from both M and > > S modes. > > > > > > > > > I think your CPU driver should only do the explicit device binding > > > > if CONFIG_RISCV_SMODE=y. > > > > > > > > > > Yep I haven't figured out a clean way to do the driver binding if > > > there are different mtimer implementations, as the mtimer driver name > > > has to be made known to the CPU driver. But I think the S-mode driver > > > can be generic. > > > > > > > There is inter-dependency here. > > > > > > > > My suggestion is, I can include you CPU driver patch and add DM > > > > driver for "rdtime" based upon that. Only If you are fine ?? > > > > > > > > > > I am fine with that. However there are some review comments that need > > > be addressed in my v1 CPU/timer driver series so this should be fixed > > > at first. Maybe we can just delay the S-mode timer driver support > > > until later, after my series goes in. > > > > > > > I think first three patches of this series can go in without S-mode timer patch. > > > > Regards, > > Anup Hi Anup get_ticks( ) is indeed an old way which shall be replace by dm timer. atcpit100_timer is SoC dm timer which is used by ae350. Maybe Bin is trying to say that rdtime shall be called in riscv_timer_get_count( ) in riscv_timer.c And then trigger a SBI call to M-mode and get mtime when u-boot is running in S-mode, right ? But the merge window is closed. I am not sure if it is appropriate to send a PR at this time. Maybe I can push your first three patches of this series into u-boot-riscv.git And send a PR to master when next merge window open. Rick
Hi Rick, On Tue, Dec 4, 2018 at 3:12 PM Rick Chen <rickchen36@gmail.com> wrote: > > > > From: Anup Patel [mailto:anup@brainfault.org] > > > Sent: Monday, December 03, 2018 6:30 PM > > > To: Bin Meng > > > Cc: Rick Jian-Zhi Chen(陳建志); Lukas Auer; Alexander Graf; Palmer Dabbelt; > > > Atish Patra; Christoph Hellwig; U-Boot Mailing List > > > Subject: Re: [PATCH v7 4/4] RISC-V: Add S-mode timer implementation > > > > > > On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > Hi Anup, > > > > > > > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > > wrote: > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > > > > rdtimeh instructions for reading timer ticks > > > > > > > > > > > > > > > (just like Linux). The frequency of timer ticks > > > > > > > > > > > > > > > is passed by prior booting stages in "timebase-frequency" > > > DT property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > > > > implementation for U-Boot running in S-mode. For > > > > > > > > > > > > > > > U-Boot running in M-mode, specific timer drivers will have > > > to be provided. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > > > > deletions(-) create mode 100644 > > > > > > > > > > > > > > > arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > > > > > > > > > > > > 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > > > > imply BLK > > > > > > > > > > > > > > > imply CLK > > > > > > > > > > > > > > > imply MTD > > > > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > > > > b/arch/riscv/Kconfig index > > > > > > > > > > > > > > > 732a357a99..20a060454b 100644 > > > > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > M-Mode. > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > S-Mode. > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +endchoice > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge > > > > > > > > > > > > > > that in the 1st patch in the series. > > > > > > > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > > > > default y @@ -55,11 +72,6 @@ config > > > > > > > > > > > > > > > RISCV_ISA_C config RISCV_ISA_A > > > > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V > > > S-Mode > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > b/arch/riscv/lib/Makefile index > > > > > > > > > > > > > > > b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += > > > > > > > > > > > > > > > interrupts.o obj-y += reset.o > > > > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git > > > > > > > > > > > > > > > a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > new file mode 100644 index > > > > > > > > > > > > > > > 0000000000..077e568ca6 > > > > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > > > > +/* > > > > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel > > > > > > > > > > > > > > > +<anup@brainfault.org> */ > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +#include <common.h> #include <fdtdec.h> > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +static void setup_tbclk(void) { > > > > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", > > > __func__); > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +"timebase-frequency", 1000000); } > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +ulong notrace get_tbclk(void) { > > > > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > + return tbclk; } > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT uint64_t notrace > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > > > > + return n; } #else uint64_t notrace > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; } > > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > > > > DM. Could you please implement a DM timer driver for > > > S-mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > > > > which we can implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > > > > driver to do the paper work. > > > > > > > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM > > > driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > > > > timer driver based on the M-mode driver patch I provided. > > > > > > > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > > > > Rick confirmed that Andes's chip implemented different > > > > > > > > mtimer/IPI block from SiFive's. > > > > > > > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > > > > 1. SOC can implement "rdtime" in HW 2. SOC can implement > > > > > > > > > some MMIO device (like CLINT) and emulate "rdtime" in > > > > > > > > > runtime firmware. > > > > > > > > > > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > > > > use CLINT device for S-mode timer driver at all. I am not sure > > > > > > > > why you misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > > > > matching device in DT? > > > > > > > > > > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > > > > http://patchwork.ozlabs.org/patch/996902/. > > > > > > > > > > Makes sense now. There is no DT based probing. Instead, you are > > > > > explicitly bind the timer driver. > > > > > > > > > > M-mode U-Boot will never use the generic riscv_timer based on > > > > > "rdtime" instruction. We will mostly have SoC specific timer drivers > > > > > for M-mode U-Boot. > > > > > > > > > > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > > > > what bothered me. It should have been architected clearly in the very > > > > beginning since the timer is supposed to deliver interrupts directly > > > > into [M|S]IP on the hart and allowing platform implementation to me is > > > > creating fragmented solutions. If there are other timers available on > > > > SoC device, I will definitely use them instead. > > > > > > Even, I think timer CSRs should be clearly defined and accessible from both M and > > > S modes. > > > > > > > > > > > > I think your CPU driver should only do the explicit device binding > > > > > if CONFIG_RISCV_SMODE=y. > > > > > > > > > > > > > Yep I haven't figured out a clean way to do the driver binding if > > > > there are different mtimer implementations, as the mtimer driver name > > > > has to be made known to the CPU driver. But I think the S-mode driver > > > > can be generic. > > > > > > > > > There is inter-dependency here. > > > > > > > > > > My suggestion is, I can include you CPU driver patch and add DM > > > > > driver for "rdtime" based upon that. Only If you are fine ?? > > > > > > > > > > > > > I am fine with that. However there are some review comments that need > > > > be addressed in my v1 CPU/timer driver series so this should be fixed > > > > at first. Maybe we can just delay the S-mode timer driver support > > > > until later, after my series goes in. > > > > > > > > > > I think first three patches of this series can go in without S-mode timer patch. > > > > > > Regards, > > > Anup > > Hi Anup > > get_ticks( ) is indeed an old way which shall be replace by dm timer. > atcpit100_timer is SoC dm timer which is used by ae350. > Maybe Bin is trying to say that rdtime shall be called in > riscv_timer_get_count( ) in riscv_timer.c > And then trigger a SBI call to M-mode and get mtime when u-boot is > running in S-mode, right ? > Correct. > But the merge window is closed. > I am not sure if it is appropriate to send a PR at this time. > I think it is OK to send the PR as the first 3 patches of S-mode support are pretty much RISC-V specific. > Maybe I can push your first three patches of this series into u-boot-riscv.git > And send a PR to master when next merge window open. > For my patch series, I will send v2 soon to address the comments and I hope they can be included in v2019.01 release too, and Anup can send out the S-mode timer support on top of that. Regards, Bin
On Tue, Dec 4, 2018 at 1:44 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Rick, > > On Tue, Dec 4, 2018 at 3:12 PM Rick Chen <rickchen36@gmail.com> wrote: > > > > > > From: Anup Patel [mailto:anup@brainfault.org] > > > > Sent: Monday, December 03, 2018 6:30 PM > > > > To: Bin Meng > > > > Cc: Rick Jian-Zhi Chen(陳建志); Lukas Auer; Alexander Graf; Palmer Dabbelt; > > > > Atish Patra; Christoph Hellwig; U-Boot Mailing List > > > > Subject: Re: [PATCH v7 4/4] RISC-V: Add S-mode timer implementation > > > > > > > > On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > Hi Anup, > > > > > > > > > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > > > > > rdtimeh instructions for reading timer ticks > > > > > > > > > > > > > > > > (just like Linux). The frequency of timer ticks > > > > > > > > > > > > > > > > is passed by prior booting stages in "timebase-frequency" > > > > DT property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > > > > > implementation for U-Boot running in S-mode. For > > > > > > > > > > > > > > > > U-Boot running in M-mode, specific timer drivers will have > > > > to be provided. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > > > > > deletions(-) create mode 100644 > > > > > > > > > > > > > > > > arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > > > > > > > > > > > > > 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > > > > > imply BLK > > > > > > > > > > > > > > > > imply CLK > > > > > > > > > > > > > > > > imply MTD > > > > > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > b/arch/riscv/Kconfig index > > > > > > > > > > > > > > > > 732a357a99..20a060454b 100644 > > > > > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > M-Mode. > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > S-Mode. > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +endchoice > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge > > > > > > > > > > > > > > > that in the 1st patch in the series. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > > > > > default y @@ -55,11 +72,6 @@ config > > > > > > > > > > > > > > > > RISCV_ISA_C config RISCV_ISA_A > > > > > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V > > > > S-Mode > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > b/arch/riscv/lib/Makefile index > > > > > > > > > > > > > > > > b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += > > > > > > > > > > > > > > > > interrupts.o obj-y += reset.o > > > > > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git > > > > > > > > > > > > > > > > a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > new file mode 100644 index > > > > > > > > > > > > > > > > 0000000000..077e568ca6 > > > > > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > > > > > +/* > > > > > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel > > > > > > > > > > > > > > > > +<anup@brainfault.org> */ > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +#include <common.h> #include <fdtdec.h> > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +static void setup_tbclk(void) { > > > > > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", > > > > __func__); > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +"timebase-frequency", 1000000); } > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +ulong notrace get_tbclk(void) { > > > > > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + return tbclk; } > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT uint64_t notrace > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > > > > > + return n; } #else uint64_t notrace > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; } > > > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > > > > > DM. Could you please implement a DM timer driver for > > > > S-mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > > > > > which we can implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > > > > > driver to do the paper work. > > > > > > > > > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM > > > > driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > > > > > timer driver based on the M-mode driver patch I provided. > > > > > > > > > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > > > > > Rick confirmed that Andes's chip implemented different > > > > > > > > > mtimer/IPI block from SiFive's. > > > > > > > > > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > > > > > 1. SOC can implement "rdtime" in HW 2. SOC can implement > > > > > > > > > > some MMIO device (like CLINT) and emulate "rdtime" in > > > > > > > > > > runtime firmware. > > > > > > > > > > > > > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > > > > > use CLINT device for S-mode timer driver at all. I am not sure > > > > > > > > > why you misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > > > > > matching device in DT? > > > > > > > > > > > > > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > > > > > http://patchwork.ozlabs.org/patch/996902/. > > > > > > > > > > > > Makes sense now. There is no DT based probing. Instead, you are > > > > > > explicitly bind the timer driver. > > > > > > > > > > > > M-mode U-Boot will never use the generic riscv_timer based on > > > > > > "rdtime" instruction. We will mostly have SoC specific timer drivers > > > > > > for M-mode U-Boot. > > > > > > > > > > > > > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > > > > > what bothered me. It should have been architected clearly in the very > > > > > beginning since the timer is supposed to deliver interrupts directly > > > > > into [M|S]IP on the hart and allowing platform implementation to me is > > > > > creating fragmented solutions. If there are other timers available on > > > > > SoC device, I will definitely use them instead. > > > > > > > > Even, I think timer CSRs should be clearly defined and accessible from both M and > > > > S modes. > > > > > > > > > > > > > > > I think your CPU driver should only do the explicit device binding > > > > > > if CONFIG_RISCV_SMODE=y. > > > > > > > > > > > > > > > > Yep I haven't figured out a clean way to do the driver binding if > > > > > there are different mtimer implementations, as the mtimer driver name > > > > > has to be made known to the CPU driver. But I think the S-mode driver > > > > > can be generic. > > > > > > > > > > > There is inter-dependency here. > > > > > > > > > > > > My suggestion is, I can include you CPU driver patch and add DM > > > > > > driver for "rdtime" based upon that. Only If you are fine ?? > > > > > > > > > > > > > > > > I am fine with that. However there are some review comments that need > > > > > be addressed in my v1 CPU/timer driver series so this should be fixed > > > > > at first. Maybe we can just delay the S-mode timer driver support > > > > > until later, after my series goes in. > > > > > > > > > > > > > I think first three patches of this series can go in without S-mode timer patch. > > > > > > > > Regards, > > > > Anup > > > > Hi Anup > > > > get_ticks( ) is indeed an old way which shall be replace by dm timer. > > atcpit100_timer is SoC dm timer which is used by ae350. > > Maybe Bin is trying to say that rdtime shall be called in > > riscv_timer_get_count( ) in riscv_timer.c > > And then trigger a SBI call to M-mode and get mtime when u-boot is > > running in S-mode, right ? > > > > Correct. > > > But the merge window is closed. > > I am not sure if it is appropriate to send a PR at this time. > > > > I think it is OK to send the PR as the first 3 patches of S-mode > support are pretty much RISC-V specific. Yes, please go ahead with first 3 patches. > > > Maybe I can push your first three patches of this series into u-boot-riscv.git > > And send a PR to master when next merge window open. > > > > For my patch series, I will send v2 soon to address the comments and I > hope they can be included in v2019.01 release too, and Anup can send > out the S-mode timer support on top of that. Sure, I will base riscv_timer on your v2. I have thought about this more.... The rdtime based riscv_timer will be enabled by default for S-mode U-Boot but for M-mode U-Boot people can write their own DM timer driver or use riscv_timer driver if their CPU support rdtime in M-mode. Regards, Anup
Hi Anup, On Tue, Dec 4, 2018 at 4:37 PM Anup Patel <anup@brainfault.org> wrote: > > On Tue, Dec 4, 2018 at 1:44 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > Hi Rick, > > > > On Tue, Dec 4, 2018 at 3:12 PM Rick Chen <rickchen36@gmail.com> wrote: > > > > > > > > From: Anup Patel [mailto:anup@brainfault.org] > > > > > Sent: Monday, December 03, 2018 6:30 PM > > > > > To: Bin Meng > > > > > Cc: Rick Jian-Zhi Chen(陳建志); Lukas Auer; Alexander Graf; Palmer Dabbelt; > > > > > Atish Patra; Christoph Hellwig; U-Boot Mailing List > > > > > Subject: Re: [PATCH v7 4/4] RISC-V: Add S-mode timer implementation > > > > > > > > > > On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng > > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel > > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng > > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel > > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > > > > > > rdtimeh instructions for reading timer ticks > > > > > > > > > > > > > > > > > (just like Linux). The frequency of timer ticks > > > > > > > > > > > > > > > > > is passed by prior booting stages in "timebase-frequency" > > > > > DT property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > > > > > > implementation for U-Boot running in S-mode. For > > > > > > > > > > > > > > > > > U-Boot running in M-mode, specific timer drivers will have > > > > > to be provided. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > > > > > > deletions(-) create mode 100644 > > > > > > > > > > > > > > > > > arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > > > > > > > > > > > > > > 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > > > > > > imply BLK > > > > > > > > > > > > > > > > > imply CLK > > > > > > > > > > > > > > > > > imply MTD > > > > > > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > b/arch/riscv/Kconfig index > > > > > > > > > > > > > > > > > 732a357a99..20a060454b 100644 > > > > > > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > > M-Mode. > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > > S-Mode. > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +endchoice > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge > > > > > > > > > > > > > > > > that in the 1st patch in the series. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > > > > > > default y @@ -55,11 +72,6 @@ config > > > > > > > > > > > > > > > > > RISCV_ISA_C config RISCV_ISA_A > > > > > > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V > > > > > S-Mode > > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > b/arch/riscv/lib/Makefile index > > > > > > > > > > > > > > > > > b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += > > > > > > > > > > > > > > > > > interrupts.o obj-y += reset.o > > > > > > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git > > > > > > > > > > > > > > > > > a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > new file mode 100644 index > > > > > > > > > > > > > > > > > 0000000000..077e568ca6 > > > > > > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > > > > > > +/* > > > > > > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel > > > > > > > > > > > > > > > > > +<anup@brainfault.org> */ > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +#include <common.h> #include <fdtdec.h> > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +static void setup_tbclk(void) { > > > > > > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", > > > > > __func__); > > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +"timebase-frequency", 1000000); } > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +ulong notrace get_tbclk(void) { > > > > > > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > + return tbclk; } > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT uint64_t notrace > > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > > > > > > + return n; } #else uint64_t notrace > > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; } > > > > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > > > > > > DM. Could you please implement a DM timer driver for > > > > > S-mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > > > > > > which we can implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > > > > > > driver to do the paper work. > > > > > > > > > > > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM > > > > > driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > > > > > > timer driver based on the M-mode driver patch I provided. > > > > > > > > > > > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > > > > > > Rick confirmed that Andes's chip implemented different > > > > > > > > > > mtimer/IPI block from SiFive's. > > > > > > > > > > > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > > > > > > 1. SOC can implement "rdtime" in HW 2. SOC can implement > > > > > > > > > > > some MMIO device (like CLINT) and emulate "rdtime" in > > > > > > > > > > > runtime firmware. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > > > > > > use CLINT device for S-mode timer driver at all. I am not sure > > > > > > > > > > why you misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > > > > > > matching device in DT? > > > > > > > > > > > > > > > > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > > > > > > http://patchwork.ozlabs.org/patch/996902/. > > > > > > > > > > > > > > Makes sense now. There is no DT based probing. Instead, you are > > > > > > > explicitly bind the timer driver. > > > > > > > > > > > > > > M-mode U-Boot will never use the generic riscv_timer based on > > > > > > > "rdtime" instruction. We will mostly have SoC specific timer drivers > > > > > > > for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > > > > > > what bothered me. It should have been architected clearly in the very > > > > > > beginning since the timer is supposed to deliver interrupts directly > > > > > > into [M|S]IP on the hart and allowing platform implementation to me is > > > > > > creating fragmented solutions. If there are other timers available on > > > > > > SoC device, I will definitely use them instead. > > > > > > > > > > Even, I think timer CSRs should be clearly defined and accessible from both M and > > > > > S modes. > > > > > > > > > > > > > > > > > > I think your CPU driver should only do the explicit device binding > > > > > > > if CONFIG_RISCV_SMODE=y. > > > > > > > > > > > > > > > > > > > Yep I haven't figured out a clean way to do the driver binding if > > > > > > there are different mtimer implementations, as the mtimer driver name > > > > > > has to be made known to the CPU driver. But I think the S-mode driver > > > > > > can be generic. > > > > > > > > > > > > > There is inter-dependency here. > > > > > > > > > > > > > > My suggestion is, I can include you CPU driver patch and add DM > > > > > > > driver for "rdtime" based upon that. Only If you are fine ?? > > > > > > > > > > > > > > > > > > > I am fine with that. However there are some review comments that need > > > > > > be addressed in my v1 CPU/timer driver series so this should be fixed > > > > > > at first. Maybe we can just delay the S-mode timer driver support > > > > > > until later, after my series goes in. > > > > > > > > > > > > > > > > I think first three patches of this series can go in without S-mode timer patch. > > > > > > > > > > Regards, > > > > > Anup > > > > > > Hi Anup > > > > > > get_ticks( ) is indeed an old way which shall be replace by dm timer. > > > atcpit100_timer is SoC dm timer which is used by ae350. > > > Maybe Bin is trying to say that rdtime shall be called in > > > riscv_timer_get_count( ) in riscv_timer.c > > > And then trigger a SBI call to M-mode and get mtime when u-boot is > > > running in S-mode, right ? > > > > > > > Correct. > > > > > But the merge window is closed. > > > I am not sure if it is appropriate to send a PR at this time. > > > > > > > I think it is OK to send the PR as the first 3 patches of S-mode > > support are pretty much RISC-V specific. > > Yes, please go ahead with first 3 patches. > > > > > > Maybe I can push your first three patches of this series into u-boot-riscv.git > > > And send a PR to master when next merge window open. > > > > > > > For my patch series, I will send v2 soon to address the comments and I > > hope they can be included in v2019.01 release too, and Anup can send > > out the S-mode timer support on top of that. > > Sure, I will base riscv_timer on your v2. > > I have thought about this more.... > > The rdtime based riscv_timer will be enabled by default for S-mode U-Boot > but for M-mode U-Boot people can write their own DM timer driver or use > riscv_timer driver if their CPU support rdtime in M-mode. > Agreed. Regards, Bin
Bin Meng <bmeng.cn@gmail.com> 於 2018年12月4日 週二 下午5:36寫道： > > Hi Anup, > > On Tue, Dec 4, 2018 at 4:37 PM Anup Patel <anup@brainfault.org> wrote: > > > > On Tue, Dec 4, 2018 at 1:44 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > Hi Rick, > > > > > > On Tue, Dec 4, 2018 at 3:12 PM Rick Chen <rickchen36@gmail.com> wrote: > > > > > > > > > > From: Anup Patel [mailto:anup@brainfault.org] > > > > > > Sent: Monday, December 03, 2018 6:30 PM > > > > > > To: Bin Meng > > > > > > Cc: Rick Jian-Zhi Chen(陳建志); Lukas Auer; Alexander Graf; Palmer Dabbelt; > > > > > > Atish Patra; Christoph Hellwig; U-Boot Mailing List > > > > > > Subject: Re: [PATCH v7 4/4] RISC-V: Add S-mode timer implementation > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:16 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 4:12 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:27 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:44 PM Anup Patel <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:08 PM Bin Meng <bmeng.cn@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:31 PM Anup Patel <anup@brainfault.org> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:56 PM Bin Meng <bmeng.cn@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 3:19 PM Anup Patel <anup@brainfault.org> > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:32 PM Bin Meng > > > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 2:43 PM Anup Patel > > > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 12:06 PM Bin Meng > > > > > > <bmeng.cn@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Anup, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Dec 3, 2018 at 1:28 PM Anup Patel > > > > > > <anup@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > When running in S-mode, we can use rdtime and > > > > > > > > > > > > > > > > > > rdtimeh instructions for reading timer ticks > > > > > > > > > > > > > > > > > > (just like Linux). The frequency of timer ticks > > > > > > > > > > > > > > > > > > is passed by prior booting stages in "timebase-frequency" > > > > > > DT property of the "/cpus" DT node. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This patch provides a generic timer > > > > > > > > > > > > > > > > > > implementation for U-Boot running in S-mode. For > > > > > > > > > > > > > > > > > > U-Boot running in M-mode, specific timer drivers will have > > > > > > to be provided. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Anup Patel <anup@brainfault.org> > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > arch/Kconfig | 1 - > > > > > > > > > > > > > > > > > > arch/riscv/Kconfig | 22 +++++++++++---- > > > > > > > > > > > > > > > > > > arch/riscv/lib/Makefile | 1 + > > > > > > > > > > > > > > > > > > arch/riscv/lib/time.c | 60 > > > > > > +++++++++++++++++++++++++++++++++++++++++ > > > > > > > > > > > > > > > > > > 4 files changed, 78 insertions(+), 6 > > > > > > > > > > > > > > > > > > deletions(-) create mode 100644 > > > > > > > > > > > > > > > > > > arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > > > > > > > > > > > > > > > 9fdd2f7e66..a4fcb3522d 100644 > > > > > > > > > > > > > > > > > > --- a/arch/Kconfig > > > > > > > > > > > > > > > > > > +++ b/arch/Kconfig > > > > > > > > > > > > > > > > > > @@ -72,7 +72,6 @@ config RISCV > > > > > > > > > > > > > > > > > > imply BLK > > > > > > > > > > > > > > > > > > imply CLK > > > > > > > > > > > > > > > > > > imply MTD > > > > > > > > > > > > > > > > > > - imply TIMER > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, please don't do this. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am removing here but selecting it only for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > imply CMD_DM > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config SANDBOX > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > > b/arch/riscv/Kconfig index > > > > > > > > > > > > > > > > > > 732a357a99..20a060454b 100644 > > > > > > > > > > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > > > > > > > > > > @@ -44,6 +44,23 @@ config ARCH_RV64I > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +choice > > > > > > > > > > > > > > > > > > + prompt "Run Mode" > > > > > > > > > > > > > > > > > > + default RISCV_MMODE > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +config RISCV_MMODE > > > > > > > > > > > > > > > > > > + bool "Machine" > > > > > > > > > > > > > > > > > > + select TIMER > > > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > > > M-Mode. > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +config RISCV_SMODE > > > > > > > > > > > > > > > > > > + bool "Supervisor" > > > > > > > > > > > > > > > > > > + help > > > > > > > > > > > > > > > > > > + Choose this option to build U-Boot for RISC-V > > > > > > S-Mode. > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +endchoice > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The above changes deserve separate patch, or merge > > > > > > > > > > > > > > > > > that in the 1st patch in the series. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sure, I will put Kconfig changes as separate patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > config RISCV_ISA_C > > > > > > > > > > > > > > > > > > bool "Emit compressed instructions" > > > > > > > > > > > > > > > > > > default y @@ -55,11 +72,6 @@ config > > > > > > > > > > > > > > > > > > RISCV_ISA_C config RISCV_ISA_A > > > > > > > > > > > > > > > > > > def_bool y > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -config RISCV_SMODE > > > > > > > > > > > > > > > > > > - bool "Run in S-Mode" > > > > > > > > > > > > > > > > > > - help > > > > > > > > > > > > > > > > > > - Enable this option to build U-Boot for RISC-V > > > > > > S-Mode > > > > > > > > > > > > > > > > > > - > > > > > > > > > > > > > > > > > > config 32BIT > > > > > > > > > > > > > > > > > > bool > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > > b/arch/riscv/lib/Makefile index > > > > > > > > > > > > > > > > > > b58db89752..98aa6d40e7 100644 > > > > > > > > > > > > > > > > > > --- a/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/Makefile > > > > > > > > > > > > > > > > > > @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += > > > > > > > > > > > > > > > > > > interrupts.o obj-y += reset.o > > > > > > > > > > > > > > > > > > obj-y += setjmp.o > > > > > > > > > > > > > > > > > > +obj-$(CONFIG_RISCV_SMODE) += time.o > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > # For building EFI apps > > > > > > > > > > > > > > > > > > CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git > > > > > > > > > > > > > > > > > > a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > new file mode 100644 index > > > > > > > > > > > > > > > > > > 0000000000..077e568ca6 > > > > > > > > > > > > > > > > > > --- /dev/null > > > > > > > > > > > > > > > > > > +++ b/arch/riscv/lib/time.c > > > > > > > > > > > > > > > > > > @@ -0,0 +1,60 @@ > > > > > > > > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0+ > > > > > > > > > > > > > > > > > > +/* > > > > > > > > > > > > > > > > > > + * Copyright (C) 2018, Anup Patel > > > > > > > > > > > > > > > > > > +<anup@brainfault.org> */ > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +#include <common.h> #include <fdtdec.h> > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR; > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +static unsigned int tbclk; > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +static void setup_tbclk(void) { > > > > > > > > > > > > > > > > > > + int cpus; > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > + if (!gd->fdt_blob || tbclk) > > > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); > > > > > > > > > > > > > > > > > > + if (cpus < 0) { > > > > > > > > > > > > > > > > > > + debug("%s: Missing /cpus node\n", > > > > > > __func__); > > > > > > > > > > > > > > > > > > + return; > > > > > > > > > > > > > > > > > > + } > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +"timebase-frequency", 1000000); } > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +ulong notrace get_tbclk(void) { > > > > > > > > > > > > > > > > > > + setup_tbclk(); > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > + return tbclk; } > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT uint64_t notrace > > > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > > > + unsigned long n; > > > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > > > + "rdtime %0" > > > > > > > > > > > > > > > > > > + : "=r" (n)); > > > > > > > > > > > > > > > > > > + return n; } #else uint64_t notrace > > > > > > > > > > > > > > > > > > +get_ticks(void) { > > > > > > > > > > > > > > > > > > + uint32_t lo, hi, tmp; > > > > > > > > > > > > > > > > > > + __asm__ __volatile__ ( > > > > > > > > > > > > > > > > > > + "1:\n" > > > > > > > > > > > > > > > > > > + "rdtimeh %0\n" > > > > > > > > > > > > > > > > > > + "rdtime %1\n" > > > > > > > > > > > > > > > > > > + "rdtimeh %2\n" > > > > > > > > > > > > > > > > > > + "bne %0, %2, 1b" > > > > > > > > > > > > > > > > > > + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); > > > > > > > > > > > > > > > > > > + return ((uint64_t)hi << 32) | lo; } > > > > > > > > > > > > > > > > > > +#endif > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The proper way to implement timer driver is via > > > > > > > > > > > > > > > > > DM. Could you please implement a DM timer driver for > > > > > > S-mode? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The M-mode timer driver is @ > > > > > > http://patchwork.ozlabs.org/patch/996892/, FYI. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > "rdtime" is an instruction. It's not an device for > > > > > > > > > > > > > > > > which we can implement DM driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I know. But that does not mean we cannot write a > > > > > > > > > > > > > > > driver to do the paper work. > > > > > > > > > > > > > > > > > > > > > > > > > > > > There is no DT node for "rdtime" so how should we probe the DM > > > > > > driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think you can do some modification to create the S-mode > > > > > > > > > > > > > timer driver based on the M-mode driver patch I provided. > > > > > > > > > > > > > > > > > > > > > > > > I think you missed my point. > > > > > > > > > > > > > > > > > > > > > > > > CLINT is SiFive specific. You have to rename your driver as > > > > > > > > > > > > clint_timer. The riscv_timer name is in-appropriate. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, I am reconsidering this. Lukas had similar comments, and > > > > > > > > > > > Rick confirmed that Andes's chip implemented different > > > > > > > > > > > mtimer/IPI block from SiFive's. > > > > > > > > > > > > > > > > > > > > > > > We cannot use CLINT udevice for "rdtime" because: > > > > > > > > > > > > 1. SOC can implement "rdtime" in HW 2. SOC can implement > > > > > > > > > > > > some MMIO device (like CLINT) and emulate "rdtime" in > > > > > > > > > > > > runtime firmware. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It looks you misunderstood things. I was not suggesting you > > > > > > > > > > > use CLINT device for S-mode timer driver at all. I am not sure > > > > > > > > > > > why you misunderstood it. Please read my comments carefully. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you explain how will the timer probed called if there is no > > > > > > > > > > matching device in DT? > > > > > > > > > > > > > > > > > > > > > > > > > > > > The timer driver is bound by the CPU driver. See > > > > > > > > > http://patchwork.ozlabs.org/patch/996902/. > > > > > > > > > > > > > > > > Makes sense now. There is no DT based probing. Instead, you are > > > > > > > > explicitly bind the timer driver. > > > > > > > > > > > > > > > > M-mode U-Boot will never use the generic riscv_timer based on > > > > > > > > "rdtime" instruction. We will mostly have SoC specific timer drivers > > > > > > > > for M-mode U-Boot. > > > > > > > > > > > > > > > > > > > > > > Unfortunately yes, M-mode timer driver cannot be generic and that's > > > > > > > what bothered me. It should have been architected clearly in the very > > > > > > > beginning since the timer is supposed to deliver interrupts directly > > > > > > > into [M|S]IP on the hart and allowing platform implementation to me is > > > > > > > creating fragmented solutions. If there are other timers available on > > > > > > > SoC device, I will definitely use them instead. > > > > > > > > > > > > Even, I think timer CSRs should be clearly defined and accessible from both M and > > > > > > S modes. > > > > > > > > > > > > > > > > > > > > > I think your CPU driver should only do the explicit device binding > > > > > > > > if CONFIG_RISCV_SMODE=y. > > > > > > > > > > > > > > > > > > > > > > Yep I haven't figured out a clean way to do the driver binding if > > > > > > > there are different mtimer implementations, as the mtimer driver name > > > > > > > has to be made known to the CPU driver. But I think the S-mode driver > > > > > > > can be generic. > > > > > > > > > > > > > > > There is inter-dependency here. > > > > > > > > > > > > > > > > My suggestion is, I can include you CPU driver patch and add DM > > > > > > > > driver for "rdtime" based upon that. Only If you are fine ?? > > > > > > > > > > > > > > > > > > > > > > I am fine with that. However there are some review comments that need > > > > > > > be addressed in my v1 CPU/timer driver series so this should be fixed > > > > > > > at first. Maybe we can just delay the S-mode timer driver support > > > > > > > until later, after my series goes in. > > > > > > > > > > > > > > > > > > > I think first three patches of this series can go in without S-mode timer patch. > > > > > > > > > > > > Regards, > > > > > > Anup > > > > > > > > Hi Anup > > > > > > > > get_ticks( ) is indeed an old way which shall be replace by dm timer. > > > > atcpit100_timer is SoC dm timer which is used by ae350. > > > > Maybe Bin is trying to say that rdtime shall be called in > > > > riscv_timer_get_count( ) in riscv_timer.c > > > > And then trigger a SBI call to M-mode and get mtime when u-boot is > > > > running in S-mode, right ? > > > > > > > > > > Correct. > > > > > > > But the merge window is closed. > > > > I am not sure if it is appropriate to send a PR at this time. > > > > > > > > > > I think it is OK to send the PR as the first 3 patches of S-mode > > > support are pretty much RISC-V specific. > > > > Yes, please go ahead with first 3 patches. > > > > > > > > > Maybe I can push your first three patches of this series into u-boot-riscv.git > > > > And send a PR to master when next merge window open. > > > > > > > > > > For my patch series, I will send v2 soon to address the comments and I > > > hope they can be included in v2019.01 release too, and Anup can send > > > out the S-mode timer support on top of that. > > > > Sure, I will base riscv_timer on your v2. > > > > I have thought about this more.... > > > > The rdtime based riscv_timer will be enabled by default for S-mode U-Boot > > but for M-mode U-Boot people can write their own DM timer driver or use > > riscv_timer driver if their CPU support rdtime in M-mode. > > > > Agreed. > > Regards, > Bin I have applied to u-boot-riscv/master Travis is running ... Then I will send a PR later Thanks Rick
diff --git a/arch/Kconfig b/arch/Kconfig index 9fdd2f7e66..a4fcb3522d 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -72,7 +72,6 @@ config RISCV imply BLK imply CLK imply MTD - imply TIMER imply CMD_DM config SANDBOX diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 732a357a99..20a060454b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -44,6 +44,23 @@ config ARCH_RV64I endchoice +choice + prompt "Run Mode" + default RISCV_MMODE + +config RISCV_MMODE + bool "Machine" + select TIMER + help + Choose this option to build U-Boot for RISC-V M-Mode. + +config RISCV_SMODE + bool "Supervisor" + help + Choose this option to build U-Boot for RISC-V S-Mode. + +endchoice + config RISCV_ISA_C bool "Emit compressed instructions" default y @@ -55,11 +72,6 @@ config RISCV_ISA_C config RISCV_ISA_A def_bool y -config RISCV_SMODE - bool "Run in S-Mode" - help - Enable this option to build U-Boot for RISC-V S-Mode - config 32BIT bool diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index b58db89752..98aa6d40e7 100644 --- a/arch/riscv/lib/Makefile +++ b/arch/riscv/lib/Makefile @@ -12,6 +12,7 @@ obj-y += cache.o obj-y += interrupts.o obj-y += reset.o obj-y += setjmp.o +obj-$(CONFIG_RISCV_SMODE) += time.o # For building EFI apps CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI) diff --git a/arch/riscv/lib/time.c b/arch/riscv/lib/time.c new file mode 100644 index 0000000000..077e568ca6 --- /dev/null +++ b/arch/riscv/lib/time.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018, Anup Patel <anup@brainfault.org> + */ + +#include <common.h> +#include <fdtdec.h> + +DECLARE_GLOBAL_DATA_PTR; + +static unsigned int tbclk; + +static void setup_tbclk(void) +{ + int cpus; + + if (!gd->fdt_blob || tbclk) + return; + + cpus = fdt_path_offset(gd->fdt_blob, "/cpus"); + if (cpus < 0) { + debug("%s: Missing /cpus node\n", __func__); + return; + } + + tbclk = fdtdec_get_int(gd->fdt_blob, cpus, + "timebase-frequency", 1000000); +} + +ulong notrace get_tbclk(void) +{ + setup_tbclk(); + + return tbclk; +} + +#ifdef CONFIG_64BIT +uint64_t notrace get_ticks(void) +{ + unsigned long n; + + __asm__ __volatile__ ( + "rdtime %0" + : "=r" (n)); + return n; +} +#else +uint64_t notrace get_ticks(void) +{ + uint32_t lo, hi, tmp; + __asm__ __volatile__ ( + "1:\n" + "rdtimeh %0\n" + "rdtime %1\n" + "rdtimeh %2\n" + "bne %0, %2, 1b" + : "=&r" (hi), "=&r" (lo), "=&r" (tmp)); + return ((uint64_t)hi << 32) | lo; +} +#endif
When running in S-mode, we can use rdtime and rdtimeh instructions for reading timer ticks (just like Linux). The frequency of timer ticks is passed by prior booting stages in "timebase-frequency" DT property of the "/cpus" DT node. This patch provides a generic timer implementation for U-Boot running in S-mode. For U-Boot running in M-mode, specific timer drivers will have to be provided. Signed-off-by: Anup Patel <anup@brainfault.org> --- arch/Kconfig | 1 - arch/riscv/Kconfig | 22 +++++++++++---- arch/riscv/lib/Makefile | 1 + arch/riscv/lib/time.c | 60 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/lib/time.c