[U-Boot] [PATCH 0/8] Adding boottime support

In this patch-set we're attempting to add boottime measurement support to u-boot. A patch-set has already hit the kernel MLs which intends to solve the other half of the puzzle.
This new tool allows an engineer to apply tags into key areas around the bootloader, which are then passed to the Linux kernel for processing and statistics analysis of full (bootloader + kernel) device booting.
arch/arm/cpu/armv7/u8500/timer.c | 7 +- arch/arm/include/asm/setup.h | 18 +++++ arch/arm/lib/board.c | 3 + arch/arm/lib/bootm.c | 48 ++++++++++++- common/Makefile | 1 + common/boottime.c | 146 ++++++++++++++++++++++++++++++++++++++ common/main.c | 5 ++ include/boottime.h | 86 ++++++++++++++++++++++ include/common.h | 1 + include/configs/snowball.h | 2 + include/configs/u8500_href.h | 2 + 11 files changed, 316 insertions(+), 3 deletions(-)

If we attempt to take a 32bit timer value and multiply it by a significant number, the core will not be able to handle it. This gives the illusion that the timer is rolling over, when in fact this is not the case. If we ensure the division in this instance is carried out before the multiplication, the issue vanishes.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/cpu/armv7/u8500/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/u8500/timer.c b/arch/arm/cpu/armv7/u8500/timer.c index 79aad99..40326d8 100644 --- a/arch/arm/cpu/armv7/u8500/timer.c +++ b/arch/arm/cpu/armv7/u8500/timer.c @@ -70,7 +70,7 @@ struct u8500_mtu { * The MTU is clocked at 133 MHz by default. (V1 and later) */ #define TIMER_CLOCK (133 * 1000 * 1000 / 16) -#define COUNT_TO_USEC(x) ((x) * 16 / 133) +#define COUNT_TO_USEC(x) ((x) / 133 * 16) #define USEC_TO_COUNT(x) ((x) * 133 / 16) #define TICKS_PER_HZ (TIMER_CLOCK / CONFIG_SYS_HZ) #define TICKS_TO_HZ(x) ((x) / TICKS_PER_HZ)

Dear Lee Jones,
In message 1353422034-28107-2-git-send-email-lee.jones@linaro.org you wrote:
If we attempt to take a 32bit timer value and multiply it by a significant number, the core will not be able to handle it. This gives the illusion that the timer is rolling over, when in fact this is not the case. If we ensure the division in this instance is carried out before the multiplication, the issue vanishes.
Are you sure this is a good idea?
--- a/arch/arm/cpu/armv7/u8500/timer.c +++ b/arch/arm/cpu/armv7/u8500/timer.c @@ -70,7 +70,7 @@ struct u8500_mtu {
- The MTU is clocked at 133 MHz by default. (V1 and later)
*/ #define TIMER_CLOCK (133 * 1000 * 1000 / 16) -#define COUNT_TO_USEC(x) ((x) * 16 / 133) +#define COUNT_TO_USEC(x) ((x) / 133 * 16)
Before the change, the result is useful for all values of x in the interval from 0 through UINT_MAX/16 = 268435455 = 268 seconds, i. e. for all values that make sense to be measured in microseconds.
After the change, the result is changed to the worse for all low values of X, especially for the ranges from 16 through 132.
You lose accuracy here, and win nothing.
This makes no sense to me.
If you need a bigger number range, then use proper arithmetics. It';s available, just use it.
Best regards,
Wolfgang Denk

If we attempt to take a 32bit timer value and multiply it by a significant number, the core will not be able to handle it. This gives the illusion that the timer is rolling over, when in fact this is not the case. If we ensure the division in this instance is carried out before the multiplication, the issue vanishes.
Are you sure this is a good idea?
Not now I'm not. :)
--- a/arch/arm/cpu/armv7/u8500/timer.c +++ b/arch/arm/cpu/armv7/u8500/timer.c @@ -70,7 +70,7 @@ struct u8500_mtu {
- The MTU is clocked at 133 MHz by default. (V1 and later)
*/ #define TIMER_CLOCK (133 * 1000 * 1000 / 16) -#define COUNT_TO_USEC(x) ((x) * 16 / 133) +#define COUNT_TO_USEC(x) ((x) / 133 * 16)
Before the change, the result is useful for all values of x in the interval from 0 through UINT_MAX/16 = 268435455 = 268 seconds, i. e. for all values that make sense to be measured in microseconds.
We're actually discussing this elsewhere. I don't have permission to paste the other side of the conversation, but I can show you my calculations:
The original implementation is fine until we reach 32.28 seconds, which as you predicted is 0x1000_0000:
0x10000000 * PRESCALER) / (CLOCK_SPEED_133_MHZ) (268435456 * 16 ) / (133 * 1000 * 1000) == 32.28 seconds
If we spend >30 seconds at the u-boot commandline, which is not unreasonable by any stretch, then the kernel assumes responsibility for the remaining of the time spent at the prompt, which is obviously not acceptable for this usecase.
I doubt x will never be < 133, as that will be 16 micro-seconds post timer-start or role-over. Do we really need that degree of resolution?
Am assuming by your response that I'm wrong somewhere, and the resolution loss will be >16us?
After the change, the result is changed to the worse for all low values of X, especially for the ranges from 16 through 132.
You lose accuracy here, and win nothing.
This makes no sense to me.
If you need a bigger number range, then use proper arithmetics. It';s available, just use it.
Would you be able to lend a hand here, as I'm no mathematician?
What are the correct arithmetics?
Kind regards, Lee

Dear Lee Jones,
In message 20121121100228.GJ28265@gmail.com you wrote:
-#define COUNT_TO_USEC(x) ((x) * 16 / 133) +#define COUNT_TO_USEC(x) ((x) / 133 * 16)
Before the change, the result is useful for all values of x in the interval from 0 through UINT_MAX/16 = 268435455 = 268 seconds, i. e. for all values that make sense to be measured in microseconds.
We're actually discussing this elsewhere. I don't have permission to paste the other side of the conversation, but I can show you my calculations:
The original implementation is fine until we reach 32.28 seconds, which as you predicted is 0x1000_0000:
0x10000000 * PRESCALER) / (CLOCK_SPEED_133_MHZ) (268435456 * 16 ) / (133 * 1000 * 1000) == 32.28 seconds
If we spend >30 seconds at the u-boot commandline, which is not unreasonable by any stretch, then the kernel assumes responsibility for the remaining of the time spent at the prompt, which is obviously not acceptable for this usecase.
This makes no sense to me. An overflow will not happen before UINT_MAX/16 = 268435455 = 268 seconds.
Anyway. If you have overflof problems, then use proper arithmetics.
If you need a bigger number range, then use proper arithmetics. It';s available, just use it.
Would you be able to lend a hand here, as I'm no mathematician?
What are the correct arithmetics?
There are routines like do_div() or lldiv() etc. that can be used when 32 bit arithmetics is really not good enough.
However, I fail to see why that should even be needed here.
Best regards,
Wolfgang Denk

-#define COUNT_TO_USEC(x) ((x) * 16 / 133) +#define COUNT_TO_USEC(x) ((x) / 133 * 16)
Before the change, the result is useful for all values of x in the interval from 0 through UINT_MAX/16 = 268435455 = 268 seconds, i. e. for all values that make sense to be measured in microseconds.
We're actually discussing this elsewhere. I don't have permission to paste the other side of the conversation, but I can show you my calculations:
The original implementation is fine until we reach 32.28 seconds, which as you predicted is 0x1000_0000:
0x10000000 * PRESCALER) / (CLOCK_SPEED_133_MHZ) (268435456 * 16 ) / (133 * 1000 * 1000) == 32.28 seconds
If we spend >30 seconds at the u-boot commandline, which is not unreasonable by any stretch, then the kernel assumes responsibility for the remaining of the time spent at the prompt, which is obviously not acceptable for this usecase.
This makes no sense to me. An overflow will not happen before UINT_MAX/16 = 268435455 = 268 seconds.
Right, but that's the timer.
The issue here is not that the timer is overflowing, it's the arithmetics that takes place once the timer value is obtained. If a value of >0x10000000 is read, then we carry out the arithmetic above, then other registers overflow.
Anyway. If you have overflof problems, then use proper arithmetics.
If you need a bigger number range, then use proper arithmetics. It';s available, just use it.
Would you be able to lend a hand here, as I'm no mathematician?
What are the correct arithmetics?
There are routines like do_div() or lldiv() etc. that can be used when 32 bit arithmetics is really not good enough.
Ah ha, thanks.
However, I fail to see why that should even be needed here.
As above.

