[U-Boot-Users] [PATCH] fix compilation problem for mpc8349itx CFG_RAMBOOT

[Timur Tabi]

Wolfgang Denk wrote:
> In message <46547E55.3020608 at freescale.com> you wrote:
>> How about this approach instead:
>>
>> #ifdef CFG_NO_FLASH
>> #define CONFIG_COMMANDS_FLASH	~(CFG_CMD_FLASH | CFG_CMD_IMLS)
>> #else
>> #define CONFIG_COMMANDS_FLASH	~0
>> #endif
>>
>> #define CONFIG_COMMANDS		(CONFIG_CMD_DFL | \
>> 				CONFIG_COMMANDS_CF	| \
>> 				CFG_CMD_NET	| \
>> 				CFG_CMD_PING	| \
>> 				CONFIG_COMMANDS_I2C	| \
>> 				CONFIG_COMMANDS_PCI	| \
>> 				CFG_CMD_SDRAM	| \
>> 				CFG_CMD_DATE	| \
>> 				CFG_CMD_CACHE	| \
>> 				CFG_CMD_IRQ)	& \
>> 				CONFIG_COMMANDS_FLASH
> 
> This is extremely inconsistent. Some CONFIG_COMMANDS_* (PCI, I2C) are
> used to add features, while others (FLASH) are used to remove
> features. I consinder this very error prone.

Well, it's supposed to be an alternative to what Nikita was proposing.  I wanted to keep 
the concept of CONFIG_CMD_DFL + changes, rather than define CONFIG_CMD_DEFAULT and then 
make changes.

Please take a look at my MPC8349ITX.h to see the full extent of what I was trying to do. 
Nikita is just expanding on the code that's already there, and I'm just refining what 
Nikita is doing.  Basically, I want to avoid putting #define CONFIG_COMMANDS inside an 
ifdef.  As the number of configurable CFG_CMD_xxx options increase, the number of 
variations on CONFIG_COMMANDS grew exponentially.

> Also, you might consider using  better  (i.  e.  local)  preprocessor
> variable   names   for   such   functions.  Note  that  CONFIG_*  and
> CONFIG_COMMANDS* is kind of reserved by U-Boot, in the sense that  it
> has  a  pre-defined  meaning.  You  should avoid using such names for
> local variables; you might run into unpleasant  faiulures  sooner  or
> later.  In  general,  it  is  always  a good idea to avoid name space
> pollution.

Should they be renamed to CFG_COMMANDS_xxx?

-- 
Timur Tabi
Linux Kernel Developer @ Freescale