[U-Boot] [PATCH v7 4/4] RISC-V: Add S-mode timer implementation
Anup Patel
anup at brainfault.org
Mon Dec 3 07:19:38 UTC 2018
On Mon, Dec 3, 2018 at 12:32 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Anup,
>
> On Mon, Dec 3, 2018 at 2:43 PM Anup Patel <anup at brainfault.org> wrote:
> >
> > 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.
> >
>
> 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
More information about the U-Boot
mailing list