Provide support for microsecond level timer support.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/cpu/armv7/u8500/timer.c | 5 +++++ include/common.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/arch/arm/cpu/armv7/u8500/timer.c b/arch/arm/cpu/armv7/u8500/timer.c index 40326d8..d9a6a2d 100644 --- a/arch/arm/cpu/armv7/u8500/timer.c +++ b/arch/arm/cpu/armv7/u8500/timer.c @@ -129,6 +129,11 @@ ulong get_timer(ulong base) return get_timer_masked() - base; }
+u64 get_timer_us(void) +{ + return COUNT_TO_USEC(READ_TIMER()); +} + /* * Emulation of Power architecture long long timebase. * diff --git a/include/common.h b/include/common.h index 5e3c5ee..5d24add 100644 --- a/include/common.h +++ b/include/common.h @@ -690,6 +690,7 @@ void irq_install_handler(int, interrupt_handler_t *, void *); void irq_free_handler (int); void reset_timer (void); ulong get_timer (ulong base); +ulong get_timer_us (void); void enable_interrupts (void); int disable_interrupts (void);

Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org --- common/Makefile | 1 + common/boottime.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++ include/boottime.h | 86 +++++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+) create mode 100644 common/boottime.c create mode 100644 include/boottime.h
diff --git a/common/Makefile b/common/Makefile index 9e43322..546acbf 100644 --- a/common/Makefile +++ b/common/Makefile @@ -192,6 +192,7 @@ COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o COBJS-$(CONFIG_UPDATE_TFTP) += update.o COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o COBJS-$(CONFIG_CMD_DFU) += cmd_dfu.o +COBJS-$(CONFIG_BOOTTIME) += boottime.o endif
ifdef CONFIG_SPL_BUILD diff --git a/common/boottime.c b/common/boottime.c new file mode 100644 index 0000000..87be467 --- /dev/null +++ b/common/boottime.c @@ -0,0 +1,146 @@ +/* + * Copyright (C) ST-Ericsson SA 2009-2010 + * Jonas Aaberg jonas.aberg@stericsson.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <common.h> +#include <boottime.h> +#include <asm/byteorder.h> +#include <asm/setup.h> + +#ifdef CONFIG_BOOTTIME + +static struct tag_boottime boottime = { + .idle = 0, + .total = 0, +}; + +void boottime_tag(char *name) +{ + if (boottime.num == BOOTTIME_MAX) { + printf("boottime: out of entries!\n"); + return; + } + + strncpy((char *)boottime.entry[boottime.num].name, + name, + BOOTTIME_MAX_NAME_LEN); + boottime.entry[boottime.num].name[BOOTTIME_MAX_NAME_LEN - 1] = '\0'; + boottime.entry[boottime.num].time = get_timer_us(); + + boottime.num++; +} + +struct boottime_entry *boottime_get_entry(unsigned int i) +{ + if (i >= boottime.num) + return NULL; + else + return &boottime.entry[i]; +} + +void boottime_idle_add(unsigned long time) +{ + boottime.idle += time; +} + +unsigned long boottime_idle_done(void) +{ + return get_timer_us(); +} + +unsigned long boottime_idle_get(void) +{ + return boottime.idle; +} + +static int do_boottime(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + unsigned int i; + struct boottime_entry *entry; + + for (i = 0; i < BOOTTIME_MAX; i++) { + entry = boottime_get_entry(i); + if (entry == NULL) + break; + printf("%s: started at %lu ms\n", entry->name, + (unsigned long)entry->time / 1000); + } + printf("idle: %d%% (%d ms)\n", + 100 * (int)boottime_idle_get() / (int)get_timer_us(), + (int)boottime_idle_get() / 1000); + return 0; +} + +static int do_boottime_tag (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + + if (argc < 2) { + cmd_usage(cmdtp); + return 1; + } + boottime_tag(argv[1]); + + return 0; +} + +U_BOOT_CMD( + boottime, 1, 1, do_boottime, + "print boottime info", + "" + " - print boottime tags\n" +); + +U_BOOT_CMD( + boottime_tag, 2, 1, do_boottime_tag, + "boottime tag 'name'", + "" + " - Add a boottime tag at the current time\n" +); + +#else +/* + * Dummy functions when boottime is not enabled. + */ +static int do_boottime_tag (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + return 0; +} + +void boottime_tag(char *name) +{ + +} + +void boottime_idle_add(unsigned long time) +{ + +} + +U_BOOT_CMD( + boottime_tag, 2, 1, do_boottime_tag, + "boottime tag 'name'", + "" + " - NOT ENABLED: Add CONFIG_BOOTIME to your boards configuration" + " file to enable boottime.\n" +); + +#endif + + + diff --git a/include/boottime.h b/include/boottime.h new file mode 100644 index 0000000..58ea314 --- /dev/null +++ b/include/boottime.h @@ -0,0 +1,86 @@ +/* + * Copyright (C) ST-Ericsson SA 2009-2010 + * Jonas Aaberg jonas.aberg@stericsson.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#ifndef BOOTTIME_H +#define BOOTTIME_H + +#define BOOTTIME_MAX_NAME_LEN 64 + +struct boottime_entry { + u32 time; /* in us */ + u8 name[BOOTTIME_MAX_NAME_LEN]; +}; + +#ifdef CONFIG_BOOTTIME + +/** + * boottime_tag() + * Add a sample point with a name now. Shall be called before function "name" + * is executed. + * @name: Sample point name. + */ +void boottime_tag(char *name); + +/** + * boottime_get_entry() + * + * Loads a boottime measure point information. + * @i: boottime measurement point entry. + * + * Returns a boottime entry. NULL, if not existing. + */ +struct boottime_entry *boottime_get_entry(unsigned int i); + +/** + * boottime_idle_get() + * + * Returns the amount of time in us that has been spent idling. + */ +unsigned long boottime_idle_get(void); + +/** + * boottime_idle_done() + * + * Returns the total time since start in us. + */ +unsigned long boottime_idle_done(void); + +/** + * boottime_idle_add() + * + * This function shall be added to all delay() functions. + * The delay time input to delay() shall be provided to this + * function as well. It is used to calculate average load + * during boot. + * @time: time in us. + */ +void boottime_idle_add(unsigned long time); + +#else /* CONFIG_BOOTTIME */ + +static inline void boottime_tag(char *name) { return; } +static inline struct boottime_entry *boottime_get_entry(unsigned int i) { return NULL; } +static inline unsigned long boottime_idle_get(void) { return 0; } +static inline unsigned long boottime_idle_done(void) { return 0; } +static inline void boottime_idle_add(unsigned long time) { return; } + +#endif /* CONFIG_BOOTTIME */ + +#endif /* BOOTTIME_H */

Dear Lee Jones,
In message 1353422034-28107-4-git-send-email-lee.jones@linaro.org you wrote:
Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Aren't we converting more and more systems to use the device treee to pass information to the kenrel, with the result that ATAGS are kind of becoming extinct?
And forcing somthing upon a mechanism that was designed for a completely different purpose, where you see right from the first glance that it does not math easily?
This makes no sense to me. Why don't you use standard mechaisms, like a shared log buffer, and simply create time-stamped entries into the kernel boot log?
The advantages should be obvious: we will need no extra kernel modification, we do not depend on ATAGS, and we are automatically architecture-independent.
Sorry, I tend to NAK this.
Best regards,
Wolfgang Denk

Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Aren't we converting more and more systems to use the device treee to pass information to the kenrel, with the result that ATAGS are kind of becoming extinct?
Yes, I intend to extend this functionality into Device Tree. That way it will be architecture and OS independent.
And forcing something upon a mechanism that was designed for a completely different purpose, where you see right from the first glance that it does not math easily?
Not entirely sure what you mean here. This mechanism works perfectly with ATAGs.
This makes no sense to me. Why don't you use standard mechanisms, like a shared log buffer, and simply create time-stamped entries into the kernel boot log?
The advantages should be obvious: we will need no extra kernel modification, we do not depend on ATAGS, and we are automatically architecture-independent.
Wouldn't this clog up the kernel's log buffer? I'm sure no user wants to see reams of otherwise useless logging scrawled throughout their bootlog. We'd also have a write a text parser to obtain the information for processing. It would be easier to either pass in a struct, as we do with the ATAG mechanism, or though Device Tree as previously discussed.

