[U-Boot] [PATCH v7 4/4] RISC-V: Add S-mode timer implementation

[Anup Patel]

On Mon, Dec 3, 2018 at 12:06 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Anup,
>
> On Mon, Dec 3, 2018 at 1:28 PM Anup Patel <anup at 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 at 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 at 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