[U-Boot] [PATCH 2/5] [PATCH 2/5] Add support for the armadeus APF27 board

[Stefano Babic]

On 28/06/2012 22:36, Philippe Reynes wrote:
> Signed-off-by: Philippe Reynes <tremyfr at yahoo.fr>
> Signed-off-by: Eric Jarrige <eric.jarrige at armadeus.org>
> Signed-off-by: Nicolas Colombain <nicolas.colombain at armadeus.com>
> ---

Hi Philippe,



> +	rm -f $(SOBJS) $(OBJS)
> +
> +distclean:	clean
> +	rm -f $(LIB) core *.bak $(obj).depend

clean / distclean are not needed, drop these rules

> +#include <common.h>
> +#include "crc.h"
> +#include <jffs2/jffs2.h>
> +#include <nand.h>
> +#include <netdev.h>
> +#include <asm/io.h>
> +#include <asm/arch/imx-regs.h>
> +#include <asm/errno.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +u32 get_board_rev(void)
> +{
> +	struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
> +
> +	return readl(&iim->bank[1].fuse_regs[8]);

For my understanding: who does store the version number in the fuses ?
The board's manufacturer ?

> +int get_num_ram_bank(void)
> +{
> +	struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
> +	int nr_dram_banks = 1;
> +
> +	if ((get_board_rev() > 0) && (CONFIG_NR_DRAM_BANKS > 1))
> +		nr_dram_banks += readl(&iim->bank[1].fuse_regs[9]) & 0x01;

It seems to me there are some hidden important information in the fuses.
Maybe can you document them adding a map of the relevant fuses for this
board ?

> +	/* Unlock whole flash but U-Boot */
> +	s = getenv("env_offset");
> +	offset = CONFIG_ENV_OFFSET;
> +	if ((s != NULL) && (0 != strcmp(s, "0")))
> +		offset = simple_strtoul(s, NULL, 16);
> +
> +	if (nand_unlock(&nand_info[0], offset, nand_info[0].size - offset))
> +		printf("NAND flash lock/unlocked failed\n");

Why is it needed ?

> +void
> +show_boot_progress(int status)
> +{
> +#ifdef CONFIG_SHOW_BOOT_PROGRESS
> +#endif
> +	return;
> +}

This is quite dead code..

> +#if defined(CONFIG_FEC_MXC)
> +	struct eth_device *dev;
> +	uchar eth_addr[6];
> +
> +	rc = fecmxc_initialize(bis);
> +
> +	if (!eth_getenv_enetaddr(STR_ENV_ETHADDR, eth_addr)) {
> +		dev = eth_get_dev_by_index(0);
> +		if (dev) {
> +			eth_setenv_enetaddr(STR_ENV_ETHADDR, dev->enetaddr);
> +		} else {
> +			printf("FEC APF27: Couldn't get eth device\n");
> +			rc = -1;
> +		}

It is not clear to me why you have your special way to set up the MAC
address instead of rely on the already working code. The FEC is already
initialized for i.MX27 by general SOC code, see imx27lite.

> +
> +void enable_caches(void)
> +{
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +	/* Enable D-cache. I-cache is already enabled in start.S */
> +	dcache_enable();
> +#endif

This is SOC-related, and not board related. It belongs to the SOC code.

> diff --git a/board/armadeus/apf27/config.mk b/board/armadeus/apf27/config.mk
> new file mode 100644
> index 0000000..9713e69
> --- /dev/null
> +++ b/board/armadeus/apf27/config.mk
> @@ -0,0 +1,38 @@
> +#
> +# (C) Copyright 2003
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> +# (C) Copyright 2012
> +# Eric Jarrige <eric.jarrige at armadeus.org>
> +#
> +# 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
> +#
> +
> +#
> +# This config file is used for compilation of armadeus sources
> +#
> +# You might change location of U-Boot in memory by setting right TEXT_BASE.
> +# This allows for example having one copy located at the end of ram and stored
> +# in flash device and later on while developing use other location to test
> +# the code in RAM device only.
> +#
> +
> +ifndef TEXT_BASE
> +TEXT_BASE = 0xAFF00000
> +endif

config.mk is obsolete and must not be added for new boards. TEXT_BASE
must be set in the configuration file, see CONFIG_SYS_TEXT_BASE.

> diff --git a/board/armadeus/apf27/lowlevel_init.S b/board/armadeus/apf27/lowlevel_init.S
> +
> diff --git a/boards.cfg b/boards.cfg
> index a723f67..d77ce77 100644
> --- a/boards.cfg
> +++ b/boards.cfg
> @@ -69,6 +69,7 @@ integratorcp_cm926ejs        arm         arm926ejs   integrator          armltd
>  aspenite                     arm         arm926ejs   -                   Marvell        armada100
>  gplugd                       arm         arm926ejs   -                   Marvell        armada100
>  afeb9260                     arm         arm926ejs   -                   -              at91
> +apf27                        arm         arm926ejs   apf27               armadeus       mx27

The board are grouped for SOC, and then sorted alphabetically. The entry
for your board should go before the imx27lite / magnesium boards.

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================