[U-Boot] [PATCH] board support patch for phyCORE-MPC5200B-tiny

Wolfgang Denk wd at denx.de
Sat Mar 21 21:32:45 CET 2009


Dear Jon Smirl,

In message <20090321182020.32596.55248.stgit at localhost> you wrote:
> Add support for the Phytec phyCORE-MPC5200B-tiny. This code is from Pengutronix.de but they
> didn't sign the patch. I have updated it from u-boot 1.2 to work on u-boot master.
> 
> Signed-off-by: Jon Smirl <jonsmirl at gmail.com>
> 
> Signed-off-by: Jon Smirl <jonsmirl at gmail.com>
> 
> ---
>  Makefile                                           |   16 +
>  board/phycore_mpc5200b_tiny/Makefile               |   50 ++
>  board/phycore_mpc5200b_tiny/config.mk              |   43 ++
>  board/phycore_mpc5200b_tiny/mt46v32m16-75.h        |   54 +++
>  .../phycore_mpc5200b_tiny/phycore_mpc5200b_tiny.c  |  307 ++++++++++++++
>  board/phycore_mpc5200b_tiny/u-boot.lds             |  123 ++++++
>  cpu/mpc5xxx/ide.c                                  |    3 
>  include/configs/phycore_mpc5200b_tiny.h            |  431 ++++++++++++++++++++
>  8 files changed, 1027 insertions(+), 0 deletions(-)
>  create mode 100644 board/phycore_mpc5200b_tiny/Makefile
>  create mode 100644 board/phycore_mpc5200b_tiny/config.mk
>  create mode 100644 board/phycore_mpc5200b_tiny/mt46v32m16-75.h
>  create mode 100644 board/phycore_mpc5200b_tiny/phycore_mpc5200b_tiny.c
>  create mode 100644 board/phycore_mpc5200b_tiny/u-boot.lds
>  create mode 100644 include/configs/phycore_mpc5200b_tiny.h

So  this  is  the  second  PHYTEC  board  we're  adding  (after   the
imx31_phycore),  and  I  think it's time to create a vendor directory
for them  -  please  prepare  a  board/phytec/  directory,  move  the
existing  imx31_phycore  code  there,  and and then add your new code
there, too.

> diff --git a/Makefile b/Makefile
> index ba6a602..fe6dd5b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -668,6 +668,22 @@ o2dnt_config:	unconfig
>  pf5200_config:	unconfig
>  	@$(MKCONFIG) pf5200  ppc mpc5xxx pf5200 esd
>  
> +phycore_mpc5200b_tiny_config \
> +phycore_mpc5200b_tiny_LOWBOOT_config \
> +phycore_mpc5200b_tiny_RAMBOOT_config:	unconfig
> +	@ >include/config.h
> +	@[ -z "$(findstring LOWBOOT_,$@)" ] || \
> +		{ echo "TEXT_BASE = 0xFF000000"	>board/phycore_mpc5200b_tiny/config.tmp ; \
> +		  echo "... with LOWBOOT configuration" ; \
> +		}
> +	@[ -z "$(findstring RAMBOOT_,$@)" ] || \
> +		{ echo "TEXT_BASE = 0x00100000"	>board/phycore_mpc5200b_tiny/config.tmp ; \
> +		  echo "... with RAMBOOT configuration" ; \
> +		  echo "... remember to make sure that MBAR is already switched to 0xF0000000 !!!" ; \
> +		}
> +	@$(MKCONFIG) -a phycore_mpc5200b_tiny ppc mpc5xxx phycore_mpc5200b_tiny
> +	@ echo "remember to set PHYCORE_MPC5200B_TINY_REV to 0 for rev 1245.0 rev or to 1 for rev 1245.1"

Please do not add that many make targets and such  long  code  for  a
single  board; restrict yourself to the typically used cases and omit
cases that are needed only  for  debugging  (RAMBOOT);  also,  please
consider using shorter names - "phycore_mpc5200b_tiny_LOWBOOT_config"
is 36 characters - that's a major PITA to type.

