[U-Boot-Users] [PATCH 1/1] Add support for ATMELAT91SAM9G20EK board

[Ken.Fuchs at bench.com]

> <Ken.Fuchs at bench.com> wrote:
> 
> > U-Boot already has too many
> > preprocessor constants and the addition of another (perhaps)
> > dubious one merits more debate.

You omitted the context of this statement and hence most of its meaning.

Haavard Skinnemoen wrote:

> I don't completely agree. U-Boot has too many #ifdefs, which isn't
> necessarily the same as too many #defines. And I don't think
> CONFIG_AT91 is dubious at all.

Perhaps the CONFIG_* symbols should be defined as TRUE or FALSE rather
than defined or not defined.

I didn't say CONFIG_AT91 was dubious.  I stated that defining
CONFIG_<SOMETHING> where it isn't clear what <SOMETHING> should
be is dubious.  If one has to think too hard about what meaningful
phrase <SOMETHING> should be, perhaps CONFIG_<SOMETHING> should
not be defined at all.  I'm complaining about defining new
preprocessor definitions to be specific aggregations of other
definitions just so a developer can type the conditions with
fewer keystrokes without any improvement in code readability
and maintainability.  Sometimes an idiom should be left as an
idiom rather than replaced by a preprocessor constant.

I have nothing against defining CONFIG_AT91 if it is useful,
unique and makes code that uses it easier to understand and
maintain.

> But since we already have a CONFIG_AVR32 #define, we can clean
> up the mess in macb.c by simply reversing the logic.

If CONFIG_AVR32 can be used in macb.c without ofuscation, why is
CONFIG_AT91 needed here?  However, "simply reversing the logic"
may be too much ofuscation though; we want clear rather than
clever code after all.

An example of what I'd be opposed to is defining
CONFIG_AT91SAM9260_OR_AT91SAM9263 where it is TRUE if either
CONFIG_AT91SAM9260 or CONFIG_AT91SAM9263 are TRUE.
Can you see where this might be used in the macb.c code?

Sincerely,

Ken Fuchs