[U-Boot] [PATCH] Marvell MV88F6281GTW_GE Board support

Wolfgang Denk wd at denx.de
Fri Apr 3 20:42:42 CEST 2009


Dear Prafulla Wadaskar,

In message <1238798370-9245-5-git-send-email-prafulla at marvell.com> you wrote:
> From: prafulla_wadaskar <prafulla at marvell.com>
> 
> This is Marvell's 88F6281_A0 based custom board developed
> for wireless access point product
> 
> This patch is tested for-
> 1. Boot from DRAM/SPI flash/NFS
> 2. File transfer using tftp and loadb
> 3. SPI flash read/write/erase
> 4. Booting Linux kernel and RFS from SPI flash
> 
> Signed-off-by: prafulla_wadaskar <prafulla at marvell.com>
> Reviewed by: Ronen Shitrit <rshitrit at marvell.com>
> Tested by: Piyush Shah <spiyush at marvell.com>
...
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -506,6 +506,7 @@ LIST_ARM9="			\
>  	lpd7a400		\
>  	mx1ads			\
>  	mx1fs2			\
> +	mv88f6281gtw_ge		\
>  	netstar			\
>  	nmdk8815		\
>  	omap1510inn		\

Please keep lists sorted.

> diff --git a/Makefile b/Makefile
> index 8bf36ce..eb2fdfb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2564,6 +2564,9 @@ DB64360_config:	unconfig
>  DB64460_config:	unconfig
>  	@$(MKCONFIG) DB64460 ppc 74xx_7xx db64460 Marvell
>  
> +mv88f6281gtw_ge_config: unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood
> +
>  ELPPC_config: unconfig
>  	@$(MKCONFIG) $(@:_config=) ppc 74xx_7xx elppc eltec

Please keep lists sorted.

> diff --git a/board/Marvell/mv88f6281gtw_ge/bin_dep.sh b/board/Marvell/mv88f6281gtw_ge/bin_dep.sh
> new file mode 100755
> index 0000000..5cb1e36
> --- /dev/null
> +++ b/board/Marvell/mv88f6281gtw_ge/bin_dep.sh
> @@ -0,0 +1,74 @@
...
> +CUR_DIR=$2
> +TOP_DIR=`pwd`
> +BIN_FILE=$TOP_DIR/$1
> +
> +DOIMAGE=$TOP_DIR/cpu/arm926ejs/kirkwood/doimage/doimage


Please explain why this would be needed for?

Note that this most probably will not fork with out of tree builds.

...
> diff --git a/board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt b/board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt
> new file mode 100644
> index 0000000..d40a9c7
> --- /dev/null
> +++ b/board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt
> @@ -0,0 +1,27 @@
> +0xFFD01400 0x43000a00
> +0xFFD01404 0x38543000
> +0xFFD01408 0x2202433D
> +0xFFD0140C 0x0000002A
> +0xFFD01410 0x0000000D
> +0xFFD01414 0x00000000
> +0xFFD01418 0x00000000
> +0xFFD0141C 0x00000c52
> +0xFFD01420 0x00000046
> +0xFFD01424 0x0000F1FF
> +0xFFD01428 0x00085520
> +0xFFD0147c 0x00008552
> +0xFFD01508 0x00000000
> +0xFFD01504 0x07FFFFF1
> +0xFFD0150C 0x00000000
> +0xFFD01514 0x00000000
> +0xFFD0151C 0x00000000
> +0xFFD01494 0x00010001
> +0xFFD01498 0x00000000
> +0xFFD0149C 0x0000E811
> +0xFFD01480 0x00000001
> +0xFFD20204 0x00000000
> +0xFFD100E0 0x1B1B1B9B
> +0xFFD100D8 0x00000060
> +0xFFD1007C 0x00000068
> +0xFFD50430 0x00000008
> +0x0 0x0

What sort of file is this? Under which license is it?

> diff --git a/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c
> new file mode 100644
> index 0000000..b88fe3e
> --- /dev/null
> +++ b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c
> @@ -0,0 +1,135 @@
...
> +#ifndef MV88F6281GTW_GE_DEBUG
> +#define MV88F6281GTW_GE_DEBUG		0
> +#endif
> +#define DEBUG_PRINT		MV88F6281GTW_GE_DEBUG

Unused code? Delete!

> +#include <common.h>
> +#include <debug_prints.h>

Non existent file included here.

> +/* this line must be removed after this machine name gets mainlined in mach_types.h*/
> +#define MACH_TYPE_MV88F6281GTW_GE      1932

Please first register the MACH ID, then fix this, then resubmit.


> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define MV88F6281GTW_GE_OE_VAL_LOW	(BIT20)	/*make GLED on */
> +#define MV88F6281GTW_GE_OE_VAL_HIGH	((BIT6)|(BIT13)|(BIT16)|(BIT17))
> +#define MV88F6281GTW_GE_OE_LOW		(~((BIT7) | (BIT20) | (BIT21)))	/*enable GLED,RLED */
> +#define MV88F6281GTW_GE_OE_HIGH		(~((BIT4)|(BIT6)|(BIT7)|(BIT12)|(BIT13)|(BIT16)|(BIT17)))

Lines too long (also check in all other files).

