[PATCH 09/10] bootstage: Migrate from timer_get_boot_us() to timer_get_us()
Michal Simek
monstr at monstr.eu
Fri Sep 30 13:30:20 CEST 2022
Hi Stefan,
st 21. 9. 2022 v 16:08 odesílatel Stefan Roese <sr at denx.de> napsal:
>
> This patch migrates the bootstage code from using the boot specific
> timer_get_boot_us() timer function to the common timer_get_us()
> function. This can only be done, also supporting the early boot phase,
> when CONFIG_TIMER_EARLY is enabled. This way, the common timer functions
> provide this functionality. Because of this, CONFIG_TIMER_EARLY is
> now selected via CONFIG_BOOTSTAGE.
>
> Additionally all 'uint32_t' timer occurances are changed to 'ulong', as
> this is the return value of timer_get_us(). Otherwise this might lead
> to 32bit vs 64bit conversion issues on 64bit platforms.
>
> Also the start time of the bootstage recording is now saved. Otherwise
> bigger timing offsets are seen on platforms where the timer does not
> start with 0.
>
> Signed-off-by: Stefan Roese <sr at denx.de>
> ---
> boot/Kconfig | 1 +
> common/bootstage.c | 26 ++++++++++++++------------
> include/bootstage.h | 17 +++++------------
> 3 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 6b3b8f072cb9..2ac7752a2eef 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -642,6 +642,7 @@ menu "Boot timing"
>
> config BOOTSTAGE
> bool "Boot timing and reporting"
> + select TIMER_EARLY
> help
> Enable recording of boot time while booting. To use it, insert
> calls to bootstage_mark() with a suitable BOOTSTAGE_ID from
> diff --git a/common/bootstage.c b/common/bootstage.c
> index 326c40f1561f..6a655b20196b 100644
> --- a/common/bootstage.c
> +++ b/common/bootstage.c
> @@ -30,7 +30,7 @@ enum {
>
> struct bootstage_record {
> ulong time_us;
> - uint32_t start_us;
> + ulong start_us;
> const char *name;
> int flags; /* see enum bootstage_flags */
> enum bootstage_id id;
> @@ -39,6 +39,7 @@ struct bootstage_record {
> struct bootstage_data {
> uint rec_count;
> uint next_id;
> + ulong start_time_us;
> struct bootstage_record record[RECORD_COUNT];
> };
>
> @@ -132,7 +133,7 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name,
> if (!rec) {
> if (data->rec_count < RECORD_COUNT) {
> rec = &data->record[data->rec_count++];
> - rec->time_us = mark;
> + rec->time_us = mark - data->start_time_us;
> rec->name = name;
> rec->flags = flags;
> rec->id = id;
> @@ -150,7 +151,7 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name,
> ulong bootstage_error_name(enum bootstage_id id, const char *name)
> {
> return bootstage_add_record(id, name, BOOTSTAGEF_ERROR,
> - timer_get_boot_us());
> + timer_get_us());
> }
>
> ulong bootstage_mark_name(enum bootstage_id id, const char *name)
> @@ -160,7 +161,7 @@ ulong bootstage_mark_name(enum bootstage_id id, const char *name)
> if (id == BOOTSTAGE_ID_ALLOC)
> flags = BOOTSTAGEF_ALLOC;
>
> - return bootstage_add_record(id, name, flags, timer_get_boot_us());
> + return bootstage_add_record(id, name, flags, timer_get_us());
> }
>
> ulong bootstage_mark_code(const char *file, const char *func, int linenum)
> @@ -190,11 +191,11 @@ ulong bootstage_mark_code(const char *file, const char *func, int linenum)
> return bootstage_mark_name(BOOTSTAGE_ID_ALLOC, str);
> }
>
> -uint32_t bootstage_start(enum bootstage_id id, const char *name)
> +ulong bootstage_start(enum bootstage_id id, const char *name)
> {
> struct bootstage_data *data = gd->bootstage;
> struct bootstage_record *rec = ensure_id(data, id);
> - ulong start_us = timer_get_boot_us();
> + ulong start_us = timer_get_us();
>
> if (rec) {
> rec->start_us = start_us;
> @@ -204,15 +205,15 @@ uint32_t bootstage_start(enum bootstage_id id, const char *name)
> return start_us;
> }
>
> -uint32_t bootstage_accum(enum bootstage_id id)
> +ulong bootstage_accum(enum bootstage_id id)
> {
> struct bootstage_data *data = gd->bootstage;
> struct bootstage_record *rec = ensure_id(data, id);
> - uint32_t duration;
> + ulong duration;
>
> if (!rec)
> return 0;
> - duration = (uint32_t)timer_get_boot_us() - rec->start_us;
> + duration = timer_get_us() - rec->start_us;
> rec->time_us += duration;
>
> return duration;
> @@ -239,11 +240,11 @@ static const char *get_record_name(char *buf, int len,
> return buf;
> }
>
> -static uint32_t print_time_record(struct bootstage_record *rec, uint32_t prev)
> +static ulong print_time_record(struct bootstage_record *rec, ulong prev)
> {
> char buf[20];
>
> - if (prev == -1U) {
> + if (prev == -1) {
> printf("%11s", "");
> print_grouped_ull(rec->time_us, BOOTSTAGE_DIGITS);
> } else {
> @@ -331,7 +332,7 @@ void bootstage_report(void)
> {
> struct bootstage_data *data = gd->bootstage;
> struct bootstage_record *rec = data->record;
> - uint32_t prev;
> + ulong prev;
> int i;
>
> printf("Timer summary in microseconds (%d records):\n",
> @@ -530,6 +531,7 @@ int bootstage_init(bool first)
> if (first) {
> data->next_id = BOOTSTAGE_ID_USER;
> bootstage_add_record(BOOTSTAGE_ID_AWAKE, "reset", 0, 0);
> + data->start_time_us = timer_get_us();
> }
>
> return 0;
> diff --git a/include/bootstage.h b/include/bootstage.h
> index 7088d0b875e4..e4516f5ca632 100644
> --- a/include/bootstage.h
> +++ b/include/bootstage.h
> @@ -3,7 +3,7 @@
> * This file implements recording of each stage of the boot process. It is
> * intended to implement timing of each stage, reporting this information
> * to the user and passing it to the OS for logging / further analysis.
> - * Note that it requires timer_get_boot_us() to be defined by the board
> + * Note that it requires CONFIG_TIMER_EARLY to be defined by the board
> *
> * Copyright (c) 2011 The Chromium OS Authors.
> */
> @@ -215,13 +215,6 @@ enum bootstage_id {
> BOOTSTAGE_ID_ALLOC,
> };
>
> -/*
> - * Return the time since boot in microseconds, This is needed for bootstage
> - * and should be defined in CPU- or board-specific code. If undefined then
> - * you will get a link error.
> - */
> -ulong timer_get_boot_us(void);
> -
> #if defined(USE_HOSTCC) || !CONFIG_IS_ENABLED(SHOW_BOOT_PROGRESS)
> #define show_boot_progress(val) do {} while (0)
> #else
> @@ -313,7 +306,7 @@ ulong bootstage_mark_code(const char *file, const char *func,
> * @param name Textual name to display for this id in the report (maybe NULL)
> * Return: start timestamp in microseconds
> */
> -uint32_t bootstage_start(enum bootstage_id id, const char *name);
> +ulong bootstage_start(enum bootstage_id id, const char *name);
>
> /**
> * Mark the end of a bootstage activity
> @@ -327,7 +320,7 @@ uint32_t bootstage_start(enum bootstage_id id, const char *name);
> * less the start time recorded in the last bootstage_start() call
> * with this id.
> */
> -uint32_t bootstage_accum(enum bootstage_id id);
> +ulong bootstage_accum(enum bootstage_id id);
>
> /* Print a report about boot time */
> void bootstage_report(void);
> @@ -418,12 +411,12 @@ static inline ulong bootstage_mark_code(const char *file, const char *func,
> return 0;
> }
>
> -static inline uint32_t bootstage_start(enum bootstage_id id, const char *name)
> +static inline ulong bootstage_start(enum bootstage_id id, const char *name)
> {
> return 0;
> }
>
> -static inline uint32_t bootstage_accum(enum bootstage_id id)
> +static inline ulong bootstage_accum(enum bootstage_id id)
> {
> return 0;
> }
> --
> 2.37.3
>
Just heads up this has been identified as a bad commit when I bisect
it for zynqmp_r5 configuration.
I will dig into it to find out what exactly is causing the issue. But
initf_dm() is failing now.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
More information about the U-Boot
mailing list