Dear Lee Jones,
In message 20121121095045.GI28265@gmail.com you wrote:
Yes, I intend to extend this functionality into Device Tree. That way it will be architecture and OS independent.
And forcing something upon a mechanism that was designed for a completely different purpose, where you see right from the first glance that it does not math easily?
Not entirely sure what you mean here. This mechanism works perfectly with ATAGs.
Neither ATAGS not the device tree are intended nor designed for passing logfile information. Yes, you can use them like that, and it will actually work. You can also drive a nail in using a microscope as hammer.
The advantages should be obvious: we will need no extra kernel modification, we do not depend on ATAGS, and we are automatically architecture-independent.
Wouldn't this clog up the kernel's log buffer? I'm sure no user wants to see reams of otherwise useless logging scrawled throughout their bootlog. We'd also have a write a text parser to obtain the information for processing. It would be easier to either pass in a struct, as we do with the ATAG mechanism, or though Device Tree as previously discussed.
I think these are pretty poor arguments. There are standard methods (like log levels) to provide adequate filtering of which messages are passed on to a user. An there exists a plethora of tools for automatic filtering and post-processing of syslog messages. You will need to write _less_ code than with your homebrew implementation.
Best regards,
Wolfgang Denk

Yes, I intend to extend this functionality into Device Tree. That way it will be architecture and OS independent.
And forcing something upon a mechanism that was designed for a completely different purpose, where you see right from the first glance that it does not math easily?
Not entirely sure what you mean here. This mechanism works perfectly with ATAGs.
Neither ATAGS not the device tree are intended nor designed for passing logfile information. Yes, you can use them like that, and it will actually work.
ATAGs were exactly designed for this type of thing. To pass small data structures though to the kernel. In our case, our trace-points are held in a small data structure. They're not logs.
You can also drive a nail in using a microscope as hammer.
Ah good idea. I have to try this. ;)
The advantages should be obvious: we will need no extra kernel modification, we do not depend on ATAGS, and we are automatically architecture-independent.
Wouldn't this clog up the kernel's log buffer? I'm sure no user wants to see reams of otherwise useless logging scrawled throughout their bootlog. We'd also have a write a text parser to obtain the information for processing. It would be easier to either pass in a struct, as we do with the ATAG mechanism, or though Device Tree as previously discussed.
I think these are pretty poor arguments. There are standard methods (like log levels) to provide adequate filtering of which messages are passed on to a user. An there exists a plethora of tools for automatic filtering and post-processing of syslog messages. You will need to write _less_ code than with your homebrew implementation.
They're not poor augments if the data stored isn't log messages, which these aren't. If anything I would say that ramming them in as textual kernel messages, then parsing the log text using a userspace tool was an abuse of the system. If we create them as data in the bootloader, then pass them to the kernel as data, then process them as data, _that_ would be the correct mechanism.

Dear Lee Jones,
In message 20121121150332.GC28899@gmail.com you wrote:
Neither ATAGS not the device tree are intended nor designed for passing logfile information. Yes, you can use them like that, and it will actually work.
ATAGs were exactly designed for this type of thing. To pass small data structures though to the kernel. In our case, our trace-points are held in a small data structure. They're not logs.
You appear to have a specific definition of log data in mind. It must be different to mine.
Also, you contradict yourself - here you write "pass small data structures", earlier you wrote about "lots of trace-points", which sounds as if the total amount of data would be not exactly small - actually so big that yu are afraid of annoying users with it.
Anyway. This doesn't take us further.
They're not poor augments if the data stored isn't log messages, which these aren't. If anything I would say that ramming them in as textual kernel messages, then parsing the log text using a userspace tool was an abuse of the system. If we create them as data in the bootloader, then pass them to the kernel as data, then process them as data, _that_ would be the correct mechanism.
Well, I could pooint out here a number of pretty basic design decisions made earlier in a number of pretty important and successful software projects, like the fact that a large number of internet protocols are based on plain text implementations. Or how useful it is if you can just to a post-mortem dump of the log buffer and actually _read_ the entries, without need to special tools.
I think I should stop here, though. It appears it makes little sense trying to discuss alternative approaches when you have already fixed your mind about the one and only "correct" way to do this.
To summarize: NAK.
Best regards,
Wolfgang Denk

On Wed, 21 Nov 2012, Wolfgang Denk wrote:
Dear Lee Jones,
In message 20121121150332.GC28899@gmail.com you wrote:
Neither ATAGS not the device tree are intended nor designed for passing logfile information. Yes, you can use them like that, and it will actually work.
ATAGs were exactly designed for this type of thing. To pass small data structures though to the kernel. In our case, our trace-points are held in a small data structure. They're not logs.
You appear to have a specific definition of log data in mind. It must be different to mine.
I would say that a log entry would contain useful descriptive information contained. It is _sometimes_ useful to display a related timestamp too. In our case, the most useful part is the timestamp. I just don't see how riddling the kernel log with timestamps would be useful, or nessersary.
Also, you contradict yourself - here you write "pass small data structures", earlier you wrote about "lots of trace-points", which sounds as if the total amount of data would be not exactly small - actually so big that yu are afraid of annoying users with it.
Right, perhaps I should have been more descriptive. I don't see the bootloader passing lots of these trace points. The current configuration will allow 10; however, the kernel logs one for every cecal. This would quickly fill the kernel log with lots of unwanted bumph. I also would like to store the data for bootloader and kernel in the same manor. Placing the bootloader's trace points in the kernel log, then putting the kernel's ones somewhere else would add unwanted complexity. Especially if you're wanting the user to write an app for parsing. I would prefer it if all of the processing could be done in the kernel and displayed nicely via debugfs.
Anyway. This doesn't take us further.
They're not poor augments if the data stored isn't log messages, which these aren't. If anything I would say that ramming them in as textual kernel messages, then parsing the log text using a userspace tool was an abuse of the system. If we create them as data in the bootloader, then pass them to the kernel as data, then process them as data, _that_ would be the correct mechanism.
Well, I could pooint out here a number of pretty basic design decisions made earlier in a number of pretty important and successful software projects, like the fact that a large number of internet protocols are based on plain text implementations. Or how useful it is if you can just to a post-mortem dump of the log buffer and actually _read_ the entries, without need to special tools.
You can do that in the current implementation though debugfs:
$ cat /sys/kernel/debug/boottime/bootgraph [ 0.023024] calling board_init+0x0/0x0 [ 0.177808] initcall board_init+0x0/0x0 returned 0 after 154 msecs. [ 0.177808] calling main_loop+0x0/0x0 [ 0.179632] initcall main_loop+0x0/0x0 returned 0 after 1 msecs.
It is your suggestion that would require _extra_ bespoke tools for parsing and actually obtaining the information you require. Where as we can just do this:
$ cat /sys/kernel/debug/boottime/summary bootloader: 5484 msecs kernel: 2265 msecs total: 7750 msecs kernel: cpu0 system: 75% idle: 25% iowait: 0% irq: 0% cpu1 system: 2% idle: 97% iowait: 0% irq: 0% bootloader: cpu0 system: 100% idle: 0% iowait: 0% irq: 0%
I think I should stop here, though. It appears it makes little sense trying to discuss alternative approaches when you have already fixed your mind about the one and only "correct" way to do this.
Not at all. I am open to new and different approaches, so long as they are better. I just don't agree that munging a timestamp and id (usually a function name) into a text string and shoving it into the kernel's log buffer is in any way 'better'.
I hope you can at least see my point.

Dear Lee Jones,
In message 20121121172604.GB417@gmail.com you wrote:
You appear to have a specific definition of log data in mind. It must be different to mine.
I would say that a log entry would contain useful descriptive information contained. It is _sometimes_ useful to display a related timestamp too. In our case, the most useful part is the timestamp. I just don't see how riddling the kernel log with timestamps would be useful, or nessersary.
Look here:
... [ 85.005969] it87: Found IT8718F chip at 0x290, revision 5 [ 85.041976] it87: VID is disabled (pins used for GPIO) [ 85.081038] it87: Beeping is supported [ 86.283285] r8169 0000:04:00.0: eth0: link down [ 86.283307] r8169 0000:04:00.0: eth0: link down [ 86.348499] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [ 88.688543] r8169 0000:04:00.0: eth0: link up [ 88.723985] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready [ 89.583337] Bridge firewalling registered [ 89.642103] device virbr0-nic entered promiscuous mode ...
You tell me what is the most interesting information here, the "descriptive information", or the "related timestamp"? In many cases, and for many people, it's not one or the other, but _both_.
But we're repeating ourselfs. I see no new arguments. Let's stop here.
Best regards,
Wolfgang Denk

Hi Lee,
On Tue, Nov 20, 2012 at 6:33 AM, Lee Jones lee.jones@linaro.org wrote:
Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
Did you take a look at bootstage, which seems at least on the surface to provide a similar mechanism? This passes the information to the kernel through a device tree, or worse case a 'stash area'.
Regards, Simon

