[U-Boot] [PATCH 1/3] arm: add CONFIG_MACH_TYPE option and documentation

[Albert ARIBAUD]

Le 07/07/2011 18:51, Igor Grinberg a écrit :

>>> If we have this option and it is documented, then any new board can use it
>>> instead of thinking (although it is simple) where and how to dereference
>>> the bi_arch_number.
>>
>> Not sure I get you there. Can you elaborate on a more precise example that would show the benefits of it?
>
> For example, if you think of Christopher's patch (ARM: Warn when the machine ID isn't set.),
> If you need Christopher's patch, then there are cases when the machid is not set, right?
> When someone gets this warning, he thinks: "Ah, I forgot the machid!" and then
> goes to fix the code, but again he thinks, where is the best place to put it?
> For us, it is trivial, that it should be in board_init() function, but for newbies, it is not that trivial.
> With this patch, you get the explanation and also a place to put the machid definition.
> With this patch, you just define the configuration "variable" and the whole thing will be done for you.
> Another example would be the board/nvidia/*, the code is shared as much as possible,
> and the mach_type is set in the common code. That is something I would expect to be done
> for all ARM boards, not just for nvidia...

I see your point.

Now the issue I foresee is that this commonalization has benefits only 
for boards which currently set their bi_arch_number in board_init_f(), 
but has no incentive -- that's a code that will be used only in a few 
places and could stay that way for quite long, because boards that will 
not adhere to it will still build unchanged.

IOW, there is no benefit for e.g. ED Mini V2, to use CONFIG_MACH_TYPE, 
so why would it? Thus instead of simplifying and commonalizing, this 
feature will *add* to the code base complexity.

Unless the goal is to add this macro *and* change all related board 
codes in the same patchset? I don't see it as feasible either.

Any suggestion for ensuring adoption of the feature wherever it can be used?

>>> Also, it can come in handy, in the configuration system (I think it is called Kbuild?),
>>> so the value can be chosen from the list or something.
>>
>> AFAICT, the Linux build system does fine with the generated mach-type.
>
> Here, I'm not talking about the mach_types, but about the config option.
> It can be set by the build system and no need to do it in board code!

Agreed.

Amicalement,
-- 
Albert.