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

Hi Sean

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@gmail.com

arch/riscv/lib/Makefile | 1 - arch/riscv/lib/rdtime.c | 38 ------------------------------------ drivers/timer/Kconfig | 6 +++--- drivers/timer/riscv_timer.c | 39 +++++++++++++++++++------------------ 4 files changed, 23 insertions(+), 61 deletions(-) delete mode 100644 arch/riscv/lib/rdtime.c

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@brainfault.org
• • Copyright (C) 2018, Bin Meng bmeng.cn@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;
  

-}

config RISCV_RDTIME in arch/riscv/Kconfig shall be removed meanwhile.

Thanks, Rick

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
    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@gmail.com
  • Copyright (C) 2018, Bin Meng bmeng.cn@gmail.com
• • Copyright (C) 2018, Anup Patel anup@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.27.0