Hi Mike,
On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger <vap...@gentoo.org> 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?
The series as is was tested MAKEALL clean.
Regards,
Simon
> -mike
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