> index 0000000..e16959f
> --- /dev/null
> +++ b/board/phycore_mpc5200b_tiny/config.mk
> @@ -0,0 +1,43 @@
...
> +# phyCORE-MPC5200B tiny board:
> +#
> +#	Valid values for TEXT_BASE are:
> +#
> +#	0xFFF00000   boot high (standard configuration)
> +#	0xFF000000   boot low
> +#	0x00100000   boot from RAM (for testing only)
> +#
> +
> +sinclude $(TOPDIR)/board/$(BOARDDIR)/config.tmp
> +
> +ifndef TEXT_BASE
> +## Standard: boot high
> +TEXT_BASE = 0xFFF00000
> +## For testing: boot from RAM
> +## TEXT_BASE = 0x00100000
> +endif

Please remove the dead code here.

> diff --git a/board/phycore_mpc5200b_tiny/phycore_mpc5200b_tiny.c b/board/phycore_mpc5200b_tiny/phycore_mpc5200b_tiny.c
> new file mode 100644
> index 0000000..4e7674e
> --- /dev/null
> +++ b/board/phycore_mpc5200b_tiny/phycore_mpc5200b_tiny.c
> @@ -0,0 +1,307 @@
...
> +#ifndef CONFIG_SYS_RAMBOOT
> +static void sdram_start (int hi_addr)
> +{
> +	long hi_addr_bit = hi_addr ? 0x01000000 : 0;
> +
> +	/* unlock mode register */
> +	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000000 | hi_addr_bit;
> +	__asm__ volatile ("sync");
> +
> +	/* precharge all banks */
> +	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000002 | hi_addr_bit;
> +	__asm__ volatile ("sync");

Please use proper accessor  functions  (out_be32()  etc.)  to  access
device  registers;  then  you  can  also  omit  the "sync"s [here and
everywhere else].

...
> +void init_ide_reset (void)
> +{
> +	debug ("init_ide_reset\n");
> +
> +    	/* Configure PSC2_4 as GPIO output for ATA reset */
> +	*(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC2_4;
> +	*(vu_long *) MPC5XXX_WU_GPIO_DIR    |= GPIO_PSC2_4;
> +	/* Deassert reset */
> +	*(vu_long *) MPC5XXX_WU_GPIO_DATA_O   |= GPIO_PSC2_4;

Please use proper bit accessor  functions  (setbits_be32()  etc.)  to
access device registers [here and everywhere else].

...
> +void video_get_info_str (int line_number, char *info)
> +{
> +	if (line_number == 1) {
> +		strcpy (info, " Board: phyCORE-MPC5200B tiny (Phytec Messtechnik GmbH)");

Line too long.

> +/*
> + * Returns OPENIP register base address. First thing called in the driver.
> + */
> +unsigned int board_video_init (void)
> +{
> +ulong dummy;
> +dummy  = *(vu_long *)OPENIP_MMIO_BASE; /*dummy read*/
> +dummy  = *(vu_long *)OPENIP_MMIO_BASE; /*dummy read*/
> +	return OPENIP_MMIO_BASE;
> +}

What exactly are these dummy reads supposed to do? This smells as if
you were hushing up some other problem here?

> +/*
> + * Returns OPENIP framebuffer address
> + */
> +unsigned int board_video_get_fb (void)
> +{
> +
> +	return OPENIP_FB_BASE;

Please remove the empty line.

> diff --git a/board/phycore_mpc5200b_tiny/u-boot.lds b/board/phycore_mpc5200b_tiny/u-boot.lds
> new file mode 100644
> index 0000000..7bd6d4d

Do you really need a private linker scripts? Most other 5200 boards
use a common script, and I don;t see a specific reason why you would
need to do different.

> diff --git a/include/configs/phycore_mpc5200b_tiny.h b/include/configs/phycore_mpc5200b_tiny.h
> new file mode 100644
> index 0000000..89e660b
> --- /dev/null
> +++ b/include/configs/phycore_mpc5200b_tiny.h
> @@ -0,0 +1,431 @@
> +/*
> + * (C) Copyright 2003-2005
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * (C) Copyright 2006
> + * Eric Schumann, Phytec Messatechnik GmbH
> + *
> + * 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
> + */
> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +/* #define DEBUG */
> +
> +#define CONFIG_BOARDINFO	 "Phytec Phycore mpc5200b tiny"
> +
> +/*------------------------------------------------------------------------------------------------------------------------------------------------------
> +High Level Configuration Options
> +(easy to change)
> +------------------------------------------------------------------------------------------------------------------------------------------------------*/
> +#define CONFIG_MPC5xxx		1		/* This is an MPC5xxx CPU */
> +#define CONFIG_MPC5200		1		/* (more precisely an MPC5200 CPU) */
> +#define CONFIG_MPC5200_DDR	1		/* (with DDR-SDRAM) */
> +#define CONFIG_PHYCORE_MPC5200B_TINY 1		/* ... on phyCORE-MPC5200B -> FEC configuration and IDE */
> +#define CONFIG_SYS_MPC5XXX_CLKIN 33333333 	/* ... running at 33.333333MHz */
> +#define BOOTFLAG_COLD		0x01		/* Normal Power-On: Boot from FLASH  */
> +#define BOOTFLAG_WARM		0x02		/* Software reboot	     */
> +
> +/*------------------------------------------------------------------------------------------------------------------------------------------------------
> +Serial console configuration
> +-----------------------------------------------------------------------------------------------------------------------------------------------------*/
> +#define CONFIG_PSC_CONSOLE	3		/* console is on PSC3 -> define gps port conf. register later on to enable UART function! */
> +#define CONFIG_BAUDRATE		115200		/* ... at 115200 bps */
> +#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400 }

All these lines are WAY too long... [check all other code as well].

> +
> +/*
> + * Command line configuration.
> + */
> +#include <config_cmd_default.h>
> +
> +#define CONFIG_CMD_DATE
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_EEPROM
> +#define CONFIG_CMD_I2C
> +#define CONFIG_CMD_MII
> +#define CONFIG_CMD_NFS
> +#define CONFIG_CMD_PCI
> +#define CONFIG_CMD_JFFS2

Please sort list.

...
> +#define CONFIG_PREBOOT	"echo;"	\
> +	"echo Type \"run bootcmd_net\" to load Kernel over TFTP and to mount root filesystem over NFS;" \
> +	"echo"
> +
> +#if 0
> +#define	CONFIG_EXTRA_ENV_SETTINGS					\
> +	"netdev=eth0\0"							\
> +	"mtdparts=mtdparts=physmap-flash.0:256k(ubootl),1792k(kernel),13312k(jffs2),256k(uboot)ro,256k(oftree),-(space)\0" \
> +	"ipaddr=192.168.23.226\0"					\
> +	"netmask=255.255.255.0\0"					\
> +	"serverip=192.168.23.1\0"					\
> +	"gateway=192.168.23.1\0"					\
> +	"uimage=uImage-pcm030\0"				\
> +	"oftree=oftree-pcm030.dtb\0"				\
> +	"jffs2=root-pcm030.jffs2\0" 				\
> +	"uboot=u-boot-pcm030.bin\0"					\
> +	"bargs_base=setenv bootargs console=ttyPSC0,$(baudrate) $(mtdparts) rw\0" \
> +	"bargs_flash=setenv bootargs $(bootargs) root=/dev/mtdblock2 rootfstype=jffs2\0" \
> +	"bargs_nfs=setenv bootargs $(bootargs) root=/dev/nfs ip=$(ipaddr):$(serverip):$(gatewayip):$(netmask)::$(netdev):off nfsroot=$(serverip):$(nfsrootfs),v3,tcp\0" \
> +	"bcmd_net=run bargs_base bargs_nfs; tftpboot 0x500000 $(uimage); tftp 0x400000 $(oftree); bootm 0x500000 - 0x400000\0" \
> +	"bcmd_flash=run bargs_base bargs_flash; bootm 0xff040000 - 0xfff40000\0" \
> +	"prg_kernel=tftp 0x400000 $(uimage); erase 0xff040000 0xff1fffff; cp.b 0x400000 0xff040000 $(filesize)\0" \
> +	"prg_jffs2=tftp 0x400000 $(jffs2); erase 0xff200000 0xffefffff; cp.b 0x400000 0xff200000 $(filesize)\0" \
> +	"prg_oftree=tftp 0x400000 $(oftree); erase 0xfff40000 0xfff5ffff; cp.b 0x400000 0xfff40000 $(filesize)\0" \
> +	"update=tftpboot 0x400000 $(uboot);erase 0xFFF00000 0xfff3ffff; cp.b 0x400000 0xFFF00000 $(filesize)\0" \
> +	"unlock=yes\0" \
> +	""
> +#endif

Please remove such dead code. [here and everywhere else].


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Only in our dreams we are free.  The rest of the time we need  wages.
                                    - Terry Pratchett, _Wyrd Sisters_


More information about the U-Boot mailing list