On 11/11/20 12:56 PM, Pragnesh Patel wrote:
Hi Heinrich,
-----Original Message-----
From: Heinrich Schuchardt <xypron.g...@gmx.de>
Sent: 11 November 2020 16:51
To: Pragnesh Patel <pragnesh.pa...@openfive.com>; u-boot@lists.denx.de
Cc: atish.pa...@wdc.com; palmerdabb...@google.com; bmeng...@gmail.com;
Paul Walmsley ( Sifive) <paul.walms...@sifive.com>; anup.pa...@wdc.com;
Sagar Kadam <sagar.ka...@openfive.com>; r...@andestech.com; Sean
Anderson <sean...@gmail.com>; Simon Glass <s...@chromium.org>
Subject: Re: [PATCH v3 1/1] riscv: Add timer_get_us() for tracing
[External Email] Do not click links or attachments unless you recognize the
sender and know the content is safe
On 11.11.20 11:14, Pragnesh Patel wrote:
Add timer_get_us() which is useful for tracing.
For S-mode U-Boot, CSR_TIMEH and CSR_TIME will provide a timer ticks
and For M-mode U-Boot, mtime register will provide the same.
Signed-off-by: Pragnesh Patel <pragnesh.pa...@sifive.com>
The default implementation of get_timer_us() in lib/time.c calls
get_ticks() which calls timer_get_count(). The get_count() operation is
implemented in drivers/timer/andes_plmt_timer.c,
drivers/timer/sifive_clint_timer.c, drivers/timer/riscv_timer.c.
Why do you need special timer_get_us() implementations?
Isn't it enough to supply the get_count() operation in the drivers?
get_ticks() is depend on gd->timer and there are 2 cases
1) if gd->timer== NULL then dm_timer_init() gets called and it will call
functions
which are not marked with "notrace" so tracing got stuck.
Thanks for the background information.
Please, identify the existing functions that lack "notrace" and fix
them. This will give us a solution for all existing boards and not for a
small selection. Furthermore it will avoid code duplication.
In lib/trace.c consider replacing
"__attribute__((no_instrument_function))" by "notrace".
Best regards
Heinrich
2) if gd->timer is already initialized then still initr_dm() will make
gd->timer = NULL;
initr_dm()
{
#ifdef CONFIG_TIMER
gd->timer = NULL;
#endif
}
So again dm_timer_init() gets called and tracing got stuck.
So I thought better to implement timer_get_us().
Best regards
Heinrich
---
Changes in v3:
- Added gd->arch.plmt in global data
- For timer_get_us(), use readq() instead of andes_plmt_get_count()
and sifive_clint_get_count()
Changes in v2:
- Added timer_get_us() in riscv_timer.c, sifive_clint_timer.c
and andes_plmt_timer.c.
arch/riscv/include/asm/global_data.h | 3 +++
drivers/timer/andes_plmt_timer.c | 19 ++++++++++++++++++-
drivers/timer/riscv_timer.c | 14 +++++++++++++-
drivers/timer/sifive_clint_timer.c | 19 ++++++++++++++++++-
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/include/asm/global_data.h
b/arch/riscv/include/asm/global_data.h
index d3a0b1d221..4e22ceb83f 100644
--- a/arch/riscv/include/asm/global_data.h
+++ b/arch/riscv/include/asm/global_data.h
@@ -24,6 +24,9 @@ struct arch_global_data { #ifdef CONFIG_ANDES_PLIC
void __iomem *plic; /* plic base address */
#endif
+#ifdef CONFIG_ANDES_PLMT
+ void __iomem *plmt; /* plmt base address */
+#endif
#if CONFIG_IS_ENABLED(SMP)
struct ipi_data ipi[CONFIG_NR_CPUS]; #endif diff --git
a/drivers/timer/andes_plmt_timer.c b/drivers/timer/andes_plmt_timer.c
index cec86718c7..7c50c46d9e 100644
--- a/drivers/timer/andes_plmt_timer.c
+++ b/drivers/timer/andes_plmt_timer.c
@@ -13,11 +13,12 @@
#include <timer.h>
#include <asm/io.h>
#include <linux/err.h>
+#include <div64.h>
/* mtime register */
#define MTIME_REG(base) ((ulong)(base))
-static u64 andes_plmt_get_count(struct udevice *dev)
+static u64 notrace andes_plmt_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv)); } @@ -26,12
+27,28 @@ static const struct timer_ops andes_plmt_ops = {
.get_count = andes_plmt_get_count, };
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_get_us(void) {
+ u64 ticks;
+
+ /* FIXME: gd->arch.plmt should contain valid base address */
+ if (gd->arch.plmt) {
+ ticks = readq((void __iomem *)MTIME_REG(gd->arch.plmt));
+ do_div(ticks, CONFIG_SYS_HZ);
+ }
+
+ return ticks;
+}
+#endif
+
static int andes_plmt_probe(struct udevice *dev) {
dev->priv = dev_read_addr_ptr(dev);
if (!dev->priv)
return -EINVAL;
+ gd->arch.plmt = dev->priv;
return timer_timebase_fallback(dev); }
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 21ae184057..7fa8773da3 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -15,8 +15,9 @@
#include <errno.h>
#include <timer.h>
#include <asm/csr.h>
+#include <div64.h>
-static u64 riscv_timer_get_count(struct udevice *dev)
+static u64 notrace riscv_timer_get_count(struct udevice *dev)
{
__maybe_unused u32 hi, lo;
@@ -31,6 +32,17 @@ static u64 riscv_timer_get_count(struct udevice *dev)
return ((u64)hi << 32) | lo;
}
+#if CONFIG_IS_ENABLED(RISCV_SMODE)
+unsigned long notrace timer_get_us(void) {
+ u64 ticks;
+
+ ticks = riscv_timer_get_count(NULL);
+ do_div(ticks, CONFIG_SYS_HZ);
+ return ticks;
+}
+#endif
+
static int riscv_timer_probe(struct udevice *dev) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); diff
--git a/drivers/timer/sifive_clint_timer.c
b/drivers/timer/sifive_clint_timer.c
index 00ce0f08d6..c341f7789b 100644
--- a/drivers/timer/sifive_clint_timer.c
+++ b/drivers/timer/sifive_clint_timer.c
@@ -10,11 +10,12 @@
#include <timer.h>
#include <asm/io.h>
#include <linux/err.h>
+#include <div64.h>
/* mtime register */
#define MTIME_REG(base) ((ulong)(base) + 0xbff8)
-static u64 sifive_clint_get_count(struct udevice *dev)
+static u64 notrace sifive_clint_get_count(struct udevice *dev)
{
return readq((void __iomem *)MTIME_REG(dev->priv)); } @@ -23,12
+24,28 @@ static const struct timer_ops sifive_clint_ops = {
.get_count = sifive_clint_get_count, };
+#if CONFIG_IS_ENABLED(RISCV_MMODE)
+unsigned long notrace timer_get_us(void) {
+ u64 ticks;
+
+ /* FIXME: gd->arch.clint should contain valid base address */
+ if (gd->arch.clint) {
+ ticks = readq((void __iomem *)MTIME_REG(gd->arch.clint));
+ do_div(ticks, CONFIG_SYS_HZ);
+ }
+
+ return ticks;
+}
+#endif
+
static int sifive_clint_probe(struct udevice *dev) {
dev->priv = dev_read_addr_ptr(dev);
if (!dev->priv)
return -EINVAL;
+ gd->arch.clint = dev->priv;
return timer_timebase_fallback(dev); }
