[PATCH v2 0/3] bootstage support for risc-v
This adds to support bootstage for risc-v. timer_get_boot_us function is required to record each boot stages with microsecond timestamp.
Changes from v1: - Correct #ifdef guard for riscv_aclint_timer and riscv_timer
Chanho Park (3): riscv: bootstage: correct bootstage_report guard riscv: timer: add timer_get_boot_us for BOOTSTAGE timer: riscv_aclint_timer: add timer_get_boot_us for BOOTSTAGE
arch/riscv/lib/bootm.c | 2 +- drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ drivers/timer/riscv_timer.c | 21 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-)
Below warning can be occurred when CONFIG_BOOTSTAGE and !CONFIG_SPL_BOOTSTAGE. It should be guarded by using CONFIG_IS_ENABLED for SPL build.
arch/riscv/lib/bootm.c:46:9: warning: implicit declaration of function 'bootstage_report' 46 | bootstage_report(); | ^~~~~~~~~~~~~~~~ | bootstage_error
Signed-off-by: Chanho Park chanho61.park@samsung.com --- arch/riscv/lib/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c index 276677a5e2f9..cc30efc90498 100644 --- a/arch/riscv/lib/bootm.c +++ b/arch/riscv/lib/bootm.c @@ -42,7 +42,7 @@ static void announce_and_cleanup(int fake) #ifdef CONFIG_BOOTSTAGE_FDT bootstage_fdt_add_report(); #endif -#ifdef CONFIG_BOOTSTAGE_REPORT +#if CONFIG_IS_ENABLED(BOOTSTAGE_REPORT) bootstage_report(); #endif
On Mon, Aug 28, 2023 at 06:49:36PM +0900, Chanho Park wrote:
Below warning can be occurred when CONFIG_BOOTSTAGE and !CONFIG_SPL_BOOTSTAGE. It should be guarded by using CONFIG_IS_ENABLED for SPL build.
arch/riscv/lib/bootm.c:46:9: warning: implicit declaration of function 'bootstage_report' 46 | bootstage_report(); | ^~~~~~~~~~~~~~~~ | bootstage_error
Signed-off-by: Chanho Park chanho61.park@samsung.com
arch/riscv/lib/bootm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Leo Yu-Chi Liang ycliang@andestech.com
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com --- drivers/timer/riscv_timer.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index 3627ed79b819..6cb589fcdc45 100644 --- a/drivers/timer/riscv_timer.c +++ b/drivers/timer/riscv_timer.c @@ -11,6 +11,7 @@ */
#include <common.h> +#include <div64.h> #include <dm.h> #include <errno.h> #include <timer.h> @@ -50,6 +51,26 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_SMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) +{ + int ret; + u64 ticks = 0; + u32 rate; + + ret = dm_timer_init(); + if (!ret) { + rate = timer_get_rate(gd->timer); + timer_get_count(gd->timer, &ticks); + } else { + rate = RISCV_SMODE_TIMER_FREQ; + ticks = riscv_timer_get_count(NULL); + } + + return lldiv(ticks * 1000, (rate / 1000)); +} +#endif + static int riscv_timer_probe(struct udevice *dev) { struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
Hi Chanho,
On Mon, Aug 28, 2023 at 06:49:37PM +0900, Chanho Park wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_timer.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index 3627ed79b819..6cb589fcdc45 100644 --- a/drivers/timer/riscv_timer.c +++ b/drivers/timer/riscv_timer.c @@ -11,6 +11,7 @@ */
#include <common.h> +#include <div64.h> #include <dm.h> #include <errno.h> #include <timer.h> @@ -50,6 +51,26 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_SMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) +{
- int ret;
- u64 ticks = 0;
- u32 rate;
- ret = dm_timer_init();
- if (!ret) {
rate = timer_get_rate(gd->timer);timer_get_count(gd->timer, &ticks);- } else {
rate = RISCV_SMODE_TIMER_FREQ;ticks = riscv_timer_get_count(NULL);- }
- return lldiv(ticks * 1000, (rate / 1000));
Could you elaborate a little how this formula is derived?
Best regards, Leo
+} +#endif
static int riscv_timer_probe(struct udevice *dev) { struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); -- 2.39.2
Hi,
-----Original Message----- From: Leo Liang ycliang@andestech.com Sent: Monday, September 4, 2023 4:01 PM To: Chanho Park chanho61.park@samsung.com Cc: Rick Chen rick@andestech.com; Simon Glass sjg@chromium.org; u- boot@lists.denx.de Subject: Re: [PATCH v2 2/3] riscv: timer: add timer_get_boot_us for BOOTSTAGE
Hi Chanho,
On Mon, Aug 28, 2023 at 06:49:37PM +0900, Chanho Park wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_timer.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index 3627ed79b819..6cb589fcdc45 100644 --- a/drivers/timer/riscv_timer.c +++ b/drivers/timer/riscv_timer.c @@ -11,6 +11,7 @@ */
#include <common.h> +#include <div64.h> #include <dm.h> #include <errno.h> #include <timer.h> @@ -50,6 +51,26 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_SMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) {
- int ret;
- u64 ticks = 0;
- u32 rate;
- ret = dm_timer_init();
- if (!ret) {
rate = timer_get_rate(gd->timer);timer_get_count(gd->timer, &ticks);- } else {
rate = RISCV_SMODE_TIMER_FREQ;ticks = riscv_timer_get_count(NULL);- }
- return lldiv(ticks * 1000, (rate / 1000));
Could you elaborate a little how this formula is derived?
Sure. Below is the original formula. To avoid zero of (tick/rate), I split the calculation to ticks and rate, respectively.
Time(us) = (ticks / rate) * 1000000 Or (ticks * 1000000) / rate Or tick / (rate / 10000000)
Best Regards, Chanho Park
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com --- drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/timer/riscv_aclint_timer.c b/drivers/timer/riscv_aclint_timer.c index e29d527c8d77..8b67745bb4a2 100644 --- a/drivers/timer/riscv_aclint_timer.c +++ b/drivers/timer/riscv_aclint_timer.c @@ -6,6 +6,7 @@
#include <common.h> #include <clk.h> +#include <div64.h> #include <dm.h> #include <timer.h> #include <asm/io.h> @@ -44,6 +45,27 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_MMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) +{ + int ret; + u64 ticks = 0; + u32 rate; + + ret = dm_timer_init(); + if (!ret) { + rate = timer_get_rate(gd->timer); + timer_get_count(gd->timer, &ticks); + } else { + rate = RISCV_MMODE_TIMER_FREQ; + ticks = readq((void __iomem *)MTIME_REG(RISCV_MMODE_TIMERBASE, + RISCV_MMODE_TIMEROFF)); + } + + return lldiv(ticks * 1001, (rate / 1000)); +} +#endif + static const struct timer_ops riscv_aclint_timer_ops = { .get_count = riscv_aclint_timer_get_count, };
Hi Chanho,
On Mon, Aug 28, 2023 at 06:49:38PM +0900, Chanho Park wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/timer/riscv_aclint_timer.c b/drivers/timer/riscv_aclint_timer.c index e29d527c8d77..8b67745bb4a2 100644 --- a/drivers/timer/riscv_aclint_timer.c +++ b/drivers/timer/riscv_aclint_timer.c @@ -6,6 +6,7 @@
#include <common.h> #include <clk.h> +#include <div64.h> #include <dm.h> #include <timer.h> #include <asm/io.h> @@ -44,6 +45,27 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_MMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) +{
- int ret;
- u64 ticks = 0;
- u32 rate;
- ret = dm_timer_init();
- if (!ret) {
rate = timer_get_rate(gd->timer);timer_get_count(gd->timer, &ticks);- } else {
rate = RISCV_MMODE_TIMER_FREQ;ticks = readq((void __iomem *)MTIME_REG(RISCV_MMODE_TIMERBASE,RISCV_MMODE_TIMEROFF));- }
- return lldiv(ticks * 1001, (rate / 1000));
Why is this dividend 1001 ?
Best regards, Leo
+} +#endif
static const struct timer_ops riscv_aclint_timer_ops = { .get_count = riscv_aclint_timer_get_count, }; -- 2.39.2
Hi,
-----Original Message----- From: Leo Liang ycliang@andestech.com Sent: Monday, September 4, 2023 4:02 PM To: Chanho Park chanho61.park@samsung.com Cc: Rick Chen rick@andestech.com; Simon Glass sjg@chromium.org; u- boot@lists.denx.de Subject: Re: [PATCH v2 3/3] timer: riscv_aclint_timer: add timer_get_boot_us for BOOTSTAGE
Hi Chanho,
On Mon, Aug 28, 2023 at 06:49:38PM +0900, Chanho Park wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/timer/riscv_aclint_timer.c b/drivers/timer/riscv_aclint_timer.c index e29d527c8d77..8b67745bb4a2 100644 --- a/drivers/timer/riscv_aclint_timer.c +++ b/drivers/timer/riscv_aclint_timer.c @@ -6,6 +6,7 @@
#include <common.h> #include <clk.h> +#include <div64.h> #include <dm.h> #include <timer.h> #include <asm/io.h> @@ -44,6 +45,27 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_MMODE) && CONFIG_IS_ENABLED(BOOTSTAGE) +ulong timer_get_boot_us(void) {
- int ret;
- u64 ticks = 0;
- u32 rate;
- ret = dm_timer_init();
- if (!ret) {
rate = timer_get_rate(gd->timer);timer_get_count(gd->timer, &ticks);- } else {
rate = RISCV_MMODE_TIMER_FREQ;ticks = readq((void __iomem*)MTIME_REG(RISCV_MMODE_TIMERBASE,
RISCV_MMODE_TIMEROFF));
- }
- return lldiv(ticks * 1001, (rate / 1000));
Why is this dividend 1001 ?
It's a typo. I'll correct when I send v2.
Best Regards, Chanho Park
Hi Chanho,
On Mon, 28 Aug 2023 at 03:50, Chanho Park chanho61.park@samsung.com wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/timer/riscv_aclint_timer.c b/drivers/timer/riscv_aclint_timer.c index e29d527c8d77..8b67745bb4a2 100644 --- a/drivers/timer/riscv_aclint_timer.c +++ b/drivers/timer/riscv_aclint_timer.c @@ -6,6 +6,7 @@
#include <common.h> #include <clk.h> +#include <div64.h> #include <dm.h> #include <timer.h> #include <asm/io.h> @@ -44,6 +45,27 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_MMODE) && CONFIG_IS_ENABLED(BOOTSTAGE)
Just a nit...you should not need this #if, since if the function is not used it will be stripped from the image by the linker.
+ulong timer_get_boot_us(void) +{
int ret;u64 ticks = 0;u32 rate;ret = dm_timer_init();if (!ret) {rate = timer_get_rate(gd->timer);timer_get_count(gd->timer, &ticks);} else {rate = RISCV_MMODE_TIMER_FREQ;ticks = readq((void __iomem *)MTIME_REG(RISCV_MMODE_TIMERBASE,RISCV_MMODE_TIMEROFF));}return lldiv(ticks * 1001, (rate / 1000));+} +#endif
static const struct timer_ops riscv_aclint_timer_ops = { .get_count = riscv_aclint_timer_get_count, }; -- 2.39.2
Regards, Simon
Hi Simon,
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, September 5, 2023 1:49 AM To: Chanho Park chanho61.park@samsung.com Cc: Rick Chen rick@andestech.com; Leo ycliang@andestech.com; u- boot@lists.denx.de Subject: Re: [PATCH v2 3/3] timer: riscv_aclint_timer: add timer_get_boot_us for BOOTSTAGE
Hi Chanho,
On Mon, 28 Aug 2023 at 03:50, Chanho Park chanho61.park@samsung.com wrote:
timer_get_boot_us function is required to record the boot stages as us-based timestamp.
Signed-off-by: Chanho Park chanho61.park@samsung.com
drivers/timer/riscv_aclint_timer.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
diff --git a/drivers/timer/riscv_aclint_timer.c
b/drivers/timer/riscv_aclint_timer.c
index e29d527c8d77..8b67745bb4a2 100644 --- a/drivers/timer/riscv_aclint_timer.c +++ b/drivers/timer/riscv_aclint_timer.c @@ -6,6 +6,7 @@
#include <common.h> #include <clk.h> +#include <div64.h> #include <dm.h> #include <timer.h> #include <asm/io.h> @@ -44,6 +45,27 @@ u64 notrace timer_early_get_count(void) } #endif
+#if CONFIG_IS_ENABLED(RISCV_MMODE) && CONFIG_IS_ENABLED(BOOTSTAGE)
Just a nit...you should not need this #if, since if the function is not used it will be stripped from the image by the linker.
Thank you for the suggestion. Without these guards, compile error can be occurred due to multiple timer_get_boot_us definitions. For risc-v, there are two timers for S-mode and M-mode, respectively. So, I put them to avoid below build errors.
riscv64-unknown-linux-gnu-ld.bfd: drivers/timer/riscv_aclint_timer.o: in function `timer_get_boot_us':
/data/risc-v/qemu/u-boot/drivers/timer/riscv_aclint_timer.c:49: multiple definition of `timer_get_boot_us'; drivers/timer/riscv_timer.o:/data/risc-v/qemu/u-boot/drivers/timer/riscv_timer.c:55: first defined here
make[1]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
Best Regards, Chanho Park
participants (3)
-
Chanho Park -
Leo Liang -
Simon Glass