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

[Ken.Fuchs at bench.com]

Jean-Christophe PLAGNIOL-VILLARD wrote:

> >       /* choose RMII or MII mode. This depends on the board */
> >  #ifdef CONFIG_RMII
> >  #if defined(CONFIG_AT91CAP9) || defined(CONFIG_AT91SAM9260) || \
> > -    defined(CONFIG_AT91SAM9263)
> > +    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20)
> >       macb_writel(macb, USRIO, MACB_BIT(RMII) | MACB_BIT(CLKEN));
> >  #else
> >       macb_writel(macb, USRIO, 0);
> >  #endif

> Same comment as Haavard,
> Could please create a CONFIG_SOMETHING for each of this tree block of
> ifdef (not necessarily the same CONFIG)

What do you suggest for <SOMETHING> in CONFIG_SOMETHING?
Surely you aren't suggesting a literal "CONFIG_SOMETHING".

The fact that you didn't easily define a meaningful <SOMETHING>
suggests that there isn't one that is sufficiently meaningful
and that perhaps the #if condition as written above should
remain as is.  Code should be written for readability, not for
terseness alone.  Do we have to define yet another preprocessor
constant for yet another purpose?  U-Boot already has too many
preprocessor constants and the addition of another (perhaps)
dubious one merits more debate.

Sincerely,

Ken Fuchs