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

Message ID 1544192072-28764-6-git-send-email-bmeng.cn@gmail.com
State Superseded
Delegated to: Andes
Headers show
Series
  • riscv: Adding RISC-V CPU and timer driver
Related show

Commit Message

Bin Meng Dec. 7, 2018, 2:14 p.m.
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

Comments

Auer, Lukas Dec. 10, 2018, 11:17 p.m. | #1
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,
> +};
Bin Meng Dec. 11, 2018, 1:49 a.m. | #2
Hi Lukas,

On Tue, Dec 11, 2018 at 7:17 AM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> 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.
>

Ah, yes!

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

OK, will remove it in v3.

> Looks good otherwise.
>
> Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
>

Regards,
Bin

Patch

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
+	help
+	  Select this to enable support for the timer as defined
+	  by the RISC-V privileged architecture spec v1.10.
+
 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,
+};