[U-Boot,2/3] arm: omap: innovator: Prepare for mach-types.h changes

This board used machine_is_* macros for identifying the arch number.
Use compile time defines instead.

Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
Cc: Sandeep Paulraj <s-paulraj@ti.com>
Cc: Nishant Kamat <nskamat@ti.com>
Cc: Kshitij Gupta <kshitij@ti.com>
---
This has been compile tested after the 1/3 patch is applied.

 board/ti/omap1610inn/omap1610innovator.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

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

On 05/01/11 23:28, Alessandro Rubini wrote:

> 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.

Exactly, this is the problem...
Please, read:
http://www.mail-archive.com/u-boot@lists.denx.de/msg51265.html

> 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?

This never was...
I agree, but it at least the board won't break when the mach-types.h is cut
down the way explained in this thread, so please, read the thread.

I can make another patch which will not convert the if into ifdef
(either way is compile time expanded), but introduce machine_is_omap_h2()
and machine_is_omap_innovator() macros definition in a board specific .h file.
Do you think it will be a better solution?

> 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.

This is a compile time expanded macros.

> 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".

That is the problem... We want to move away from Russell's mach-types.h
as it gets cut down to only machines supported by mainline Linux kernel
and apparently does not suit U-Boot needs.

> 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.

Please, read the thread...