Hi Simon,
On Tue, Nov 20, 2012 at 6:33 AM, Lee Jones lee.jones@linaro.org wrote:
Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
Did you take a look at bootstage, which seems at least on the surface to provide a similar mechanism? This passes the information to the kernel through a device tree, or worse case a 'stash area'.
I didn't see this before.
I don't see the kernel counterpart, did it make it upstream?
Wolfgang, didn't you know about bootstage? This could have saved us a great deal of time.
Kind regards, Lee

Hi Lee,
On Mon, Nov 26, 2012 at 1:00 AM, Lee Jones lee.jones@linaro.org wrote:
Hi Simon,
On Tue, Nov 20, 2012 at 6:33 AM, Lee Jones lee.jones@linaro.org wrote:
Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
Did you take a look at bootstage, which seems at least on the surface to provide a similar mechanism? This passes the information to the kernel through a device tree, or worse case a 'stash area'.
I didn't see this before.
I don't see the kernel counterpart, did it make it upstream?
It was sent upstream, just for a feeler, but before U-Boot support was mainlined and before we had a way to deal with the non-fdt case. That is now implemented and in mainline, although it has not yet gone out in a release (should be Jan 2013). So I was planning to address that again in the kernel at some point.
Wolfgang, didn't you know about bootstage? This could have saved us a great deal of time.
Kind regards, Lee
-- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Regards, Simon

Hi Simon,
On Tue, Nov 20, 2012 at 6:33 AM, Lee Jones lee.jones@linaro.org wrote:
Boottime is a tool which can be used for full system booting time measurement. Bootloader boot time is passed to the kernel component though ATAGS. The kernel-side driver then uses this information to provide full system boot time diagnostics though debugfs.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
Did you take a look at bootstage, which seems at least on the surface to provide a similar mechanism? This passes the information to the kernel through a device tree, or worse case a 'stash area'.
I didn't see this before.
I don't see the kernel counterpart, did it make it upstream?
It was sent upstream, just for a feeler, but before U-Boot support was mainlined and before we had a way to deal with the non-fdt case. That is now implemented and in mainline, although it has not yet gone out in a release (should be Jan 2013). So I was planning to address that again in the kernel at some point.
Well if this does the same job as the boottime implementation, I'll scrap my efforts to upstream it.
By the way, if Wolfgang didn't want these tracepoints in DT, then how was your implementations upstreamed into u-boot?
Kind regards, Lee

Dear Lee Jones,
In message 20121127085548.GC7897@gmail.com you wrote:
By the way, if Wolfgang didn't want these tracepoints in DT, then how was your implementations upstreamed into u-boot?
Because I don;t manage a 100% review coverage over all submitted patches, i. e. it escaped my attention (and I'm sorry for that).
Best regards,
Wolfgang Denk

On Tue, 27 Nov 2012, Wolfgang Denk wrote:
Dear Lee Jones,
In message 20121127085548.GC7897@gmail.com you wrote:
By the way, if Wolfgang didn't want these tracepoints in DT, then how was your implementations upstreamed into u-boot?
Because I don;t manage a 100% review coverage over all submitted patches, i. e. it escaped my attention (and I'm sorry for that).
Ah, I see. Makes sense.
Well I'm pleased someone saw sense in any case. :)

Here we add boottime tags to the start of the main loop and just before the opportunity to break into the u-boot shell. This will provide a more verbose bootgraph when viewed within debugfs.
Signed-off-by: Lee Jones lee.jones@linaro.org --- common/main.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/common/main.c b/common/main.c index 592ce07..84c88e9 100644 --- a/common/main.c +++ b/common/main.c @@ -40,6 +40,7 @@ #include <hush.h> #endif
+#include <boottime.h> #include <post.h> #include <linux/ctype.h> #include <menu.h> @@ -219,6 +220,8 @@ int abortboot(int bootdelay) { int abort = 0;
+ boottime_tag("autoboot_delay"); + #ifdef CONFIG_MENUPROMPT printf(CONFIG_MENUPROMPT); #else @@ -299,6 +302,8 @@ void main_loop (void) char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */
+ boottime_tag("main_loop"); + #ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); bootcount++;

Dear Lee Jones,
In message 1353422034-28107-5-git-send-email-lee.jones@linaro.org you wrote:
Here we add boottime tags to the start of the main loop and just before the opportunity to break into the u-boot shell. This will provide a more verbose bootgraph when viewed within debugfs.
Assuming we would take this route - then why do we need to add new boottime_tag() entries - is there anything wrong with the existing show_boot_progress() code? Did you consider using this instead? If so, why did you not use it?
Best regards,
Wolfgang Denk

On Tue, 20 Nov 2012, Wolfgang Denk wrote:
Dear Lee Jones,
In message 1353422034-28107-5-git-send-email-lee.jones@linaro.org you wrote:
Here we add boottime tags to the start of the main loop and just before the opportunity to break into the u-boot shell. This will provide a more verbose bootgraph when viewed within debugfs.
Assuming we would take this route - then why do we need to add new boottime_tag() entries - is there anything wrong with the existing show_boot_progress() code? Did you consider using this instead? If so, why did you not use it?
No, I didn't know it existed. Basically I've taken responsibility to upstream someone else's driver. It's more of a kernel thing, but it requires boottime information from u-boot too, as the intention is to cover full system booting, rather than the kernel-only tracing mechanisms which already exist.
I've just taken a look at show_boot_process() though. It doesn't appear to be suitable for what we require. Each tag needs to be individually identifiable, and I'm not sure how you would achieve this if we were to call back into a single function which would do the tagging.

Dear Lee Jones,
In message 20121121093647.GH28265@gmail.com you wrote:
show_boot_progress() code? Did you consider using this instead? If so, why did you not use it?
No, I didn't know it existed. Basically I've taken responsibility to upstream someone else's driver. It's more of a kernel thing, but it
This shouldbe considered a design fault.
Why do you need an extra driver when standard mechanisms exist that provide exactly the needed funtionality?
requires boottime information from u-boot too, as the intention is to cover full system booting, rather than the kernel-only tracing mechanisms which already exist.
But we can share a log buffer with Linux, both hence and back, so why do you not simply use this feature?
I've just taken a look at show_boot_process() though. It doesn't appear to be suitable for what we require. Each tag needs to be individually identifiable, and I'm not sure how you would achieve this if we were to call back into a single function which would do the tagging.
Each call takes an argument which is exactly used for such identification purposes. And you implementation can be mapped to write syslog entries into a shared (with Linux) log buffer, so no changes in Linux are needed.
Best regards,
Wolfgang Denk

show_boot_progress() code? Did you consider using this instead? If so, why did you not use it?
No, I didn't know it existed. Basically I've taken responsibility to upstream someone else's driver. It's more of a kernel thing, but it
This shouldbe considered a design fault.
Why do you need an extra driver when standard mechanisms exist that provide exactly the needed funtionality?
requires boottime information from u-boot too, as the intention is to cover full system booting, rather than the kernel-only tracing mechanisms which already exist.
But we can share a log buffer with Linux, both hence and back, so why do you not simply use this feature?
I've just taken a look at show_boot_process() though. It doesn't appear to be suitable for what we require. Each tag needs to be individually identifiable, and I'm not sure how you would achieve this if we were to call back into a single function which would do the tagging.
Each call takes an argument which is exactly used for such identification purposes. And you implementation can be mapped to write syslog entries into a shared (with Linux) log buffer, so no changes in Linux are needed.
It looked to me as though it took an integer identifier, which isn't going to mean anything to anyone. Unless there is a way to change the semantics of the function so that it would take a string, but then how would it play with the existing show_boot_progress() calls?

This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/include/asm/setup.h | 18 +++++++++++++++++ arch/arm/lib/bootm.c | 45 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/setup.h b/arch/arm/include/asm/setup.h index 78a7fac..6088440 100644 --- a/arch/arm/include/asm/setup.h +++ b/arch/arm/include/asm/setup.h @@ -205,6 +205,19 @@ struct tag_memclk { u32 fmemclk; };
+/* for automatic boot timing testcases */ +#define ATAG_BOOTTIME 0x41000403 +#define BOOTTIME_MAX 10 + +#include <boottime.h> + +struct tag_boottime { + struct boottime_entry entry[BOOTTIME_MAX]; + u32 idle; /* in us */ + u32 total; /* in us */ + u8 num; +}; + struct tag { struct tag_header hdr; union { @@ -227,6 +240,11 @@ struct tag { * DC21285 specific */ struct tag_memclk memclk; + + /* + * Boot time + */ + struct tag_boottime boottime; } u; };
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 1bd2730..03774c8 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -28,6 +28,7 @@ #include <common.h> #include <command.h> #include <image.h> +#include <boottime.h> #include <u-boot/zlib.h> #include <asm/byteorder.h> #include <fdt.h> @@ -114,7 +115,8 @@ static void announce_and_cleanup(void) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \ - defined(CONFIG_REVISION_TAG) + defined(CONFIG_REVISION_TAG) || \ + defined(CONFIG_BOOTTIME) static void setup_start_tag (bd_t *bd) { params = (struct tag *)bd->bi_boot_params; @@ -130,6 +132,37 @@ static void setup_start_tag (bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void) +{ + unsigned int i; + struct boottime_entry *b; + + params->hdr.tag = ATAG_BOOTTIME; + params->hdr.size = tag_size(tag_boottime); + + params->u.boottime.idle = boottime_idle_get(); + params->u.boottime.total = boottime_idle_done(); + + for (i = 0; i < BOOTTIME_MAX; i++) { + b = boottime_get_entry(i); + if (b == NULL) + break; + + params->u.boottime.entry[i].time = b->time; + strncpy((char *)params->u.boottime.entry[i].name, + (char *)b->name, BOOTTIME_MAX_NAME_LEN); + params->u.boottime.entry[i].name[BOOTTIME_MAX_NAME_LEN - 1] = '\0'; + + } + + params->u.boottime.num = i; + + params = tag_next(params); + +} +#endif + #ifdef CONFIG_SETUP_MEMORY_TAGS static void setup_memory_tags(bd_t *bd) { @@ -233,6 +266,10 @@ static void setup_end_tag(bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void); +#endif + #ifdef CONFIG_OF_LIBFDT static int create_fdt(bootm_headers_t *images) { @@ -293,9 +330,13 @@ static void boot_prep_linux(bootm_headers_t *images) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \ - defined(CONFIG_REVISION_TAG) + defined(CONFIG_REVISION_TAG) || \ + defined (CONFIG_BOOTTIME) debug("using: ATAGS\n"); setup_start_tag(gd->bd); +#ifdef CONFIG_BOOTTIME + setup_boottime_tags(); +#endif #ifdef CONFIG_SERIAL_TAG setup_serial_tag(¶ms); #endif

On Tue, Nov 20, 2012 at 12:33 PM, Lee Jones lee.jones@linaro.org wrote:
This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
Linus or Linux?
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
arch/arm/include/asm/setup.h | 18 +++++++++++++++++ arch/arm/lib/bootm.c | 45 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/setup.h b/arch/arm/include/asm/setup.h index 78a7fac..6088440 100644 --- a/arch/arm/include/asm/setup.h +++ b/arch/arm/include/asm/setup.h @@ -205,6 +205,19 @@ struct tag_memclk { u32 fmemclk; };
+/* for automatic boot timing testcases */ +#define ATAG_BOOTTIME 0x41000403 +#define BOOTTIME_MAX 10
+#include <boottime.h>
+struct tag_boottime {
struct boottime_entry entry[BOOTTIME_MAX];
u32 idle; /* in us */
u32 total; /* in us */
u8 num;
+};
struct tag { struct tag_header hdr; union { @@ -227,6 +240,11 @@ struct tag { * DC21285 specific */ struct tag_memclk memclk;
/*
* Boot time
*/
struct tag_boottime boottime; } u;
};
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 1bd2730..03774c8 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -28,6 +28,7 @@ #include <common.h> #include <command.h> #include <image.h> +#include <boottime.h> #include <u-boot/zlib.h> #include <asm/byteorder.h> #include <fdt.h> @@ -114,7 +115,8 @@ static void announce_and_cleanup(void) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \
defined(CONFIG_REVISION_TAG)
defined(CONFIG_REVISION_TAG) || \
defined(CONFIG_BOOTTIME)
static void setup_start_tag (bd_t *bd) { params = (struct tag *)bd->bi_boot_params; @@ -130,6 +132,37 @@ static void setup_start_tag (bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void) +{
unsigned int i;
struct boottime_entry *b;
params->hdr.tag = ATAG_BOOTTIME;
params->hdr.size = tag_size(tag_boottime);
params->u.boottime.idle = boottime_idle_get();
params->u.boottime.total = boottime_idle_done();
for (i = 0; i < BOOTTIME_MAX; i++) {
b = boottime_get_entry(i);
if (b == NULL)
break;
params->u.boottime.entry[i].time = b->time;
strncpy((char *)params->u.boottime.entry[i].name,
(char *)b->name, BOOTTIME_MAX_NAME_LEN);
params->u.boottime.entry[i].name[BOOTTIME_MAX_NAME_LEN -
1] = '\0';
}
params->u.boottime.num = i;
params = tag_next(params);
+} +#endif
#ifdef CONFIG_SETUP_MEMORY_TAGS static void setup_memory_tags(bd_t *bd) { @@ -233,6 +266,10 @@ static void setup_end_tag(bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void); +#endif
#ifdef CONFIG_OF_LIBFDT static int create_fdt(bootm_headers_t *images) { @@ -293,9 +330,13 @@ static void boot_prep_linux(bootm_headers_t *images) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \
defined(CONFIG_REVISION_TAG)
defined(CONFIG_REVISION_TAG) || \
defined (CONFIG_BOOTTIME) debug("using: ATAGS\n"); setup_start_tag(gd->bd);
+#ifdef CONFIG_BOOTTIME
setup_boottime_tags();
+#endif #ifdef CONFIG_SERIAL_TAG setup_serial_tag(¶ms);
#endif
1.7.9.5
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Tue, 20 Nov 2012, Otavio Salvador wrote:
On Tue, Nov 20, 2012 at 12:33 PM, Lee Jones lee.jones@linaro.org wrote:
This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
Linus or Linux?
Linux.
I'll fix-up when the review process has finished.
Thanks.
Based heavily on the original driver by Jonas Aaberg.
Signed-off-by: Lee Jones lee.jones@linaro.org
arch/arm/include/asm/setup.h | 18 +++++++++++++++++ arch/arm/lib/bootm.c | 45 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/setup.h b/arch/arm/include/asm/setup.h index 78a7fac..6088440 100644 --- a/arch/arm/include/asm/setup.h +++ b/arch/arm/include/asm/setup.h @@ -205,6 +205,19 @@ struct tag_memclk { u32 fmemclk; };
+/* for automatic boot timing testcases */ +#define ATAG_BOOTTIME 0x41000403 +#define BOOTTIME_MAX 10
+#include <boottime.h>
+struct tag_boottime {
struct boottime_entry entry[BOOTTIME_MAX];
u32 idle; /* in us */
u32 total; /* in us */
u8 num;
+};
struct tag { struct tag_header hdr; union { @@ -227,6 +240,11 @@ struct tag { * DC21285 specific */ struct tag_memclk memclk;
/*
* Boot time
*/
struct tag_boottime boottime; } u;
};
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 1bd2730..03774c8 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -28,6 +28,7 @@ #include <common.h> #include <command.h> #include <image.h> +#include <boottime.h> #include <u-boot/zlib.h> #include <asm/byteorder.h> #include <fdt.h> @@ -114,7 +115,8 @@ static void announce_and_cleanup(void) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \
defined(CONFIG_REVISION_TAG)
defined(CONFIG_REVISION_TAG) || \
defined(CONFIG_BOOTTIME)
static void setup_start_tag (bd_t *bd) { params = (struct tag *)bd->bi_boot_params; @@ -130,6 +132,37 @@ static void setup_start_tag (bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void) +{
unsigned int i;
struct boottime_entry *b;
params->hdr.tag = ATAG_BOOTTIME;
params->hdr.size = tag_size(tag_boottime);
params->u.boottime.idle = boottime_idle_get();
params->u.boottime.total = boottime_idle_done();
for (i = 0; i < BOOTTIME_MAX; i++) {
b = boottime_get_entry(i);
if (b == NULL)
break;
params->u.boottime.entry[i].time = b->time;
strncpy((char *)params->u.boottime.entry[i].name,
(char *)b->name, BOOTTIME_MAX_NAME_LEN);
params->u.boottime.entry[i].name[BOOTTIME_MAX_NAME_LEN -
1] = '\0';
}
params->u.boottime.num = i;
params = tag_next(params);
+} +#endif
#ifdef CONFIG_SETUP_MEMORY_TAGS static void setup_memory_tags(bd_t *bd) { @@ -233,6 +266,10 @@ static void setup_end_tag(bd_t *bd) } #endif
+#ifdef CONFIG_BOOTTIME +static void setup_boottime_tags(void); +#endif
#ifdef CONFIG_OF_LIBFDT static int create_fdt(bootm_headers_t *images) { @@ -293,9 +330,13 @@ static void boot_prep_linux(bootm_headers_t *images) defined(CONFIG_CMDLINE_TAG) || \ defined(CONFIG_INITRD_TAG) || \ defined(CONFIG_SERIAL_TAG) || \
defined(CONFIG_REVISION_TAG)
defined(CONFIG_REVISION_TAG) || \
defined (CONFIG_BOOTTIME) debug("using: ATAGS\n"); setup_start_tag(gd->bd);
+#ifdef CONFIG_BOOTTIME
setup_boottime_tags();
+#endif #ifdef CONFIG_SERIAL_TAG setup_serial_tag(¶ms);
#endif
1.7.9.5
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Otavio Salvador O.S. Systems E-mail: otavio@ossystems.com.br http://www.ossystems.com.br Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br

Dear Lee Jones,
In message 1353422034-28107-6-git-send-email-lee.jones@linaro.org you wrote:
This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
I implicitly mentioned this before, here it comes clear again:
I dislike the idea of adding such infrastructure in an archittecture dependent way, knowing from day one that we cannot use this as is for other architectures, and that the mechanism being used is even going to go away for this architecutre, too.
Please come up with a solution that works for all architectures instead.
Best regards,
Wolfgang Denk

Hi Wolfgang,
In message 1353422034-28107-6-git-send-email-lee.jones@linaro.org you wrote:
This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
I implicitly mentioned this before, here it comes clear again:
Ah, this has been tried before? Sorry, I didn't know that.
I dislike the idea of adding such infrastructure in an archittecture dependent way, knowing from day one that we cannot use this as is for other architectures, and that the mechanism being used is even going to go away for this architecutre, too.
Please come up with a solution that works for all architectures instead.
So I guess Device Tree it is then.
Kind regards, Lee

Dear Lee Jones,
In message 20121121091717.GG28265@gmail.com you wrote:
Hi Wolfgang,
In message 1353422034-28107-6-git-send-email-lee.jones@linaro.org you wrote:
This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
I implicitly mentioned this before, here it comes clear again:
Ah, this has been tried before? Sorry, I didn't know that.
I expolained it in my reply to your cover letter, i.e. in the message immediately preceeding the one you replied to here.
I dislike the idea of adding such infrastructure in an archittecture dependent way, knowing from day one that we cannot use this as is for other architectures, and that the mechanism being used is even going to go away for this architecutre, too.
Please come up with a solution that works for all architectures instead.
So I guess Device Tree it is then.
No. The device tree is for passing hardware information to U-Boot and the kernel. It is NOT intended for carrying things like debug or timing logs. It is not a good idea to misuse such services for things they were not made for nor where they fit.
Please use a standard facility, and one designed for such purposes like the Linux log buffer for this purpose. As explained, this has the added benefit that you don't need to change any Linux code. And you can build on the (also existing) show_boot_progress() support in U-Boot, so the extesions should actually be really small and pretty clear.
Best regards,
Wolfgang Denk

This patch adds support for passing boot time information to the Linus kernel using ATAGS when booting on ARM based devices.
I implicitly mentioned this before, here it comes clear again:
Ah, this has been tried before? Sorry, I didn't know that.
I expolained it in my reply to your cover letter, i.e. in the message immediately preceeding the one you replied to here.
So you're telling me off for sending a patch which doesn't agree with something you've said, despite you saying it _after_ I sent the patch?
Sounds sensible. :)
I dislike the idea of adding such infrastructure in an archittecture dependent way, knowing from day one that we cannot use this as is for other architectures, and that the mechanism being used is even going to go away for this architecutre, too.
Please come up with a solution that works for all architectures instead.
So I guess Device Tree it is then.
No. The device tree is for passing hardware information to U-Boot and the kernel. It is NOT intended for carrying things like debug or timing logs. It is not a good idea to misuse such services for things they were not made for nor where they fit.
Okay, got it.
Please use a standard facility, and one designed for such purposes like the Linux log buffer for this purpose. As explained, this has the added benefit that you don't need to change any Linux code. And you can build on the (also existing) show_boot_progress() support in U-Boot, so the extesions should actually be really small and pretty clear.
When you say log buffer, do you mean __log_buf? Doesn't this contain logs used for dmesg; thus won't all this crud end up in a user's dmesg kernel log? Unless there is another log which is used only for the kernel.
Also, wouldn't I then have to write a text parser to process this information? Sounds horrendous. Hopefully, I have missed something and it's actually easier than what I've mentioned.

Dear Lee Jones,
In message 20121121101310.GL28265@gmail.com you wrote:
I expolained it in my reply to your cover letter, i.e. in the message immediately preceeding the one you replied to here.
So you're telling me off for sending a patch which doesn't agree with something you've said, despite you saying it _after_ I sent the patch?
Sounds sensible. :)
Arghh... you don't _want_ to understand, right?
I was referring to my reply to your cover letter (patch 0/8) within this very patch series. It makes little sense to repeat what I've already told you just one or two messages before, or does it?
like the Linux log buffer for this purpose. As explained, this has the added benefit that you don't need to change any Linux code. And you can build on the (also existing) show_boot_progress() support in U-Boot, so the extesions should actually be really small and pretty clear.
When you say log buffer, do you mean __log_buf? Doesn't this contain logs used for dmesg; thus won't all this crud end up in a user's dmesg kernel log? Unless there is another log which is used only for the kernel.
Yes, I do mean __log_buf resp. the syslog services.
Yes, this will end up in the log buffer than can be displayed with dmesg.
If you consider this information "crud", then consider to disable the feature.
But then, guess why highres timestamps have been added to the kenrel logs? For people not interested in such informtion this is eventually "crud", but for others it appears important enough that it got added to Linux mainline.
If you are not interested in such information, then just use appropriate log levels and filtering.
Also, wouldn't I then have to write a text parser to process this information? Sounds horrendous. Hopefully, I have missed something and it's actually easier than what I've mentioned.
Guess how many tools are out there that already deal with filtering and post-processing of kernel log messages? A google search for "syslog filter" returns millions of hits...
Best regards,
Wolfgang Denk

I expolained it in my reply to your cover letter, i.e. in the message immediately preceeding the one you replied to here.
So you're telling me off for sending a patch which doesn't agree with something you've said, despite you saying it _after_ I sent the patch?
Sounds sensible. :)
Arghh... you don't _want_ to understand, right?
I was referring to my reply to your cover letter (patch 0/8) within this very patch series. It makes little sense to repeat what I've already told you just one or two messages before, or does it?
I think this is meerly a communication issue. I took "I implicitly mentioned this before, here it comes clear again", to mean "I've told you already, why aren't you listening to me".
like the Linux log buffer for this purpose. As explained, this has the added benefit that you don't need to change any Linux code. And you can build on the (also existing) show_boot_progress() support in U-Boot, so the extesions should actually be really small and pretty clear.
When you say log buffer, do you mean __log_buf? Doesn't this contain logs used for dmesg; thus won't all this crud end up in a user's dmesg kernel log? Unless there is another log which is used only for the kernel.
Yes, I do mean __log_buf resp. the syslog services.
Yes, this will end up in the log buffer than can be displayed with dmesg.
If you consider this information "crud", then consider to disable the feature.
It's only "curd" to the user typing `dmesg`. If we're trying to measure whole system boot-up time, it's useful information.
But then, guess why highres timestamps have been added to the kenrel logs? For people not interested in such informtion this is eventually "crud", but for others it appears important enough that it got added to Linux mainline.
If you are not interested in such information, then just use appropriate log levels and filtering.
I think the kernel log is the wrong place for this to go. Although, the kernel driver will allow you to print the information in a log format by cat'ing <debugfs>/boottime/bootgraph, it's not really kernel logging information. It's mearly a collection of trace-points containing a timestamp and some means of identification.
Filling the kernel log with lots of trace-points is definitely wrong.
Also, wouldn't I then have to write a text parser to process this information? Sounds horrendous. Hopefully, I have missed something and it's actually easier than what I've mentioned.
Guess how many tools are out there that already deal with filtering and post-processing of kernel log messages? A google search for "syslog filter" returns millions of hits...
So you're suggesting that we create a userland portion of the driver too? I don't think this is acceptable. This tool will be used by kernel engineers, who would be more happy taking the information from debugfs. At least I know I would.

Dear Lee Jones,
In message 20121121143928.GA28899@gmail.com you wrote:
If you are not interested in such information, then just use appropriate log levels and filtering.
I think the kernel log is the wrong place for this to go. Although,
OK, this is your opinion, then, and I will respect it.
It is my opinion that mechanisms as ATAGS or device tree are inappropriate for passing any kind of log data to the kernel.
So far, the established way of passing logging information (like results of Power-On Selft Tests,e tc.) is through a shared log buffer.
the kernel driver will allow you to print the information in a log format by cat'ing <debugfs>/boottime/bootgraph, it's not really kernel logging information. It's mearly a collection of trace-points containing a timestamp and some means of identification.
I don't see what a kernel driver may be needed for, but for this part of the discussion this is not relevant anyway.
Filling the kernel log with lots of trace-points is definitely wrong.
Again, this is your opinion, and I respect it. On the other hand, what is the kernel log being used for on log level INFO, for example?
So you're suggesting that we create a userland portion of the driver too? I don't think this is acceptable. This tool will be used by kernel engineers, who would be more happy taking the information from debugfs. At least I know I would.
I do not suggest anything like that. I suggest not to write any driver at all, nor any other code. I suggest to use existing tools, as I recommend to reuse existing (standard) methods and protocols instead of inventing new ones.
Best regards,
Wolfgang Denk

If you are not interested in such information, then just use appropriate log levels and filtering.
I think the kernel log is the wrong place for this to go. Although,
OK, this is your opinion, then, and I will respect it.
Thank you, and I yours.
It is my opinion that mechanisms as ATAGS or device tree are inappropriate for passing any kind of log data to the kernel.
I agree with you.
So far, the established way of passing logging information (like results of Power-On Selft Tests,e tc.) is through a shared log buffer.
Also true, but is that data used in this way? Or is it just printed out at boot time? This tool aims to gather all boot time statistics in one, easy to access place so that (generally kernel) engineers may make improvements based on the data provided.
the kernel driver will allow you to print the information in a log format by cat'ing <debugfs>/boottime/bootgraph, it's not really kernel logging information. It's mearly a collection of trace-points containing a timestamp and some means of identification.
I don't see what a kernel driver may be needed for, but for this part of the discussion this is not relevant anyway.
I've already had lots of interest in this from the kernel community, but if it can't make use of the bootloader portion easily, then it just becomes another tracing/bootchart mechanism.
Filling the kernel log with lots of trace-points is definitely wrong.
Again, this is your opinion, and I respect it. On the other hand, what is the kernel log being used for on log level INFO, for example?
If I print my current bootgraph it's currently 507 lines. This should not go into the kernel log at any level. Albeit almost all of them are kernel related entries, I've already mentioned that I don't want two different mechanisms for storing bootloader and kernel entries. It's very rare that you'd even need to print it out, but when you do it's there in debugfs.
So you're suggesting that we create a userland portion of the driver too? I don't think this is acceptable. This tool will be used by kernel engineers, who would be more happy taking the information from debugfs. At least I know I would.
I do not suggest anything like that. I suggest not to write any driver at all, nor any other code. I suggest to use existing tools, as I recommend to reuse existing (standard) methods and protocols instead of inventing new ones.
You're suggesting that we parse the log with a bespoke text parsing tool in order to get the information we need. However, this would a) require us to overwhelm the kernel's log and b) We'd have to maintain a separate script or app that would be capable of the task. I'm just not happy with the implementation.
To have something in the kernel, which does all this automagically would be much simpler for the user. This tool was created to solve a real problem. Kernel engineers do not currently have an easy way to trace booting time from bootloader. This is the solution to that problem.
1. Enable boottime config 2. Enable debugfs config (if it's not already) 3. Mount debugfs (if it's not already) 4. cat /sys/kernel/debug/[summary|bootgraph]
Simples.

Dear Lee Jones,
In message 20121121174856.GC417@gmail.com you wrote:
So far, the established way of passing logging information (like results of Power-On Selft Tests,e tc.) is through a shared log buffer.
Also true, but is that data used in this way? Or is it just printed out at boot time? This tool aims to gather all boot
It is definitely used that way. For example, there are systems that parse the syslog output for specific POST results, and prevent certain functionality to be enabled when the POST detected problems with the hardware, or other conditions that require specific actions.
time statistics in one, easy to access place so that (generally kernel) engineers may make improvements based on the data provided.
And it has to be implemented using a completely new method, because all existing ones are crap or not the "correct way" to do it. We've been there before.
If I print my current bootgraph it's currently 507 lines. This should not go into the kernel log at any level. Albeit almost
"This should not". OK, this is your decision, whatever the reasoning for it may be, and whatever others may think about it.
I accept this, but please also accept that I ask you not to add code that duplicates existing functionality to U-Boot.
parsing tool in order to get the information we need. However, this would a) require us to overwhelm the kernel's log and b) We'd have to maintain a separate script or app that would be capable of the task. I'm just not happy with the implementation.
To have something in the kernel, which does all this automagically would be much simpler for the user. This tool was created to solve a real problem. Kernel engineers do not currently have an easy way to trace booting time from bootloader. This is the solution to that problem.
Again, this is your opinion. My strong believe is that it is almost always much more advisable to implement such functionality not in the kernel, but in user space. There are certain things that belong to the kernel, but everything else should not be done there. Gathering and processing statistical information, generating charts and the like are nothing I consider typical kernel tasks.
But we're off topic here - this is the U-Boot mailing list, and I'm not in a position to citizise the design of your kernel code.
But as much as you "don't want two different mechanisms for storing bootloader and kernel entries" I don't want that either. And that is the reason to reject these patches, asking for a change of the implementation to use the already existing methods.
Best regards,
Wolfgang Denk

Let's try to move this forward.
And it has to be implemented using a completely new method, because all existing ones are crap or not the "correct way" to do it. We've been there before.
I accept this, but please also accept that I ask you not to add code that duplicates existing functionality to U-Boot.
But as much as you "don't want two different mechanisms for storing bootloader and kernel entries" I don't want that either. And that is the reason to reject these patches, asking for a change of the implementation to use the already existing methods.
Okay, to summarise so far:
1. Bootloader and kernel mechanisms should be the same
So putting bootloader tracepoints in the bootlog and the kernel's in an internal data structure is not acceptable, as it would add too much extra overhead to link them together and parse.
2. The kernel bootlog is not the correct place for tracepoints
If we were to adhere to point No1 and bootloader & kernel entries would be placed into the bootlog; no self respecting kernel engineer/maintainer will allow 100's of spurious tracepoint entries in the kernel log, regardless of log level. No other tracing mechanism does this and there's a good reason for that.
3. Existing mechanisms to be used if they exist and are suitable
I am more than happy to use anything that already exists, but I haven't seen anything yet. I can see how we could use the show_boot_progress() call-back, but we'd have to use #defines to store anything useful with regards to unique and readable identifiers. Also, we'd have to be careful to use it in such a what that it wouldn't trash any architecture's implementation, which would probably mean calling boottime_tag from within them.
As for passing the information to the kernel; munging it as text and sticking it in __log_buf is out of the question, ATAGs are architecture specific and are leaving us and DT is designed to carry hardware information. So where does that leave us?
Actually, putting it in DT has lots of advantages. 1) DT works cross-architecture and cross-platform, so your architecture independent box is inherently ticked 2) DT already carries non-hardware specific information such as the kernel cmdline. I don't see how adding <10 (but would more likely to be 2-3) tracepoint entries would completely break convention. We can either get the kernel driver to scrub the entries if you'd be that offended by keeping them in.
Let's be pragmatic about this. I'm happy to make changes to the current implementation and code-up any sensible solutions to this issue. It's a real problem that has gained a great deal of interest amongst my kernel engineer peers. So much so that people have stopped to ask me about it at kernel conferences. Let's find a nice, long lasting way to solve it.

