[U-Boot] [PATCH] Add support for the digsy MTC board.

[Stefan Roese]

On Thursday 12 February 2009, Grzegorz Bernacki wrote:
> This is the InterControl custom device based on the MPC5200B chip.
>
> Signed-off-by: Grzegorz Bernacki <gjb at semihalf.com>

Please find some comments below.

> diff --git a/board/digsy_mtc/digsy_mtc.c b/board/digsy_mtc/digsy_mtc.c
> new file mode 100644
> index 0000000..0cde8a8
> --- /dev/null
> +++ b/board/digsy_mtc/digsy_mtc.c
> @@ -0,0 +1,333 @@
> +/*
> + * (C) Copyright 2003
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * (C) Copyright 2004
> + * Mark Jonas, Freescale Semiconductor, mark.jonas at motorola.com.
> + *
> + * (C) Copyright 2005-2009
> + * Modified for InterControl digsyMTC MPC5200 board by
> + * Frank Bodammer, GCD Hard- & Software GmbH,
> + *                 frank.bodammer at gcd-solutions.de
> + *
> + * (C) Copyright 2009
> + * Grzegorz Bernacki, Semihalf, gjb at semihalf.com
> + *
> + * 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
> + */
> +
> +#include <common.h>
> +#include <mpc5xxx.h>
> +#include <pci.h>
> +#include <asm/processor.h>
> +#include <i2c.h>
> +#include "eeprom.h"
> +#include "is42s16800a-7t.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +extern int usb_cpu_init(void);
> +
> +#ifndef CONFIG_SYS_RAMBOOT
> +static void sdram_start(int hi_addr)
> +{
> +	long hi_addr_bit = hi_addr ? 0x01000000 : 0;
> +	long control = SDRAM_CONTROL | hi_addr_bit;
> +
> +	/* unlock mode register */
> +	out32(MPC5XXX_SDRAM_CTRL, control | 0x80000000);

Please use the out_be32()/in_be32() accessor functions for accessing SoC 
perepherals instead. They have the io-barriers included. You could git into 
trouble by "just" using out/in32(). This is a general comment meant for the 
complete patch.

> +
> +	/* precharge all banks */
> +	out32(MPC5XXX_SDRAM_CTRL, control | 0x80000002);
> +
> +	/* auto refresh */
> +	out32(MPC5XXX_SDRAM_CTRL, control | 0x80000004);
> +
> +	/* set mode register */
> +	out32(MPC5XXX_SDRAM_MODE, SDRAM_MODE);
> +
> +	/* normal operation */
> +	out32(MPC5XXX_SDRAM_CTRL, control);
> +}
> +#endif
> +
> +/*
> + * ATTENTION: Although partially referenced initdram does NOT make real
> use + *            use of CONFIG_SYS_SDRAM_BASE. The code does not work if
> + *            CONFIG_SYS_SDRAM_BASE is something else than 0x00000000. +
> */
> +
> +phys_size_t initdram(int board_type)
> +{
> +	ulong dramsize = 0;
> +	ulong dramsize2 = 0;
> +	uint svr, pvr;
> +#ifndef CONFIG_SYS_RAMBOOT
> +	ulong test1, test2;
> +
> +	/* setup SDRAM chip selects */
> +	out32(MPC5XXX_SDRAM_CS0CFG, 0x0000001C);	/* 512MB at 0x0 */
> +	out32(MPC5XXX_SDRAM_CS1CFG, 0x80000000);	/* disabled */
> +
> +	/* setup config registers */
> +	out32(MPC5XXX_SDRAM_CONFIG1, SDRAM_CONFIG1);
> +	out32(MPC5XXX_SDRAM_CONFIG2, SDRAM_CONFIG2);
> +
> +	/* find RAM size using SDRAM CS0 only */
> +	sdram_start(0);
> +	test1 = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE, 0x08000000);
> +	sdram_start(1);
> +	test2 = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE, 0x08000000);
> +	if (test1 > test2) {
> +		sdram_start(0);
> +		dramsize = test1;
> +	} else {
> +		dramsize = test2;
> +	}
> +
> +	/* memory smaller than 1MB is impossible */
> +	if (dramsize < (1 << 20)) {
> +		dramsize = 0;
> +	}

Nitpicking: Remove the curly braces for single-line statements.

> +
> +	/* set SDRAM CS0 size according to the amount of RAM found */
> +	if (dramsize > 0) {
> +		out32(MPC5XXX_SDRAM_CS0CFG,
> +			(0x13 + __builtin_ffs(dramsize >> 20) - 1));
> +	} else {
> +		out32(MPC5XXX_SDRAM_CS0CFG, 0); /* disabled */
> +	}
> +
> +	/* let SDRAM CS1 start right after CS0 */
> +	out32(MPC5XXX_SDRAM_CS1CFG, dramsize + 0x0000001C); /* 512MB */
> +
> +	/* find RAM size using SDRAM CS1 only */
> +	test1 = get_ram_size((long *)(CONFIG_SYS_SDRAM_BASE + dramsize),
> +			0x08000000);
> +		dramsize2 = test1;
> +
> +	/* memory smaller than 1MB is impossible */
> +	if (dramsize2 < (1 << 20)) {
> +		dramsize2 = 0;
> +	}

Again. Not curly braces here.

Best regards,
Stefan

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