[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