[U-Boot-Users] [PATCH] Add support for the MPC8349E-mITX-GP

[Timur Tabi]

Kim Phillips wrote:
> On Fri, 26 Jan 2007 10:46:54 +0100
> Wolfgang Denk <wd at denx.de> wrote:
> 
>> In message <BB798853-8000-4747-A6BD-26E5E71AD80B at kernel.crashing.org> you wrote:
>>> Since they are different physical boards they should have different  
>>> <config>.h, that rule's been pretty standard in u-boot.
>> No, not at all. We have many similar boards share  one  configuration
>> file  -  especially  in  the  case  of  modules  that  can be used on
>> different carrier boards.
>>
> I didn't see any that had the same config behaviour as this patch does (ifdeffing itself). 

Some boards solve this problem by specifying the options in the Makefile. 
However, Wolfgang says he doesn't like this.

> My reluctance is stemmed from the prediction of the growing number of ifdefs in the config file.  Perhaps the best thing to do here is have the main config file have a single set of ifdefs for each individual board, and include that board's delta config from a sub-board config file.  All common elements remain in the main config file, and if a board define is not matched, #error out.

I presume you mean board-specific ifdefs in a config file, and not just any 
ifdefs.  Frankly, I don't think 3 board-specific ifdefs is that many.

Technically speaking, the config file for the ITX will work on the ITX-GP, but 
you'll have to deal with:

1) A warning stating that flash bank #2 is missing
2) Loading support for compact flash and IDE, and a message saying that IDE 
times out.
3) A command prompt that says "MPC8349E-mITX" instead of "MPC8349E-mITX-GP"

I could modify the header file to make it somewhat more generic, but I can't 
completely eliminate "#ifdef CONFIG_MPC8349ITX" unless I move some config stuff 
to the Makefile, by adding something like this to the make rule:

	[if ITX and not ITX-GP]
	echo "#define CONFIG_COMPACT_FLASH" >> $(obj)include/config.h
	echo "#define CFG_MAX_FLASH_BANKS 2" >> $(obj)include/config.h
	echo '#define CFG_PROMPT "MPC8349E-mITX> "' >> $(obj)include/config.h

And then use this code in the config file to handle the flash configuration:

#define CFG_FLASH_SIZE		(CFG_MAX_FLASH_BANKS * 8)
#define CFG_MAX_FLASH_BANKS	(2 + CFG_MAX_FLASH_BANKS)
#define CFG_FLASH_BANKS_LIST 	{CFG_FLASH_BASE, CFG_FLASH_BASE + 0x800000}

This produces this warning when compiling cfi_flash.c, however:

cfi_flash.c:167: warning: excess elements in array initializer
cfi_flash.c:167: warning: (near initialization for 'bank_base')

To fix that, we could modify cfi_flash.c like this:

-static ulong bank_base[CFG_MAX_FLASH_BANKS] = CFG_FLASH_BANKS_LIST;
+static ulong bank_base[] = CFG_FLASH_BANKS_LIST;

Again, Wolfgang says he doesn't like options in Makefiles, although personally I 
don't have a problem with that.  I just mention the above for purposes of 
discussion.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale