[U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes
Alessandro Rubini
rubini-list at gnudd.com
Sun May 1 22:28:17 CEST 2011
I'm sorry for sounding rude, it's not my intention.
I didn't follow closely the discussion about mach_types.h, but I think
we are heading in the wrong direction.
For example, this patch:
> - if (machine_is_omap_h2())
> - gd->bd->bi_arch_number = MACH_TYPE_OMAP_H2;
> - else if (machine_is_omap_innovator())
> - gd->bd->bi_arch_number = MACH_TYPE_OMAP_INNOVATOR;
> - else
> - gd->bd->bi_arch_number = MACH_TYPE_OMAP_GENERIC;
> +#if defined(CONFIG_MACH_OMAP_H2)
> + gd->bd->bi_arch_number = MACH_TYPE_OMAP_H2;
> +#elif defined(CONFIG_MACH_OMAP_INNOVATOR)
> + gd->bd->bi_arch_number = MACH_TYPE_OMAP_INNOVATOR;
> +#else
> + gd->bd->bi_arch_number = MACH_TYPE_OMAP_GENERIC;
> +#endif
Since when turning if into ifdef has been a wise move for
maintainability?
The commis says:
> This board used machine_is_* macros for identifying the arch number.
> Use compile time defines instead.
But this already was compile-time: no code generated. But even if it
generated code, I prefer 3 run-time comparisons than 3 compile-time
ifdefs.
Note that mach_types.h, as designed by Russell King, already had
compile time selection, becuase if you selected one machine only (like
in u-boot), one of the "if" becomes compile-time-true and the other
ones become "0".
I see a lot of discussion about checkpatch compliance and cleanup-only
patches are being accepted; this goes in the opposite direction, for
no reason apparent to me.
thanks for your patience
/alessandro
More information about the U-Boot
mailing list