[U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement

From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH v2 13/15] bootstage: Add microsecond boot time measurement
Date: Sun, 8 Jan 2012 09:42:02 -0800	[thread overview]
Message-ID: <CAPnjgZ3t5A0L+zEmyvyHbXGnJadmucnkxe24y2FBF_xvDh4atA@mail.gmail.com> (raw)
In-Reply-To: <201201080335.31904.vapier@gentoo.org>

Hi Mike,

On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
>> This defines the basics of a new boot time measurement feature. This allows
>> logging of very accurate time measurements as the boot proceeds, by using
>> an available microsecond counter.
>>
>> To enable the feature, define CONFIG_BOOTSTAGE in your board config file.
>> Also available is CONFIG_BOOTSTAGE_REPORT which will cause a report to be
>> printed just before handing off to the OS.
>
> the summary says "Add microsecond boot time measurement". ?that sounds like
> extending existing functionality. ?in reality, this is the core of the new
> CONFIG_BOOTSTAGE logic right ?

Yes - I will update the commit title.

>
> also, this logging framework seems to overlap the existing POST logging
> framework functionality ...

Please see previous message - I have integrated show_boot_progress()
as per previous comments, But power-on self test has some separate
functionality. It does call show_boot_progress() on failure.

This series is about the U-Boot side of supporting boot timing from
reset to Linux user space.

>
>> --- a/include/bootstage.h
>> +++ b/include/bootstage.h
>>
>> +static inline ulong bootstage_mark(enum bootstage_id id)
>> ?{
>> - ? ? show_boot_progress(-val);
>> +#ifdef CONFIG_SHOW_BOOT_PROGRESS
>> + ? ? show_boot_progress(id);
>> +#endif
>> + ? ? return 0;
>> ?}
>>
>> +static inline ulong bootstage_error(enum bootstage_id id)
>> +{
>> +#ifdef CONFIG_SHOW_BOOT_PROGRESS
>> + ? ? show_boot_progress(-id);
>> +#endif
>> + ? ? return 0;
>> +}
>
> why isn't show_boot_progress() just a stub when CONFIG_SHOW_BOOT_PROGRESS
> isn't defined ? ?then you don't have to protect the call sites.

show_boot_progress() has been part of U-Boot for a while. Quite a lot
of boards define this function with the expectation that they can turn
CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
you suggest I will break that expectation.

One fix would be to bracket all show_boot_progress() function
implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
haven't done that.

>
>> +static inline ulong bootstage_mark_name(enum bootstage_id id, const char
>> *name)
>> +{ return 0; }
>
> please unroll this

done

> -mike

Regards,
Simon