Dear Lee Jones,
In message 20121122101433.GA4328@gmail.com you wrote:
Let's try to move this forward.
Actually you don't. You insist on not changing anything on your side, and ask others to adapt to it.
Okay, to summarise so far:
- Bootloader and kernel mechanisms should be the same
So putting bootloader tracepoints in the bootlog and the kernel's in an internal data structure is not acceptable, as it would add too much extra overhead to link them together and parse.
OK. But this appears a new requirement - your original implementation did not bother about this, using ATAGs here and somethign else there...
- The kernel bootlog is not the correct place for tracepoints
If we were to adhere to point No1 and bootloader & kernel entries would be placed into the bootlog; no self respecting kernel engineer/maintainer will allow 100's of spurious tracepoint entries in the kernel log, regardless of log level.
I wonder about the self-assuredness you speak for all of them. Has this been dicussed in full context before?
Actually, putting it in DT has lots of advantages. 1) DT works cross-architecture and cross-platform, so your architecture independent box is inherently ticked 2) DT already carries non-hardware specific information such as the kernel cmdline. I don't see how adding <10 (but would more likely to be 2-3) tracepoint entries would completely break convention. We can either get the kernel driver to scrub the entries if you'd be that offended by keeping them in.
Hm... in accordance to No. 1 above your kernel code will add new entries to the device tree while the kernel is booting? So instead of "adding <10" it now will be adding "100's of spurious tracepoint entries" to the DT?
Are you sure this is a good idea? [Not considering if it's actually possible.]
But then, if you drop item 1, then what's wrong with "<10 (but would more likely to be 2-3)" additional log messages?
Best regards,
Wolfgang Denk

Let's try to move this forward.
Actually you don't. You insist on not changing anything on your side, and ask others to adapt to it.
Not at all.
This current implementation is unacceptable to you and your counter- suggestion is unacceptable to me (and all other kernel engineers).
I'm seeking a new avenue.
Okay, to summarise so far:
- Bootloader and kernel mechanisms should be the same
So putting bootloader tracepoints in the bootlog and the kernel's in an internal data structure is not acceptable, as it would add too much extra overhead to link them together and parse.
OK. But this appears a new requirement - your original implementation did not bother about this, using ATAGs here and somethign else there...
No, they're the same in the current implementation:
struct tag_boottime { struct boottime_entry entry[BOOTTIME_MAX]; u32 idle; /* in us */ u32 total; /* in us */ u8 num; };
ATAGs is mearly a way to pass the data structure through.
Once the data is passed to though, the kernel then just populates the structure with bootloader and kernel tracepoints alike. No differentiation is made between the two.
- The kernel bootlog is not the correct place for tracepoints
If we were to adhere to point No1 and bootloader & kernel entries would be placed into the bootlog; no self respecting kernel engineer/maintainer will allow 100's of spurious tracepoint entries in the kernel log, regardless of log level.
I wonder about the self-assuredness you speak for all of them. Has this been dicussed in full context before?
It's just not logical.
Putting 100's of tracepoints in the kernel log is insane.
Actually, putting it in DT has lots of advantages. 1) DT works cross-architecture and cross-platform, so your architecture independent box is inherently ticked 2) DT already carries non-hardware specific information such as the kernel cmdline. I don't see how adding <10 (but would more likely to be 2-3) tracepoint entries would completely break convention. We can either get the kernel driver to scrub the entries if you'd be that offended by keeping them in.
Hm... in accordance to No. 1 above your kernel code will add new entries to the device tree while the kernel is booting? So instead of "adding <10" it now will be adding "100's of spurious tracepoint entries" to the DT?
Are you sure this is a good idea? [Not considering if it's actually possible.]
But then, if you drop item 1, then what's wrong with "<10 (but would more likely to be 2-3)" additional log messages?
Granted, I can understand the points above.
If it's feasible to put them in the buffer, parse it, then scrub them so they don't appear in dmesg output, I guess that could be doable.
Ideally I'd like to keep it all as data, as it will save lots of text parsing code in the kernel. Surely there must be a call for passing data structures from the bootloader to the kernel. After all, that's why ATAGs were brought about wasn't it?

Dear Lee Jones,
In message 20121122160847.GD10986@gmail.com you wrote:
Ideally I'd like to keep it all as data, as it will save lots of text parsing code in the kernel. Surely there must be a call for passing data structures from the bootloader to the kernel. After all, that's why ATAGs were brought about wasn't it?
Look at which ATAGS exist to see what they have been invented for. Read the previous discussions why for example pretty useful extensions like a tag to pass a MAC address from a boot loader to the kernel have never been accepted for mainline. But you claim that trace data are different, and fit better?
I consider it a design flaw to do such statictics stuff in the kernel. It does not belong there. Such functions belong to user space.
But as mentioned before, this is actually off topic here.
Best regards,
Wolfgang Denk

Ideally I'd like to keep it all as data, as it will save lots of text parsing code in the kernel. Surely there must be a call for passing data structures from the bootloader to the kernel. After all, that's why ATAGs were brought about wasn't it?
Look at which ATAGS exist to see what they have been invented for. Read the previous discussions why for example pretty useful extensions like a tag to pass a MAC address from a boot loader to the kernel have never been accepted for mainline. But you claim that trace data are different, and fit better?
I don't know the history.
I consider it a design flaw to do such statictics stuff in the kernel. It does not belong there. Such functions belong to user space.
I don't agree.
But as mentioned before, this is actually off topic here.
Then stop mentioning it. ;)

