[U-Boot] [RFC PATCH v2 01/15] bootstage: Create an initial header for boot progress integers

[Simon Glass]

Hi Mike,

On Sun, Jan 8, 2012 at 12:26 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Saturday 10 December 2011 16:07:53 Simon Glass wrote:
>> --- /dev/null
>> +++ b/include/bootstage.h

Thanks for looking at this.

>>
>> +/*
>> + * This file implements recording of each stage of the boot process. It is
>> + * intended to implement timing of each stage, reporting this information
>> + * to the user and passing it to the OS for logging / further analysis.
>> + */
>
> maybe it's me, but i'd expect this at the top of the file before the
> copyright/license notice

done

>
>>
>> + * progres action1
>
> typo ?  shows up a few times ...

done

>
>> +enum bootstage_id {
>> +     BOOTSTAGE_ID_RUN_OS     = 15,   /* Exiting U-Boot, entering OS */
>> +};
>
> what relevance does this # have ?  since it gets passed to Linux, it becomes
> part of the ABI and cannot be changed, so that should be mentioned in the
> comment.

The number is currently open-coded in U-Boot but this series turns all
these occurrences into a global enum. Actually I don't plan that the
number be passed to Linux, or at least that Linux can rely on it being
anything in particular.

I haven't done it in this series (which is large enough already) as
this is a code translation exercise and I want it to be easy to see
whether I have treated each site correctly.

But I want to remove all the numbers later, so that we can add new
stages easily and everything is numbered from 0 to n with no gaps.

>
>> +/*
>> + * Board-specific platform code can implement show_boot_progress () if
>
> "board-specific platform" is kind of redundant
>
> also, no space before "()"

done

>
>> --- a/include/common.h
>> +++ b/include/common.h
>> @@ -801,10 +801,8 @@ int      pcmcia_init (void);
>>  #ifdef CONFIG_STATUS_LED
>>  # include <status_led.h>
>>  #endif
>> -/*
>> - * Board-specific Platform code can reimplement show_boot_progress () if
>> needed - */
>> -void show_boot_progress(int val);
>> +
>> +#include <bootstage.h>
>>
>>  /* Multicore arch functions */
>>  #ifdef CONFIG_MP

Regards,
Simon