Hi Mike,
On Mon, Jan 9, 2012 at 9:33 AM, Mike Frysinger vapier@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