[PATCH v3 1/7] riscv: Rework riscv timer driver to only support S-mode

[Pragnesh Patel]

Hi Sean,

>-----Original Message-----
>From: Sean Anderson <seanga2 at gmail.com>
>Sent: 01 September 2020 16:02
>To: u-boot at lists.denx.de
>Cc: Rick Chen <rickchen36 at gmail.com>; Bin Meng <bmeng.cn at gmail.com>;
>Pragnesh Patel <pragnesh.patel at openfive.com>; Sean Anderson
><seanga2 at gmail.com>; Bin Meng <bin.meng at windriver.com>; Anup Patel
><anup at brainfault.org>
>Subject: [PATCH v3 1/7] riscv: Rework riscv timer driver to only support S-mode
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>The riscv-timer driver currently serves as a shim for several riscv timer drivers.
>This is not too desirable because it bypasses the usual timer selection via the
>driver model. There is no easy way to specify an alternate timing driver, or have
>the tick rate depend on the cpu's configured frequency. The timer drivers also do
>not have device structs, and so have to rely on storing parameters in gd_t. Lastly,
>there is no initialization call, so driver init is done in the same function which
>reads the time. This can result in confusing error messages. To a user, it looks like
>the driver failed when trying to read the time, whereas it may have failed while
>initializing.
>
>This patch removes the shim functionality from the riscv-timer driver, and has it
>instead implement the former rdtime.c timer driver. This is because existing u-
>boot users who pass in a device tree (e.g. qemu) do not create a timer device for
>S-mode u-boot. The existing behavior of creating the riscv-timer device in the
>riscv cpu driver must be kept. The actual reading of the CSRs has been redone in
>the style of Linux's get_cycles64.
>
>Signed-off-by: Sean Anderson <seanga2 at gmail.com>
>Reviewed-by: Bin Meng <bin.meng at windriver.com>
>---
>
>(no changes since v2)
>
>Changes in v2:
>- Remove RISCV_RDTIME KConfig option
>
> arch/riscv/Kconfig          |  8 --------
> arch/riscv/lib/Makefile     |  1 -
> arch/riscv/lib/rdtime.c     | 38 ------------------------------------
> drivers/timer/Kconfig       |  6 +++---
> drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------
> 5 files changed, 23 insertions(+), 69 deletions(-)  delete mode 100644
>arch/riscv/lib/rdtime.c
>
>diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 009a545fcf..21e6690f4d
>100644
>--- a/arch/riscv/Kconfig
>+++ b/arch/riscv/Kconfig
>@@ -185,14 +185,6 @@ config ANDES_PLMT
>          The Andes PLMT block holds memory-mapped mtime register
>          associated with timer tick.
>
>-config RISCV_RDTIME
>-       bool
>-       default y if RISCV_SMODE || SPL_RISCV_SMODE
>-       help
>-         The provides the riscv_get_time() API that is implemented using the
>-         standard rdtime instruction. This is the case for S-mode U-Boot, and
>-         is useful for processors that support rdtime in M-mode too.
>-
> config SYS_MALLOC_F_LEN
>        default 0x1000
>
>diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index
>6c503ff2b2..10ac5b06d3 100644
>--- a/arch/riscv/lib/Makefile
>+++ b/arch/riscv/lib/Makefile
>@@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
> obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o  else
>-obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> obj-$(CONFIG_SBI) += sbi.o
> obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> endif
>diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c deleted file mode
>100644 index e128d7fce6..0000000000
>--- a/arch/riscv/lib/rdtime.c
>+++ /dev/null
>@@ -1,38 +0,0 @@
>-// SPDX-License-Identifier: GPL-2.0+
>-/*
>- * Copyright (C) 2018, Anup Patel <anup at brainfault.org>
>- * Copyright (C) 2018, Bin Meng <bmeng.cn at gmail.com>
>- *
>- * The riscv_get_time() API implementation that is using the
>- * standard rdtime instruction.
>- */
>-
>-#include <common.h>
>-
>-/* Implement the API required by RISC-V timer driver */ -int riscv_get_time(u64
>*time) -{ -#ifdef CONFIG_64BIT
>-       u64 n;
>-
>-       __asm__ __volatile__ (
>-               "rdtime %0"
>-               : "=r" (n));
>-
>-       *time = n;
>-#else
>-       u32 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));
>-
>-       *time = ((u64)hi << 32) | lo;
>-#endif
>-
>-       return 0;
>-}
>diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index
>637024445c..b85fa33e47 100644
>--- a/drivers/timer/Kconfig
>+++ b/drivers/timer/Kconfig
>@@ -144,10 +144,10 @@ config OMAP_TIMER
>
> config RISCV_TIMER
>        bool "RISC-V timer support"
>-       depends on TIMER && RISCV
>+       depends on TIMER && RISCV_SMODE

What about SPL_RISCV_SMODE ?

>        help
>-         Select this to enable support for the timer as defined
>-         by the RISC-V privileged architecture spec.
>+         Select this to enable support for a generic RISC-V S-Mode timer
>+         driver.
>
> config ROCKCHIP_TIMER
>        bool "Rockchip timer support"
>diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index
>9f9f070e0b..449fcfcfd5 100644
>--- a/drivers/timer/riscv_timer.c
>+++ b/drivers/timer/riscv_timer.c
>@@ -1,36 +1,37 @@
> // SPDX-License-Identifier: GPL-2.0+
> /*
>+ * Copyright (C) 2020, Sean Anderson <seanga2 at gmail.com>
>  * Copyright (C) 2018, Bin Meng <bmeng.cn at gmail.com>
>+ * Copyright (C) 2018, Anup Patel <anup at brainfault.org>
>+ * Copyright (C) 2012 Regents of the University of California
>  *
>- * RISC-V privileged architecture defined generic timer driver
>+ * RISC-V architecturally-defined generic timer driver
>  *
>- * This driver relies on RISC-V platform codes to provide the essential API
>- * riscv_get_time() which is supposed to return the timer counter as defined
>- * by the RISC-V privileged architecture spec.
>- *
>- * This driver can be used in both M-mode and S-mode U-Boot.
>+ * This driver provides generic timer support for S-mode U-Boot.
>  */
>
> #include <common.h>
> #include <dm.h>
> #include <errno.h>
> #include <timer.h>
>-#include <asm/io.h>
>-
>-/**
>- * riscv_get_time() - get the timer counter
>- *
>- * Platform codes should provide this API in order to make this driver function.
>- *
>- * @time:      the 64-bit timer count  as defined by the RISC-V privileged
>- *             architecture spec.
>- * @return:    0 on success, -ve on error.
>- */
>-extern int riscv_get_time(u64 *time);
>+#include <asm/csr.h>
>
> static int riscv_timer_get_count(struct udevice *dev, u64 *count)  {
>-       return riscv_get_time(count);
>+       if (IS_ENABLED(CONFIG_64BIT)) {
>+               *count = csr_read(CSR_TIME);
>+       } else {
>+               u32 hi, lo;
>+
>+               do {
>+                       hi = csr_read(CSR_TIMEH);
>+                       lo = csr_read(CSR_TIME);
>+               } while (hi != csr_read(CSR_TIMEH));
>+
>+               *count = ((u64)hi << 32) | lo;
>+       }
>+
>+       return 0;
> }
>
> static int riscv_timer_probe(struct udevice *dev)
>--
>2.28.0