[U-Boot] [PATCH 2/3 RESEND] devkit3250: add Timll DevKit3250 board initial support

[Vladimir Zapolskiy <vz@mleia.com>]

Hi Igor,

thank you for your review, especially because it's a sole one.

On 08.11.2011 10:16, Igor Grinberg wrote:
> Hi Vladimir,
>
> On 11/07/11 23:58, Vladimir Zapolskiy wrote:
>> This change adds a basic support for Embest/Timll DevKit3250 board,
>> NOR and UART are the only supported peripherals for a moment. The board
>> doesn't require low-level init, because the initial SDRAM and GPIO
>> configuration is performed during kickstart bootloader execution.
>>
>> Signed-off-by: Vladimir Zapolskiy<vz@mleia.com>
>> ---
>>   board/timll/devkit3250/Makefile     |   50 ++++++++++++++
>>   board/timll/devkit3250/devkit3250.c |   74 +++++++++++++++++++++
>>   boards.cfg                          |    3 +-
>>   include/configs/devkit3250.h        |  121 +++++++++++++++++++++++++++++++++++
>>   4 files changed, 247 insertions(+), 1 deletions(-)
>>   create mode 100644 board/timll/devkit3250/Makefile
>>   create mode 100644 board/timll/devkit3250/devkit3250.c
>>   create mode 100644 include/configs/devkit3250.h
>>
>> diff --git a/board/timll/devkit3250/Makefile b/board/timll/devkit3250/Makefile
>> new file mode 100644
>> index 0000000..99cb16f
>> --- /dev/null
>> +++ b/board/timll/devkit3250/Makefile
>> @@ -0,0 +1,50 @@
>> +#
>> +# Copyright (C) 2011 by Vladimir Zapolskiy<vz@mleia.com>
>> +# Copyright (C) 2008, Guennadi Liakhovetski<lg@denx.de>
>> +#
>> +# See file CREDITS for list of people who contributed to this
>> +# project.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation; either version 2 of
>> +# the License, or (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> +# MA 02111-1307 USA
>
> Probably, it will be a good thing to drop the address part globally,
> as it can change and you may not be around to look for your
> files to change it.

I checked the current license from gnu.org, and yes, it has another 
address in its body. I corrected the address in newly introduced files, 
but globally in my opinion it should be one mass change of about 5000 
files though.

>> +#
>> +
>> +include $(TOPDIR)/config.mk
>> +
>> +LIB	= $(obj)lib$(BOARD).o
>> +
>> +COBJS	:= devkit3250.o
>> +
>> +SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
>> +OBJS	:= $(addprefix $(obj),$(COBJS))
>> +SOBJS	:= $(addprefix $(obj),$(SOBJS))
>> +
>> +$(LIB):	$(obj).depend $(OBJS) $(SOBJS)
>> +	$(call cmd_link_o_target, $(OBJS) $(SOBJS))
>> +
>> +clean:
>> +	rm -f $(SOBJS) $(OBJS)
>> +
>> +distclean:	clean
>> +	rm -f $(LIB) core *.bak .depend
>
> clean and distclean have no use in this directory level
> and have been removed all over by Mike,
> so please don't add any new ones.
>
Removed in the next version, thanks.

>> +
>> +#########################################################################
>> +
>> +# defines $(obj).depend target
>> +include $(SRCTREE)/rules.mk
>> +
>> +sinclude $(obj).depend
>> +
>> +#########################################################################
>> diff --git a/board/timll/devkit3250/devkit3250.c b/board/timll/devkit3250/devkit3250.c
>> new file mode 100644
>> index 0000000..b2f7863
>> --- /dev/null
>> +++ b/board/timll/devkit3250/devkit3250.c
>> @@ -0,0 +1,74 @@
>> +/*
>> + * Embest/Timll DevKit3250 board support
>> + *
>> + * Copyright (C) 2011 Vladimir Zapolskiy<vz@mleia.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
>> + * MA 02110-1301, USA.
>> + */
>> +
>> +#include<common.h>
>> +#include<asm/arch/sys_proto.h>
>> +#include<asm/arch/cpu.h>
>> +#include<asm/arch/emc.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static struct emc_t *emc = (struct emc_t *)EMC_BASE;
>> +
>> +int board_early_init_f(void)
>> +{
>> +	lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
>> +
>> +	return 0;
>> +}
>> +
>> +int board_init(void)
>> +{
>> +	/*
>> +	 * It might be necessary to flush data cache, if U-boot is loaded
>> +	 * from kickstart bootloader, e.g. from S1L loader
>> +	 */
>> +	flush_cache(0, 0);
>> +
>> +	/* set machine id of Embest DevKit3250 */
>> +	gd->bd->bi_arch_number  = MACH_TYPE_DEVKIT3250;
>
> You already setup the CONFIG_MACH_TYPE.
> The above line should be removed (along with the comment).
>
Removed.

>> +
>> +	/* adress of boot parameters */
>> +	gd->bd->bi_boot_params  = CONFIG_ENV_ADDR;
>> +
>> +#ifdef CONFIG_SYS_FLASH_CFI
>> +	/* Use 16-bit memory interface for NOR Flash */
>> +	emc->stat[0].config     = EMC_STAT_CONFIG_PB | EMC_STAT_CONFIG_16BIT;
>> +
>> +	/* Change the NOR timings to optimum value to get maximum bandwidth */
>> +	emc->stat[0].waitwen    = EMC_STAT_WAITWEN(1);
>> +	emc->stat[0].waitoen    = EMC_STAT_WAITOEN(1);
>> +	emc->stat[0].waitrd     = EMC_STAT_WAITRD(12);
>> +	emc->stat[0].waitpage   = EMC_STAT_WAITPAGE(12);
>> +	emc->stat[0].waitwr     = EMC_STAT_WAITWR(5);
>> +	emc->stat[0].waitturn   = EMC_STAT_WAITTURN(2);
>
> Indentation of all the above should be done with tabs.
>
No objections here, done.

[snip]

-- 
With best wishes,
Vladimir