This will provide a more verbose bootgraph when viewed within debugfs. It will also ensure that we have a tag at the latest possible point in the bootloader, right before we pass the ATAGs though to the kernel.
Signed-off-by: Lee Jones lee.jones@linaro.org --- arch/arm/lib/board.c | 3 +++ arch/arm/lib/bootm.c | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index 92cad9a..f8c7b5d 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -52,6 +52,7 @@ #include <fdtdec.h> #include <post.h> #include <logbuff.h> +#include <boottime.h>
#ifdef CONFIG_BITBANGMII #include <miiphy.h> @@ -486,6 +487,8 @@ void board_init_r(gd_t *id, ulong dest_addr) ulong flash_size; #endif
+ boottime_tag("board_init"); + gd = id;
gd->flags |= GD_FLG_RELOC; /* tell others: relocation done */ diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 03774c8..fa3291c 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -333,6 +333,7 @@ static void boot_prep_linux(bootm_headers_t *images) defined(CONFIG_REVISION_TAG) || \ defined (CONFIG_BOOTTIME) debug("using: ATAGS\n"); + boottime_tag("passing_atags"); setup_start_tag(gd->bd); #ifdef CONFIG_BOOTTIME setup_boottime_tags(); @@ -402,6 +403,8 @@ static void boot_jump_linux(bootm_headers_t *images) */ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) { + boottime_tag("do_bootm_linux"); + /* No need for those on ARM */ if (flag & BOOTM_STATE_OS_BD_T || flag & BOOTM_STATE_OS_CMDLINE) return -1;

