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

[Simon Glass <sjg@chromium.org>]

Hi Mike,

On Sat, Jan 14, 2012 at 5:09 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 12 January 2012 00:41:24 Simon Glass wrote:
>> On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger wrote:
>> > On Sunday 08 January 2012 12:42:02 Simon Glass wrote:
>> >> On Sun, Jan 8, 2012 at 12:35 AM, Mike Frysinger wrote:
>> >> > On Saturday 10 December 2011 16:08:05 Simon Glass wrote:
>> >> >> --- a/include/bootstage.h
>> >> >> +++ b/include/bootstage.h
>> >> >>
>> >> >> +static inline ulong bootstage_mark(enum bootstage_id id)
>> >> >> ?{
>> >> >> - ? ? show_boot_progress(-val);
>> >> >> +#ifdef CONFIG_SHOW_BOOT_PROGRESS
>> >> >> + ? ? show_boot_progress(id);
>> >> >> +#endif
>> >> >> + ? ? return 0;
>> >> >> ?}
>> >> >>
>> >> >> +static inline ulong bootstage_error(enum bootstage_id id)
>> >> >> +{
>> >> >> +#ifdef CONFIG_SHOW_BOOT_PROGRESS
>> >> >> + ? ? show_boot_progress(-id);
>> >> >> +#endif
>> >> >> + ? ? return 0;
>> >> >> +}
>> >> >
>> >> > why isn't show_boot_progress() just a stub when
>> >> > CONFIG_SHOW_BOOT_PROGRESS isn't defined ? ?then you don't have to
>> >> > protect the call sites.
>> >>
>> >> show_boot_progress() has been part of U-Boot for a while. Quite a lot
>> >> of boards define this function with the expectation that they can turn
>> >> CONFIG_SHOW_BOOT_PROGRESS on and off independently. So If I do what
>> >> you suggest I will break that expectation.
>> >>
>> >> One fix would be to bracket all show_boot_progress() function
>> >> implementations in the boards with CONFIG_SHOW_BOOT_PROGRESS, but I
>> >> haven't done that.
>> >
>> > it seemed like part of your clean up series was to merge
>> > show_boot_progress() into your new bootstage framework. ?in which case,
>> > we have full control over it now, and ifdef bracketing for it should go
>> > away ...
>>
>> Still don't quite get it though. For example, the beagle board defines
>> show_boot_progress() but does not define CONFIG_SHOW_BOOT_PROGRESS, so
>> wouldn't that break that board?
>
> that sounds like an odd-man-out that needs fixing rather than allowing to live

Fair enough. although I suspect there will be many. If I could
actually get a MAKEALL to run without producing 100s of broken boards
then it would be easier to do this sort of thing. At the moment it's
like looking for a needle in a haystack. My warnings series aimed to
improve things slightly, but I don't think others have these problems.

Regards,
Simon

> -mike