[U-Boot] [RESEND PATCH v2 1/5] Tegra2: Add macros to calculate bitfield shifts and masks

[Albert ARIBAUD]

Le 14/07/2011 19:29, Anton Staaf a écrit :
> On Thu, Jul 14, 2011 at 9:00 AM, Albert ARIBAUD
> <albert.u.boot at aribaud.net>  wrote:
>> Hi Anton,
>>
>> Le 13/07/2011 18:47, Anton Staaf a écrit :
>>
>>> I agree in general that it is preferable to be as explicit as
>>> possible.  But it is also good to be able to express your intent,
>>> instead of implementation when possible.  In other words, I would
>>> rather be explicit about my intent, than the particular
>>> implementation.
>>
>> Seems to me that for you, showing intent and implementation are necessarily
>> exclusive; however, Wolfgang's examples indeed show implementation, but they
>> show intent as well, at least for me they do.
>
> I'm not sure which example you mean.  If you mean his #define of the
> masks explicitly, those are fine by me.  My above statement is about
> the masking, oring and shifting that is done in the same way every
> time and could be encoded in a macro that makes it easier to see what
> exactly is going on.  Or did I misunderstand which example you mean?

You did not misunderstand the example -- but the way you just stated 
what you think of it is, I think, a confirmation of what I said: that it 
is your approach of the issue which is at odds with Wolfgang's and mine.

Let us look at the terms you've just use to describe what you dislike : 
these are 'masking, oring and shifting' and 'done the same way every 
time'. I assume the second is not a criticism, but the foundation for 
suggesting a macro.

That leaves 'masking, oring and shifting': it seems like for you it is 
unclear what this does, but it *does* tell what is going on just as much 
as a bitfield macro would -- actually it tells more, because '(x & y) | 
z' (there is usually no shifting involved, BTW) is a design pattern in 
embedded software development, where this pattern is recognized at first 
sight for what it does -- at least I see them that way and Wolfgang 
probably does as well. Granted, a non-specialist in embedded SW might 
have problems understanding this, but U-Boot has more or less a 
requirement that developers contributing to it have some knowledge of 
embedded SW.

The 'done in the same way every time' part, OTOH, might make sense -- 
that's code factorization, after all. But you could say the same of, for 
instance, assignments from structure members: they're done the same way 
every time : 'x = y.z', but there would be little point in wanting to 
hide that in a macro, because the macro would not add enough value.

I think that's the main problem: a bitfield macro for computing masks 
and shifts and anding and oring would not add sufficient value with 
respect to the bare expression, which is still simple enough to be 
understood by most readers.

(plus the issue of portability raised by Wolfgang, which I won't delve 
into as he's already developed it)

 > Thanks,
 >      Anton

HTH.

Amicalement,
-- 
Albert.