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

[Simon Glass]

Hi Mike,

On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger <vapier at 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