Re: [U-Boot] [PATCH v2 05/20] timer: Add generic driver for RISC-V privileged architecture defined timer

Hi Bin,

On Fri, 2018-12-07 at 06:14 -0800, Bin Meng wrote:

RISC-V privileged architecture v1.10 defines a real-time counter, exposed as a memory-mapped machine-mode register - mtime. mtime must run at constant frequency, and the platform must provide a mechanism for determining the timebase of mtime. The mtime register has a 64-bit precision on all RV32, RV64, and RV128 systems.

Different platform may have different implementation of the mtime block hence an API riscv_get_time() is required by this driver for platform codes to hide such implementation details. For example, on some platforms mtime is provided by the CLINT module, while on some other platforms a simple 'rdtime' can be used to get the timer counter.

With this timer driver the U-Boot timer functionalities like delay works correctly now.

Signed-off-by: Bin Meng bmeng.cn@gmail.com

---

Changes in v2:

• remove the probe to syscon driver in the timer probe, to make the driver generic, and rely on platform codes to provide the API riscv_get_time().

drivers/timer/Kconfig | 8 +++++++ drivers/timer/Makefile | 1 + drivers/timer/riscv_timer.c | 57 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 drivers/timer/riscv_timer.c

diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index b0e6f32..8995979 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -126,6 +126,14 @@ config OMAP_TIMER help Select this to enable an timer for Omap devices.

+config RISCV_TIMER

• bool "RISC-V timer support"
• depends on RISCV && TIMER
• select RISCV_CLINT

Since we have one generic timer for RISC-V now, I don't think it makes sense to specifically select the CLINT here.

• help

•  Select this to enable support for the timer as defined
  

•  by the RISC-V privileged architecture spec v1.10.
  

nit: should we explicitly mention the version here? v1.11 will also include the mtime CSR, for example. This is not really important, just noticed it now.

Looks good otherwise.

Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de

Thanks, Lukas

config ROCKCHIP_TIMER bool "Rockchip timer support" depends on TIMER diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile index c4fbab2..d0bf218 100644 --- a/drivers/timer/Makefile +++ b/drivers/timer/Makefile @@ -13,6 +13,7 @@ obj-$(CONFIG_CADENCE_TTC_TIMER) += cadence- ttc.o obj-$(CONFIG_DESIGNWARE_APB_TIMER) += dw-apb-timer.o obj-$(CONFIG_MPC83XX_TIMER) += mpc83xx_timer.o obj-$(CONFIG_OMAP_TIMER) += omap-timer.o +obj-$(CONFIG_RISCV_TIMER) += riscv_timer.o obj-$(CONFIG_ROCKCHIP_TIMER) += rockchip_timer.o obj-$(CONFIG_SANDBOX_TIMER) += sandbox_timer.o obj-$(CONFIG_STI_TIMER) += sti-timer.o diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c new file mode 100644 index 0000000..ef3bedc --- /dev/null +++ b/drivers/timer/riscv_timer.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2018, Bin Meng bmeng.cn@gmail.com
• • RISC-V privileged architecture 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 v1.10.
• • This driver can be used by both M-mode and S-mode.
• */

+#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.

• • @return: 64-bit timer counter as defined by the RISC-V

privileged

• • architecture spec v1.10.
    

• */

+extern u64 riscv_get_time(void);

+static int riscv_timer_get_count(struct udevice *dev, u64 *count) +{

• *count = riscv_get_time();
• return 0;

+}

+static int riscv_timer_probe(struct udevice *dev) +{

• struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
• /* clock frequency was passed from the cpu driver as driver

data */

• uc_priv->clock_rate = dev->driver_data;
• return 0;

+}

+static const struct timer_ops riscv_timer_ops = {

• .get_count = riscv_timer_get_count,

+};

+U_BOOT_DRIVER(riscv_timer) = {

• .name = "riscv_timer",
• .id = UCLASS_TIMER,
• .probe = riscv_timer_probe,
• .ops = &riscv_timer_ops,
• .flags = DM_FLAG_PRE_RELOC,

+};