Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Hi Mike, On Sat, Jan 14, 2012 at 5:27 PM, Simon Glass s...@chromium.org wrote: Hi Mike, On Sat, Jan 14, 2012 at 5:22 PM, Mike Frysinger vap...@gentoo.org wrote: On Saturday 14 January 2012 20:16:35 Simon Glass wrote: On Sat, Jan 14, 2012 at 5:09 PM, Mike Frysinger wrote: On Thursday 12 January 2012 00:41:24 Simon Glass wrote: Still don't quite get it though. For example, the beagle board defines show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so wouldn't that break that board? that sounds like an odd-man-out that needs fixing rather than allowing to live Fair enough. although I suspect there will be many. If I could actually get a MAKEALL to run without producing 100s of broken boards then it would be easier to do this sort of thing. At the moment it's like looking for a needle in a haystack. My warnings series aimed to improve things slightly, but I don't think others have these problems. that's fair. if it's a small # of boards, i'd prefer to migrate them. if it's a lot more, we can punt for now (add to the TODO?) and add comments to the code why we have these checks. OK, I don't know the answer to that but will try it out, and adjust that patch as needed. I have got back to this now - it seem that I can rely on the existing weak function, so I will do so. I only want to update two of the patches, so will just send those ones. The rest should be fine as is. Regards, SImon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
On Thursday 12 January 2012 00:41:24 Simon Glass wrote:
On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger wrote:
On Sunday 08 January 2012 12:42:02 Simon Glass wrote:
On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger wrote:
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when
CONFIG_SHOW_BOOT_PROGRESS isn't defined ? then you don't have to
protect the call sites.
show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.
One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.
it seemed like part of your clean up series was to merge
show_boot_progress() into your new bootstage framework. in which case,
we have full control over it now, and ifdef bracketing for it should go
away ...
Still don't quite get it though. For example, the beagle board defines
show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so
wouldn't that break that board?
that sounds like an odd-man-out that needs fixing rather than allowing to live
-mike
signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Hi Mike,
On Sat, Jan 14, 2012 at 5:09 PM, Mike Frysinger vap...@gentoo.org wrote:
On Thursday 12 January 2012 00:41:24 Simon Glass wrote:
On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger wrote:
On Sunday 08 January 2012 12:42:02 Simon Glass wrote:
On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger wrote:
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when
CONFIG_SHOW_BOOT_PROGRESS isn't defined ? then you don't have to
protect the call sites.
show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.
One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.
it seemed like part of your clean up series was to merge
show_boot_progress() into your new bootstage framework. in which case,
we have full control over it now, and ifdef bracketing for it should go
away ...
Still don't quite get it though. For example, the beagle board defines
show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so
wouldn't that break that board?
that sounds like an odd-man-out that needs fixing rather than allowing to live
Fair enough. although I suspect there will be many. If I could
actually get a MAKEALL to run without producing 100s of broken boards
then it would be easier to do this sort of thing. At the moment it's
like looking for a needle in a haystack. My warnings series aimed to
improve things slightly, but I don't think others have these problems.
Regards,
Simon
-mike
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
On Saturday 14 January 2012 20:16:35 Simon Glass wrote: On Sat, Jan 14, 2012 at 5:09 PM, Mike Frysinger wrote: On Thursday 12 January 2012 00:41:24 Simon Glass wrote: Still don't quite get it though. For example, the beagle board defines show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so wouldn't that break that board? that sounds like an odd-man-out that needs fixing rather than allowing to live Fair enough. although I suspect there will be many. If I could actually get a MAKEALL to run without producing 100s of broken boards then it would be easier to do this sort of thing. At the moment it's like looking for a needle in a haystack. My warnings series aimed to improve things slightly, but I don't think others have these problems. that's fair. if it's a small # of boards, i'd prefer to migrate them. if it's a lot more, we can punt for now (add to the TODO?) and add comments to the code why we have these checks. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Hi Mike, On Sat, Jan 14, 2012 at 5:22 PM, Mike Frysinger vap...@gentoo.org wrote: On Saturday 14 January 2012 20:16:35 Simon Glass wrote: On Sat, Jan 14, 2012 at 5:09 PM, Mike Frysinger wrote: On Thursday 12 January 2012 00:41:24 Simon Glass wrote: Still don't quite get it though. For example, the beagle board defines show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so wouldn't that break that board? that sounds like an odd-man-out that needs fixing rather than allowing to live Fair enough. although I suspect there will be many. If I could actually get a MAKEALL to run without producing 100s of broken boards then it would be easier to do this sort of thing. At the moment it's like looking for a needle in a haystack. My warnings series aimed to improve things slightly, but I don't think others have these problems. that's fair. if it's a small # of boards, i'd prefer to migrate them. if it's a lot more, we can punt for now (add to the TODO?) and add comments to the code why we have these checks. OK, I don't know the answer to that but will try it out, and adjust that patch as needed. Regards, Simon -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Hi Mike,
On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger vap...@gentoo.org wrote:
On Sunday 08 January 2012 12:42:02 Simon Glass wrote:
On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger wrote:
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when CONFIG_SHOW_BOOT_PROGRESS
isn't defined ? then you don't have to protect the call sites.
show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.
One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.
it seemed like part of your clean up series was to merge show_boot_progress()
into your new bootstage framework. in which case, we have full control over
it now, and ifdef bracketing for it should go away ...
Still don't quite get it though. For example, the beagle board defines
show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so
wouldn't that break that board?
The series as is was tested MAKEALL clean.
Regards,
Simon
-mike
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
On Sunday 08 January 2012 12:42:02 Simon Glass wrote:
On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger wrote:
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when CONFIG_SHOW_BOOT_PROGRESS
isn't defined ? then you don't have to protect the call sites.
show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.
One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.
it seemed like part of your clean up series was to merge show_boot_progress()
into your new bootstage framework. in which case, we have full control over
it now, and ifdef bracketing for it should go away ...
-mike
signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
This defines the basics of a new boot time measurement feature. This allows
logging of very accurate time measurements as the boot proceeds, by using
an available microsecond counter.
To enable the feature, define CONFIG_BOOTSTAGE in your board config file.
Also available is CONFIG_BOOTSTAGE_REPORT which will cause a report to be
printed just before handing off to the OS.
the summary says Add microsecond boot time measurement. that sounds like
extending existing functionality. in reality, this is the core of the new
CONFIG_BOOTSTAGE logic right ?
also, this logging framework seems to overlap the existing POST logging
framework functionality ...
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when CONFIG_SHOW_BOOT_PROGRESS
isn't defined ? then you don't have to protect the call sites.
+static inline ulong bootstage_mark_name(enum bootstage_id id, const char
*name)
+{ return 0; }
please unroll this
-mike
signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Hi Mike,
On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger vap...@gentoo.org wrote:
On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
This defines the basics of a new boot time measurement feature. This allows
logging of very accurate time measurements as the boot proceeds, by using
an available microsecond counter.
To enable the feature, define CONFIG_BOOTSTAGE in your board config file.
Also available is CONFIG_BOOTSTAGE_REPORT which will cause a report to be
printed just before handing off to the OS.
the summary says Add microsecond boot time measurement. that sounds like
extending existing functionality. in reality, this is the core of the new
CONFIG_BOOTSTAGE logic right ?
Yes - I will update the commit title.
also, this logging framework seems to overlap the existing POST logging
framework functionality ...
Please see previous message - I have integrated show_boot_progress()
as per previous comments, But power-on self test has some separate
functionality. It does call show_boot_progress() on failure.
This series is about the U-Boot side of supporting boot timing from
reset to Linux user space.
--- a/include/bootstage.h
+++ b/include/bootstage.h
+static inline ulong bootstage_mark(enum bootstage_id id)
{
- show_boot_progress(-val);
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(id);
+#endif
+ return 0;
}
+static inline ulong bootstage_error(enum bootstage_id id)
+{
+#ifdef CONFIG_SHOW_BOOT_PROGRESS
+ show_boot_progress(-id);
+#endif
+ return 0;
+}
why isn't show_boot_progress() just a stub when CONFIG_SHOW_BOOT_PROGRESS
isn't defined ? then you don't have to protect the call sites.
show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.
One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.
+static inline ulong bootstage_mark_name(enum bootstage_id id, const char
*name)
+{ return 0; }
please unroll this
done
-mike
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
This defines the basics of a new boot time measurement feature. This allows
logging of very accurate time measurements as the boot proceeds, by using
an available microsecond counter.
To enable the feature, define CONFIG_BOOTSTAGE in your board config file.
Also available is CONFIG_BOOTSTAGE_REPORT which will cause a report to be
printed just before handing off to the OS.
Most IDs are not named at this stage. For that I would first like to
renumber them all.
Timer summary in microseconds:
MarkElapsed Stage
0 0 reset
205,000205,000 board_init_f
6,053,000 5,848,000 bootm_start
6,053,000 0 id=1
6,058,000 5,000 id=101
6,058,000 0 id=100
6,061,000 3,000 id=103
6,064,000 3,000 id=104
6,093,000 29,000 id=107
6,093,000 0 id=106
6,093,000 0 id=105
6,093,000 0 id=108
7,089,000996,000 id=7
7,089,000 0 id=15
7,089,000 0 id=8
7,097,000 8,000 start_kernel
Signed-off-by: Simon Glass s...@chromium.org
---
README | 25
common/Makefile |1 +
common/bootstage.c | 160 +++
include/bootstage.h | 75 +++-
4 files changed, 259 insertions(+), 2 deletions(-)
create mode 100644 common/bootstage.c
diff --git a/README b/README
index e9d1891..20c09e6 100644
--- a/README
+++ b/README
@@ -2198,6 +2198,31 @@ The following options need to be configured:
example, some LED's) on your board. At the moment,
the following checkpoints are implemented:
+- Detailed boot stage timing
+ CONFIG_BOOTSTAGE
+ Define this option to get detailed timing of each stage
+ of the boot process.
+
+ CONFIG_BOOTSTAGE_USER_COUNT
+ This is the number of available user bootstage records.
+ Each time you call bootstage_mark(BOOTSTAGE_ID_ALLOC, ...)
+ a new ID will be allocated from this stash. If you exceed
+ the limit, recording will stop.
+
+ CONFIG_BOOTSTAGE_REPORT
+ Define this to print a report before boot, similar to this:
+
+ Timer summary in microseconds:
+ MarkElapsed Stage
+ 0 0 reset
+ 3,575,678 3,575,678 board_init_f start
+ 3,575,695 17 arch_cpu_init A9
+ 3,575,777 82 arch_cpu_init done
+ 3,659,598 83,821 board_init_r start
+ 3,910,375250,777 main_loop
+29,916,167 26,005,792 bootm_start
+30,361,327445,160 start_kernel
+
Legacy uImage format:
Arg Where When
diff --git a/common/Makefile b/common/Makefile
index 1be7236..5530f2c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -175,6 +175,7 @@ SPD := y
endif
COBJS-$(SPD) += ddr_spd.o
COBJS-$(CONFIG_HWCONFIG) += hwconfig.o
+COBJS-$(CONFIG_BOOTSTAGE) += bootstage.o
COBJS-$(CONFIG_CONSOLE_MUX) += iomux.o
COBJS-y += flash.o
COBJS-$(CONFIG_CMD_KGDB) += kgdb.o kgdb_stubs.o
diff --git a/common/bootstage.c b/common/bootstage.c
new file mode 100644
index 000..358e1ca
--- /dev/null
+++ b/common/bootstage.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2011, Google Inc. All rights reserved.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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
+ */
+
+
+/*
+ * This module records the progress of boot and arbitrary commands, and
+ * permits accurate timestamping of each.
+ *
+ * TBD: Pass timings to kernel in the FDT
+ */
+
+#include common.h
+#include libfdt.h
+
+DECLARE_GLOBAL_DATA_PTR;
+
+enum bootstage_flags {
+ BOOTSTAGEF_ERROR= 1 0, /* Error record */
+ BOOTSTAGEF_ALLOC= 1 1, /* Allocate an id */
+};
+
+struct bootstage_record {
+ ulong time_us;
+ const char *name;
+ int flags; /* see enum bootstage_flags */
+ enum bootstage_id id;
+};
+
+static struct bootstage_record record[BOOTSTAGE_ID_COUNT] = { {1} };
+static int next_id = BOOTSTAGE_ID_USER;
+
+ulong bootstage_add_record(enum