> +/*
> + * Default values for MPP registers
> + */
> +#define MV88F6281GTW_GE_MPP0_7		0x01112222
> +#define MV88F6281GTW_GE_MPP8_15		0x11103311
> +#define MV88F6281GTW_GE_MPP16_23	0x00001111
> +#define MV88F6281GTW_GE_MPP24_31	0x22222222
> +#define MV88F6281GTW_GE_MPP32_39	0x40440222
> +#define MV88F6281GTW_GE_MPP40_47	0x00004444
> +#define MV88F6281GTW_GE_MPP48_55	0x00000000
> +
> +/*
> + * function definitations
> + */
> +#ifdef CONFIG_SWITCH_88E61XX
> +extern int mv_switch_88e61xx_init(u32 eth_port_num);
> +#endif

Do not use extern. Privde proper prototypes in the respective header
files.

> +int board_init(void)
> +{
...
> +	/* relocate the exception vectors */
> +	/* U-Boot is running from DRAM at this stage */
> +	for (i = 0; i < 0x100; i += 4) {
> +		*(unsigned int *)(0x0 + i) = *(unsigned int *)(TEXT_BASE + i);
> +	}

Are you absolutely sure this code is correct?

> +int dram_init(void)
> +{
> +	int i;
> +
> +	debug_print_ftrace();

You probably want to get rid of these. I don't think we will accept
this.

> diff --git a/board/Marvell/mv88f6281gtw_ge/u-boot.lds b/board/Marvell/mv88f6281gtw_ge/u-boot.lds
> new file mode 100644
> index 0000000..0338757
> --- /dev/null
> +++ b/board/Marvell/mv88f6281gtw_ge/u-boot.lds
> @@ -0,0 +1,53 @@
> +/*
> + * (C) Copyright 2009
> + * Marvell Semiconductor <www.marvell.com>
> + * Prafulla Wadaskar <prafulla at marvell.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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> +	. = _start;
> +	. = ALIGN(4);
> +	.text	:
> +	{
> +	  cpu/arm926ejs/start.o	(.text)
> +	  *(.text)
> +	}
> +	.rodata : { *(.rodata) }
> +	. = ALIGN(4);
> +	.data : { *(.data) }
> +	. = ALIGN(4);
> +	.got : { *(.got) }
> +
> +	. = .;
> +	__u_boot_cmd_start = .;
> +	.u_boot_cmd : { *(.u_boot_cmd) }
> +	__u_boot_cmd_end = .;
> +
> +	. = ALIGN(4);
> +	__bss_start = .;
> +	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
> +	_end = .;
> +}
> +
> diff --git a/include/configs/mv88f6281gtw_ge.h b/include/configs/mv88f6281gtw_ge.h
> new file mode 100644
> index 0000000..70e6dca
> --- /dev/null
> +++ b/include/configs/mv88f6281gtw_ge.h
...
> +/*
> + * Above definitions used in below file, do not change the sequence
> + */
> +#include <configs/kirkwood.h>	/* CPU specific information */

include/configs/ is strictly for board specific information only; CPU
specific data has no place there.

...
> +#define	CONFIG_BOOTMAPSZ		(8<<20)	/* Initial Memmap for Linux */
> +#define CONFIG_CMDLINE_TAG	1	/* enable passing of ATAGs  */
> +#define CONFIG_INITRD_TAG	1	/* enable INITRD tag */
> +#define CONFIG_SETUP_MEMORY_TAGS 1	/* enable memory tag */
> +
> +#define	CONFIG_SYS_PROMPT		"Marvell>> "	/* Command Prompt */
> +#define	CONFIG_SYS_CBSIZE		1024	/* Console I/O Buff Size */
> +#define	CONFIG_SYS_PBSIZE 		(CONFIG_SYS_CBSIZE \
> +				+sizeof(CONFIG_SYS_PROMPT)+16)	/* Print Buff */

Lines too long, here and elsewhere. Please fix globally.

> + * Commands configuration
> + */
> +#define CONFIG_CMD_ENV
> +#define CONFIG_CMD_RUN
> +#define CONFIG_CMD_LOADB
> +#define CONFIG_CMD_NET
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_AUTOSCRIPT

AUTOSCRIPT is deprecated. Please use SOURCE instead.


> +#ifdef CONFIG_SPI_FLASH
> +#define CONFIG_ENV_IS_IN_SPI_FLASH	1
> +#define CONFIG_ENV_SIZE			_64K	/* 1 spi flash block */
> +#define CONFIG_ENV_SECT_SIZE		_64K

Please use real numbers  and get rid of such "cool" constants.


> +/* Default environment variables */
> +#define CONFIG_BOOTCOMMAND	"$(x_bootcmd_kernel); setenv bootargs $(x_bootargs) $(x_bootargs_root); bootm 0x6400000; "
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> +	"x_bootargs=console=ttyS0,115200 mtdparts=spi0.0:512k(uboot),512k at 512k(psm),2m at 1m(kernel),13m at 3m(rootfs)\0" \


Lines WAY too long.

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
Sometimes a feeling is all we humans have to go on.
	-- Kirk, "A Taste of Armageddon", stardate 3193.9


More information about the U-Boot mailing list