Allow the bootloader to pass bootloader specific boot-up time information to the Linux kernel via ATAGs when booting the db8500 based HREF development board.
Signed-off-by: Lee Jones lee.jones@linaro.org --- include/configs/u8500_href.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/u8500_href.h b/include/configs/u8500_href.h index 1bb6128..a6c29f5 100644 --- a/include/configs/u8500_href.h +++ b/include/configs/u8500_href.h @@ -37,6 +37,8 @@ #define CONFIG_BOARD_EARLY_INIT_F #define CONFIG_BOARD_LATE_INIT
+#define CONFIG_BOOTTIME + /* * Size of malloc() pool */

Allow the bootloader to pass bootloader specific boot-up time information to the Linux kernel via ATAGs when booting the db8500 based Snowball development board.
Signed-off-by: Lee Jones lee.jones@linaro.org --- include/configs/snowball.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/configs/snowball.h b/include/configs/snowball.h index 30f4a4e..6ce45b9 100644 --- a/include/configs/snowball.h +++ b/include/configs/snowball.h @@ -206,6 +206,8 @@ #define CONFIG_INITRD_TAG 1 #define CONFIG_CMDLINE_TAG 1 /* enable passing of ATAGs */
+#define CONFIG_BOOTTIME + /* * Physical Memory Map */

Dear Lee Jones,
In message 1353422034-28107-1-git-send-email-lee.jones@linaro.org you wrote:
In this patch-set we're attempting to add boottime measurement support to u-boot. A patch-set has already hit the kernel MLs which intends to solve the other half of the puzzle.
This new tool allows an engineer to apply tags into key areas around the bootloader, which are then passed to the Linux kernel for processing and statistics analysis of full (bootloader
- kernel) device booting.
tags into key areas around the boot loader?
Why do you reinvent the wheel, squarely this time? Why don't you simply use a shared log buffer?
Best regards,
Wolfgang Denk

In this patch-set we're attempting to add boottime measurement support to u-boot. A patch-set has already hit the kernel MLs which intends to solve the other half of the puzzle.
This new tool allows an engineer to apply tags into key areas around the bootloader, which are then passed to the Linux kernel for processing and statistics analysis of full (bootloader
- kernel) device booting.
tags into key areas around the boot loader?
Why do you reinvent the wheel, squarely this time? Why don't you simply use a shared log buffer?
Please see my previous email.
Kind regards, Lee
participants (4)
-
Lee Jones
-
Otavio Salvador
-
Simon Glass
-
Wolfgang Denk