| Message ID | 1b84ad9ac47894ff2591caffd702dc40978dcd37.1660294474.git.michal.simek@amd.com |
|---|---|
| State | Accepted |
| Commit | 55bc22760c362bba478d449ea7ba59667c4eefe4 |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [v2] bootstage: Show func name for bootstage_mark/error | expand |
Hi Michal, On Fri, 12 Aug 2022 at 02:55, Michal Simek <michal.simek@amd.com> wrote: > > bootstage_mark() and bootstate_error() are not recording any name and in > report it is showing as id=<value>. That's not useful and it is better to > show function name which calls it. > That's why use macros with passing __func__ as recorded name for bootstage. > > Origin report looks like this: > ZynqMP> bootstage report > Timer summary in microseconds (10 records): > Mark Elapsed Stage > 0 0 reset > 2,482,383 2,482,383 board_init_f > 4,278,821 1,796,438 board_init_r > 4,825,331 546,510 id=64 > 4,858,409 33,078 id=65 > 4,862,382 3,973 main_loop > 4,921,713 59,331 usb_start > 9,345,345 4,423,632 id=175 > > When this patch is applied. > ZynqMP> bootstage report > Timer summary in microseconds (31 records): > Mark Elapsed Stage > 0 0 reset > 2,465,624 2,465,624 board_init_f > 4,278,628 1,813,004 board_init_r > 4,825,139 546,511 eth_common_init > 4,858,228 33,089 eth_initialize > 4,862,201 3,973 main_loop > 4,921,530 59,329 usb_start > 8,885,334 3,963,804 cli_loop > > Signed-off-by: Michal Simek <michal.simek@amd.com> > --- > > Changes in v2: > - Add kernel-doc description > > common/bootstage.c | 10 ++-------- > include/bootstage.h | 21 ++++++++++++++++++--- > 2 files changed, 20 insertions(+), 11 deletions(-) I can see what is going on here after applying it. Reviewed-by: Simon Glass <sjg@chromium.org> But I think it would be better as inline functions rather than #define > > diff --git a/common/bootstage.c b/common/bootstage.c > index 0fd33be97eae..326c40f1561f 100644 > --- a/common/bootstage.c > +++ b/common/bootstage.c > @@ -147,15 +147,9 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, > return mark; > } > > - > -ulong bootstage_mark(enum bootstage_id id) > -{ > - return bootstage_add_record(id, NULL, 0, timer_get_boot_us()); > -} > - > -ulong bootstage_error(enum bootstage_id id) > +ulong bootstage_error_name(enum bootstage_id id, const char *name) > { > - return bootstage_add_record(id, NULL, BOOTSTAGEF_ERROR, > + return bootstage_add_record(id, name, BOOTSTAGEF_ERROR, > timer_get_boot_us()); > } > > diff --git a/include/bootstage.h b/include/bootstage.h > index bca9438418f5..7088d0b875e4 100644 > --- a/include/bootstage.h > +++ b/include/bootstage.h > @@ -268,12 +268,27 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, > /** > * Mark a time stamp for the current boot stage. > */ > -ulong bootstage_mark(enum bootstage_id id); > - > -ulong bootstage_error(enum bootstage_id id); > +#define bootstage_mark(id) bootstage_mark_name(id, __func__) > +#define bootstage_error(id) bootstage_error_name(id, __func__) > > +/** > + * bootstage_mark_name - record bootstage with passing id and name > + * @id: Bootstage id to record this timestamp against > + * @name: Textual name to display for this id in the report > + * > + * Return: recorded time stamp > + */ > ulong bootstage_mark_name(enum bootstage_id id, const char *name); > > +/** > + * bootstage_error_name - record bootstage error with passing id and name > + * @id: Bootstage id to record this timestamp against > + * @name: Textual name to display for this id in the report > + * > + * Return: recorded time stamp > + */ > +ulong bootstage_error_name(enum bootstage_id id, const char *name); > + > /** > * Mark a time stamp in the given function and line number > * > -- > 2.36.1 > Regards, Simon
so 13. 8. 2022 v 4:21 odesílatel Simon Glass <sjg@chromium.org> napsal: > > Hi Michal, > > On Fri, 12 Aug 2022 at 02:55, Michal Simek <michal.simek@amd.com> wrote: > > > > bootstage_mark() and bootstate_error() are not recording any name and in > > report it is showing as id=<value>. That's not useful and it is better to > > show function name which calls it. > > That's why use macros with passing __func__ as recorded name for bootstage. > > > > Origin report looks like this: > > ZynqMP> bootstage report > > Timer summary in microseconds (10 records): > > Mark Elapsed Stage > > 0 0 reset > > 2,482,383 2,482,383 board_init_f > > 4,278,821 1,796,438 board_init_r > > 4,825,331 546,510 id=64 > > 4,858,409 33,078 id=65 > > 4,862,382 3,973 main_loop > > 4,921,713 59,331 usb_start > > 9,345,345 4,423,632 id=175 > > > > When this patch is applied. > > ZynqMP> bootstage report > > Timer summary in microseconds (31 records): > > Mark Elapsed Stage > > 0 0 reset > > 2,465,624 2,465,624 board_init_f > > 4,278,628 1,813,004 board_init_r > > 4,825,139 546,511 eth_common_init > > 4,858,228 33,089 eth_initialize > > 4,862,201 3,973 main_loop > > 4,921,530 59,329 usb_start > > 8,885,334 3,963,804 cli_loop > > > > Signed-off-by: Michal Simek <michal.simek@amd.com> > > --- > > > > Changes in v2: > > - Add kernel-doc description > > > > common/bootstage.c | 10 ++-------- > > include/bootstage.h | 21 ++++++++++++++++++--- > > 2 files changed, 20 insertions(+), 11 deletions(-) > > I can see what is going on here after applying it. > > Reviewed-by: Simon Glass <sjg@chromium.org> > > But I think it would be better as inline functions rather than #define I was trying static inline functions but __func__ is using bootstage names instead of the function which is calling them. Maybe there is another function specifier which can be used to do the same job as macros. Thanks, Michal
Hi Michal, On Fri, 12 Aug 2022 at 23:39, Michal Simek <monstr@monstr.eu> wrote: > > so 13. 8. 2022 v 4:21 odesílatel Simon Glass <sjg@chromium.org> napsal: > > > > Hi Michal, > > > > On Fri, 12 Aug 2022 at 02:55, Michal Simek <michal.simek@amd.com> wrote: > > > > > > bootstage_mark() and bootstate_error() are not recording any name and in > > > report it is showing as id=<value>. That's not useful and it is better to > > > show function name which calls it. > > > That's why use macros with passing __func__ as recorded name for bootstage. > > > > > > Origin report looks like this: > > > ZynqMP> bootstage report > > > Timer summary in microseconds (10 records): > > > Mark Elapsed Stage > > > 0 0 reset > > > 2,482,383 2,482,383 board_init_f > > > 4,278,821 1,796,438 board_init_r > > > 4,825,331 546,510 id=64 > > > 4,858,409 33,078 id=65 > > > 4,862,382 3,973 main_loop > > > 4,921,713 59,331 usb_start > > > 9,345,345 4,423,632 id=175 > > > > > > When this patch is applied. > > > ZynqMP> bootstage report > > > Timer summary in microseconds (31 records): > > > Mark Elapsed Stage > > > 0 0 reset > > > 2,465,624 2,465,624 board_init_f > > > 4,278,628 1,813,004 board_init_r > > > 4,825,139 546,511 eth_common_init > > > 4,858,228 33,089 eth_initialize > > > 4,862,201 3,973 main_loop > > > 4,921,530 59,329 usb_start > > > 8,885,334 3,963,804 cli_loop > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com> > > > --- > > > > > > Changes in v2: > > > - Add kernel-doc description > > > > > > common/bootstage.c | 10 ++-------- > > > include/bootstage.h | 21 ++++++++++++++++++--- > > > 2 files changed, 20 insertions(+), 11 deletions(-) > > > > I can see what is going on here after applying it. > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > But I think it would be better as inline functions rather than #define > > I was trying static inline functions but __func__ is using bootstage > names instead of the function > which is calling them. Maybe there is another function specifier which > can be used to do the same job > as macros. Not that I know of...I was thinking of using a #define to call a function with a similar name (e.,g. with a trailing underscore). But I think I'm just being too precious. Let's leave it as it is and see how things go. Regards, Simon
Hi Michal, On Fri, 12 Aug 2022 at 23:39, Michal Simek <monstr@monstr.eu> wrote: > > so 13. 8. 2022 v 4:21 odesílatel Simon Glass <sjg@chromium.org> napsal: > > > > Hi Michal, > > > > On Fri, 12 Aug 2022 at 02:55, Michal Simek <michal.simek@amd.com> wrote: > > > > > > bootstage_mark() and bootstate_error() are not recording any name and in > > > report it is showing as id=<value>. That's not useful and it is better to > > > show function name which calls it. > > > That's why use macros with passing __func__ as recorded name for bootstage. > > > > > > Origin report looks like this: > > > ZynqMP> bootstage report > > > Timer summary in microseconds (10 records): > > > Mark Elapsed Stage > > > 0 0 reset > > > 2,482,383 2,482,383 board_init_f > > > 4,278,821 1,796,438 board_init_r > > > 4,825,331 546,510 id=64 > > > 4,858,409 33,078 id=65 > > > 4,862,382 3,973 main_loop > > > 4,921,713 59,331 usb_start > > > 9,345,345 4,423,632 id=175 > > > > > > When this patch is applied. > > > ZynqMP> bootstage report > > > Timer summary in microseconds (31 records): > > > Mark Elapsed Stage > > > 0 0 reset > > > 2,465,624 2,465,624 board_init_f > > > 4,278,628 1,813,004 board_init_r > > > 4,825,139 546,511 eth_common_init > > > 4,858,228 33,089 eth_initialize > > > 4,862,201 3,973 main_loop > > > 4,921,530 59,329 usb_start > > > 8,885,334 3,963,804 cli_loop > > > > > > Signed-off-by: Michal Simek <michal.simek@amd.com> > > > --- > > > > > > Changes in v2: > > > - Add kernel-doc description > > > > > > common/bootstage.c | 10 ++-------- > > > include/bootstage.h | 21 ++++++++++++++++++--- > > > 2 files changed, 20 insertions(+), 11 deletions(-) > > > > I can see what is going on here after applying it. > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > But I think it would be better as inline functions rather than #define > > I was trying static inline functions but __func__ is using bootstage > names instead of the function > which is calling them. Maybe there is another function specifier which > can be used to do the same job > as macros. Not that I know of...I was thinking of using a #define to call a function with a similar name (e.,g. with a trailing underscore). But I think I'm just being too precious. Let's leave it as it is and see how things go. Regards, Simon Applied to u-boot-dm, thanks!
diff --git a/common/bootstage.c b/common/bootstage.c index 0fd33be97eae..326c40f1561f 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -147,15 +147,9 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, return mark; } - -ulong bootstage_mark(enum bootstage_id id) -{ - return bootstage_add_record(id, NULL, 0, timer_get_boot_us()); -} - -ulong bootstage_error(enum bootstage_id id) +ulong bootstage_error_name(enum bootstage_id id, const char *name) { - return bootstage_add_record(id, NULL, BOOTSTAGEF_ERROR, + return bootstage_add_record(id, name, BOOTSTAGEF_ERROR, timer_get_boot_us()); } diff --git a/include/bootstage.h b/include/bootstage.h index bca9438418f5..7088d0b875e4 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -268,12 +268,27 @@ ulong bootstage_add_record(enum bootstage_id id, const char *name, /** * Mark a time stamp for the current boot stage. */ -ulong bootstage_mark(enum bootstage_id id); - -ulong bootstage_error(enum bootstage_id id); +#define bootstage_mark(id) bootstage_mark_name(id, __func__) +#define bootstage_error(id) bootstage_error_name(id, __func__) +/** + * bootstage_mark_name - record bootstage with passing id and name + * @id: Bootstage id to record this timestamp against + * @name: Textual name to display for this id in the report + * + * Return: recorded time stamp + */ ulong bootstage_mark_name(enum bootstage_id id, const char *name); +/** + * bootstage_error_name - record bootstage error with passing id and name + * @id: Bootstage id to record this timestamp against + * @name: Textual name to display for this id in the report + * + * Return: recorded time stamp + */ +ulong bootstage_error_name(enum bootstage_id id, const char *name); + /** * Mark a time stamp in the given function and line number *
bootstage_mark() and bootstate_error() are not recording any name and in report it is showing as id=<value>. That's not useful and it is better to show function name which calls it. That's why use macros with passing __func__ as recorded name for bootstage. Origin report looks like this: ZynqMP> bootstage report Timer summary in microseconds (10 records): Mark Elapsed Stage 0 0 reset 2,482,383 2,482,383 board_init_f 4,278,821 1,796,438 board_init_r 4,825,331 546,510 id=64 4,858,409 33,078 id=65 4,862,382 3,973 main_loop 4,921,713 59,331 usb_start 9,345,345 4,423,632 id=175 When this patch is applied. ZynqMP> bootstage report Timer summary in microseconds (31 records): Mark Elapsed Stage 0 0 reset 2,465,624 2,465,624 board_init_f 4,278,628 1,813,004 board_init_r 4,825,139 546,511 eth_common_init 4,858,228 33,089 eth_initialize 4,862,201 3,973 main_loop 4,921,530 59,329 usb_start 8,885,334 3,963,804 cli_loop Signed-off-by: Michal Simek <michal.simek@amd.com> --- Changes in v2: - Add kernel-doc description common/bootstage.c | 10 ++-------- include/bootstage.h | 21 ++++++++++++++++++--- 2 files changed, 20 insertions(+), 11 deletions(-)