On 7/28/20 5:11 AM, Rick Chen wrote: > 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 <sean...@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 <a...@brainfault.org> >> - * Copyright (C) 2018, Bin Meng <bmeng...@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
Ok, I will remove that in V2. >> 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 <sean...@gmail.com> >> * Copyright (C) 2018, Bin Meng <bmeng...@gmail.com> >> + * Copyright (C) 2018, Anup Patel <a...@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 >>