[U-Boot] [PATCH 4/7] Adding uC5272 dimm module support

[David Wu <davidwu@arcturusnetworks.com>]

Hi Wolfgang,

On Fri, 09 Apr 2010 18:54:33 -0400, Wolfgang Denk <wd@denx.de> wrote:

> Dear "David Wu",
>
> In message <op.vatgy5upqigx4y@cyprus.local> you wrote:
>> Signed-off-by: David Wu <davidwu@arcturusnetworks.com>
>> ---
>>   Makefile                         |   46 +++++
>>   board/Arcturus/uC5272/Makefile   |   44 +++++
>>   board/Arcturus/uC5272/config.mk  |    1 +
>>   board/Arcturus/uC5272/u-boot.lds |  142 +++++++++++++++
>>   board/Arcturus/uC5272/uC5272.c   |   57 ++++++
>>   include/configs/uC5272.h         |  354
>> ++++++++++++++++++++++++++++++++++++++
>>   6 files changed, 644 insertions(+), 0 deletions(-)
>>   create mode 100644 board/Arcturus/uC5272/Makefile
>>   create mode 100644 board/Arcturus/uC5272/config.mk
>>   create mode 100644 board/Arcturus/uC5272/u-boot.lds
>>   create mode 100644 board/Arcturus/uC5272/uC5272.c
>>   create mode 100644 include/configs/uC5272.h
>>
>> diff --git a/Makefile b/Makefile
>> index 1b61049..c9215d0 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2222,6 +2222,52 @@ M5485HFE_config :	unconfig
>>   TASREG_config :		unconfig
>>   	@$(MKCONFIG) $(@:_config=) m68k mcf52x2 tasreg esd
>>
>> +uC5272-4E16U48_config \
>> +uC5272-8EE16U66_config \
>> +uC5272-8E32U66_config \
>> +uC5272-4EE16U48_config \
>> +uC5272-4E8U48_config \
>> +uC5272-4EE8U48_config \
>> +uC5272-4E8U66_config \
>> +uC5272-4EE8U66_config \
>> +uC5272-4E16U66_config \
>> +uC5272-4EE16U66_config \
>> +uC5272-4EE32U66_config \
>> +uC5272-8EE32U66_config:		unconfig
>> +	@mkdir -p $(obj)include
>> +	@if [ "$(findstring U48,$@)" ] ; then \
>> +		echo "#define SYSCLK_48MHZ " >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@if [ "$(findstring U66,$@)" ] ; then \
>> +		echo "#define SYSCLK_66MHZ " >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@if [ "$(findstring EE,$@)" ] ; then \
>> +		echo "#define HAS_ETH1 " >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@if [ "$(findstring 4E,$@)" ] ; then \
>> +		echo "#define __4MFlash__ " >>$(obj)include/config.h ;\
>> +		echo "#define CONFIG_SYS_FLASH_SIZE 0x00400000"
>> >>$(obj)include/config.h ;\
>> +		echo "TEXT_BASE = 0x10c00000" > board/Arcturus/uC5272/config.mk ;\
>> +	fi ;
>> +	@if [ "$(findstring 8E,$@)" ] ; then \
>> +		echo "#define __8MFlash__ " >>$(obj)include/config.h ;\
>> +		echo "#define CONFIG_SYS_FLASH_SIZE 0x00800000"
>> >>$(obj)include/config.h ;\
>> +		echo "TEXT_BASE = 0x40000000" > board/Arcturus/uC5272/config.mk ;\
>> +	fi ;
>> +	@if [ "$(findstring E8,$@)" ] ; then \
>> +		echo "#define __8MRam__ " >>$(obj)include/config.h ;\
>> +		echo "#define CONFIG_SYS_SDRAM_SIZE 8" >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@if [ "$(findstring E16,$@)" ] ; then \
>> +		echo "#define __16MRam__ " >>$(obj)include/config.h ;\
>> +		echo "#define CONFIG_SYS_SDRAM_SIZE 16" >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@if [ "$(findstring E32,$@)" ] ; then \
>> +		echo "#define __32MRam__ " >>$(obj)include/config.h ;\
>> +		echo "#define CONFIG_SYS_SDRAM_SIZE 32" >>$(obj)include/config.h ;\
>> +	fi ;
>> +	@$(MKCONFIG) -a uC5272 m68k mcf52x2 uC5272 Arcturus
>> +
>
> NAK!
>
>
> You must be joking.
>
> We will not accept such a mess of scriting in the top level Makefile.
It was/is a mess already. I just followed the exact top level Makefile.
If it is not acceptable then I'd like to know if it is OK to
  -- make one  separate header file in include/configs for each board
  -- and one config.mk per board
-- or other methods -- advise please
>
> Also, I don't understand why adding DIMM module support would result
> in a new config.mk file and a new linker script being added?
TEXT_BASE is changed due to changes in the memory map for different DIMM  
modules with different size of SDRAM and FLASH.
TEXT_BASE is defined in config.mk - I thought this is the only location  
that really matters. Other boards may using another temporary file and  
included in config.mk. For me why bother to modify config.mk file  
directly. If there is a better choice I would like to use.
One way to do is each board has a unique config.mk.

> The additions to the board config file are an unacceptable mess, too.
You mean you cannot understand the ifdefs so you think it is a mess? But I  
feel it is so neat and it supports many different configurations for  
uCdimm 5272 modules.
I also can create many of these files with a few lines differences by  
each. How about this?

> This needs a complete rework.
Sure. I need more feed back. then I will.

thanks,
David

> Best regards,
>
> Wolfgang Denk
>