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

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

[Lee Jones]

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(-)

[U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over

[Lee Jones]

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)
-- 
1.7.9.5

[U-Boot] [PATCH 2/8] u8500: Add utimer support

[Lee Jones]

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);
 
-- 
1.7.9.5

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

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@%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 */
-- 
1.7.9.5

[U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code

[Lee Jones]

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++;
-- 
1.7.9.5

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

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(&params);
 #endif
-- 
1.7.9.5

[U-Boot] [PATCH 6/8] arm: Add some boottime tags into prime booting locations

[Lee Jones]

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;
-- 
1.7.9.5

[U-Boot] [PATCH 7/8] href: Enable boottime functionality

[Lee Jones]

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
  */
-- 
1.7.9.5

[U-Boot] [PATCH 8/8] snowball: Enable boottime functionality

[Lee Jones]

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
  */
-- 
1.7.9.5

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Otavio Salvador]

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(&params);
>  #endif
> --
> 1.7.9.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>



-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

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(&params);
> >  #endif
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
> 
> 
> 
> -- 
> Otavio Salvador                             O.S. Systems
> E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
> Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Command, n.:
            Statement presented by a human and accepted by a computer
in such a manner as to make the human feel as if he is in control.

[U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Bei genauerem Hinsehen ist die  Arbeit  weniger  langweilig  als  das
Vergn?gen.                                      -- Charles Baudelaire

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A person with one watch knows what time it  is;  a  person  with  two
watches is never sure.                                       Proverb

[U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The question of whether a computer can think is no more  interesting
than the question of whether a submarine can swim"
                                                - Edsgar W.  Dijkstra

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Due to lack of disk space, this fortune database has been discontinued.

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

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

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
backups: always in season, never out of style.

[U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code

[Lee Jones]

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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

> > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over

[Lee Jones]

> > 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

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

> > 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

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

> > > > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Computers are not intelligent.  They only think they are.

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
And now remains  That we find out the cause of this effect, Or rather
say, the cause of this defect...           -- Hamlet, Act II, Scene 2

[U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If God had wanted us to use the metric system, Jesus would have  had
10 apostles."

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
As far as the laws of mathematics refer to reality, they are not cer-
tain, and as far as they are certain, they do not refer  to  reality.
                                                   -- Albert Einstein

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

> > > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over

[Lee Jones]

> > > > -#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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

> > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code

[Lee Jones]

> > > 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?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A witty saying proves nothing, but saying  something  pointless  gets
people's attention.

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No journaling file system can recover your data if the disk dies.
                                 - Steve Rago in <D4Cw1p.L9E@plc.com>

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

> > > 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.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the hours are long enough and the pay  is  short  enough,  someone
will say it's women's work.

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No more blah, blah, blah!
	-- Kirk, "Miri", stardate 2713.6

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

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@kernel
conferences. Let's find a nice, long lasting way to solve it.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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:
> 
> 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.

OK.  But this appears a new requirement - your original implementation
did not bother about this, using ATAGs here and somethign else
there...

> 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.

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
I must follow the people.  Am I not their leader? - Benjamin Disraeli

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

> > 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:
> > 
> > 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.
> 
> 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.

> > 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.
> 
> 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?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Just about every computer on the market today runs Unix,  except  the
Mac (and nobody cares about it).                   - Bill Joy 6/21/85

[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones]

> > 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. ;)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Simon Glass]

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

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

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

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Simon Glass]

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

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

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

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is much easier to suggest solutions when you know nothing

[U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support

[Lee Jones]

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. :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog