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

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

[Prafulla Wadaskar]

From: prafulla_wadaskar <prafulla@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

Reviewed-by: Ronen Shitrit <rshitrit@marvell.com>
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
Change log
v2: updated as per first review comments debug_prints updated to debug

v3: updaed as per review comments for v2 added mv88f6281gtw_ge.h file removed BITxx macros

v4: updated as per review comments for v3 arch_misc_init support is added and used from kirkwood

v5: updated as per review comments for v4 CONFIG_MACH_MV88F6281GTW_GE added more comments added and serial configuration removed from mv88f6281gtw_ge.c

V6: clean switch configuration using netdev.h

v7: Marvell copyright removed from u-boot.lds Maintainer added for this board

v8: u-boot.lds removed
finetuned for cosmetic and switch related changes

v9: new mpp configuration used
CONFIG_ARCH_LOWLEVEL_INIT defination removed

v10: CONFIG_ENV_SIZE set to 4kb
CONFIG_SYS_MALLOC_LEN set to 128kb

 MAKEALL                                         |    1 +
 Makefile                                        |    3 +
 board/Marvell/mv88f6281gtw_ge/Makefile          |   51 ++++++
 board/Marvell/mv88f6281gtw_ge/config.mk         |   25 +++
 board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c |  141 +++++++++++++++++
 board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.h |   36 +++++
 include/configs/mv88f6281gtw_ge.h               |  187 +++++++++++++++++++++++
 7 files changed, 444 insertions(+), 0 deletions(-)
 create mode 100644 board/Marvell/mv88f6281gtw_ge/Makefile
 create mode 100644 board/Marvell/mv88f6281gtw_ge/config.mk
 create mode 100644 board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c
 create mode 100644 board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.h
 create mode 100644 include/configs/mv88f6281gtw_ge.h

diff --git a/MAKEALL b/MAKEALL
index c98d03a..6719d7b 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -505,6 +505,7 @@ LIST_ARM9="			\
 	cp946es			\
 	cp966			\
 	lpd7a400		\
+	mv88f6281gtw_ge		\
 	mx1ads			\
 	mx1fs2			\
 	netstar			\
diff --git a/Makefile b/Makefile
index 81a5cd0..8144ecd 100644
--- a/Makefile
+++ b/Makefile
@@ -2808,6 +2808,9 @@ lpd7a400_config \
 lpd7a404_config:	unconfig
 	@$(MKCONFIG) $(@:_config=) arm lh7a40x lpd7a40x
 
+mv88f6281gtw_ge_config: unconfig
+	@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood
+
 mx1ads_config	:	unconfig
 	@$(MKCONFIG) $(@:_config=) arm arm920t mx1ads NULL imx
 
diff --git a/board/Marvell/mv88f6281gtw_ge/Makefile b/board/Marvell/mv88f6281gtw_ge/Makefile
new file mode 100644
index 0000000..8c49a3e
--- /dev/null
+++ b/board/Marvell/mv88f6281gtw_ge/Makefile
@@ -0,0 +1,51 @@
+#
+# (C) Copyright 2009
+# Marvell Semiconductor <www.marvell.com>
+# Prafulla Wadaskar <prafulla@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
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(BOARD).a
+
+COBJS	:= mv88f6281gtw_ge.o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+SOBJS	:= $(addprefix $(obj),$(SOBJS))
+
+$(LIB):	$(obj).depend $(OBJS) $(SOBJS)
+	$(AR) $(ARFLAGS) $@ $(OBJS) $(SOBJS)
+
+clean:
+	rm -f $(SOBJS) $(OBJS)
+
+distclean:	clean
+	rm -f $(LIB) core *.bak .depend
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/board/Marvell/mv88f6281gtw_ge/config.mk b/board/Marvell/mv88f6281gtw_ge/config.mk
new file mode 100644
index 0000000..fb29a1b
--- /dev/null
+++ b/board/Marvell/mv88f6281gtw_ge/config.mk
@@ -0,0 +1,25 @@
+#
+# (C) Copyright 2009
+# Marvell Semiconductor <www.marvell.com>
+# Prafulla Wadaskar <prafulla@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
+#
+
+TEXT_BASE = 0x00600000
diff --git a/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c
new file mode 100644
index 0000000..7ea61be
--- /dev/null
+++ b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c
@@ -0,0 +1,141 @@
+/*
+ * Maintainer : Prafulla Wadaskar <prafulla@marvell.com>
+ *
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <prafulla@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
+ */
+
+#include <common.h>
+#include <netdev.h>
+#include <asm/arch/kirkwood.h>
+#include <asm/arch/mpp.h>
+#include "mv88f6281gtw_ge.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int board_init(void)
+{
+	/*
+	 * default gpio configuration
+	 * There are maximum 64 gpios controlled through 2 sets of registers
+	 * the  below configuration configures mainly initial LED status
+	 */
+	kw_config_gpio(MV88F6281GTW_GE_OE_VAL_LOW,
+			MV88F6281GTW_GE_OE_VAL_HIGH,
+			MV88F6281GTW_GE_OE_LOW, MV88F6281GTW_GE_OE_HIGH);
+
+	/* Multi-Purpose Pins Functionality configuration */
+	u32 kwmpp_config[] = {
+		MPP0_SPI_SCn,
+		MPP1_SPI_MOSI,
+		MPP2_SPI_SCK,
+		MPP3_SPI_MISO,
+		MPP4_GPIO,
+		MPP5_GPO,
+		MPP6_SYSRST_OUTn,
+		MPP7_SPI_SCn,
+		MPP8_TW_SDA,
+		MPP9_TW_SCK,
+		MPP10_UART0_TXD,
+		MPP11_UART0_RXD,
+		MPP12_GPO,
+		MPP13_GPIO,
+		MPP14_GPIO,
+		MPP15_GPIO,
+		MPP16_GPIO,
+		MPP17_GPIO,
+		MPP18_GPO,
+		MPP19_GPO,
+		MPP20_GPIO,
+		MPP21_GPIO,
+		MPP22_GPIO,
+		MPP23_GPIO,
+		MPP24_GPIO,
+		MPP25_GPIO,
+		MPP26_GPIO,
+		MPP27_GPIO,
+		MPP28_GPIO,
+		MPP29_GPIO,
+		MPP30_GPIO,
+		MPP31_GPIO,
+		MPP32_GPIO,
+		MPP33_GPIO,
+		MPP34_GPIO,
+		MPP35_GPIO,
+		MPP36_GPIO,
+		MPP37_GPIO,
+		MPP38_GPIO,
+		MPP39_GPIO,
+		MPP40_GPIO,
+		MPP41_GPIO,
+		MPP42_GPIO,
+		MPP43_GPIO,
+		MPP44_GPIO,
+		MPP45_GPIO,
+		MPP46_GPIO,
+		MPP47_GPIO,
+		MPP48_GPIO,
+		MPP49_GPIO,
+		0
+	};
+	kirkwood_mpp_conf(kwmpp_config);
+
+	/*
+	 * arch number of board
+	 */
+	gd->bd->bi_arch_number = MACH_TYPE_MV88F6281GTW_GE;
+
+	/* adress of boot parameters */
+	gd->bd->bi_boot_params = kw_sdram_bar(0) + 0x100;
+
+	return 0;
+}
+
+int dram_init(void)
+{
+	int i;
+
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		gd->bd->bi_dram[i].start = kw_sdram_bar(i);
+		gd->bd->bi_dram[i].size = kw_sdram_bs(i);
+	}
+	return 0;
+}
+
+#ifdef CONFIG_MV88E61XX_SWITCH
+void reset_phy(void)
+{
+	/* configure and initialize switch */
+	struct mv88e61xx_config swcfg = {
+		.name = "egiga0",
+		.vlancfg = MV88E61XX_VLANCFG_ROUTER,
+		.rgmii_delay = MV88E61XX_RGMII_DELAY_EN,
+		.led_init = MV88E61XX_LED_INIT_EN,
+		.mdip = MV88E61XX_MDIP_REVERSE,
+		.portstate = MV88E61XX_PORTSTT_FORWARDING,
+		.cpuport = (1 << 5),
+		.ports_enabled = 0x3f
+	};
+
+	mv88e61xx_switch_initialize(&swcfg);
+}
+#endif /* CONFIG_MV88E61XX_SWITCH */
diff --git a/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.h b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.h
new file mode 100644
index 0000000..fb0e571
--- /dev/null
+++ b/board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.h
@@ -0,0 +1,36 @@
+/*
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <prafulla@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
+ */
+
+#ifndef __MV88F6281GTW_GE_H
+#define __MV88F6281GTW_GE_H
+
+#define MV88F6281GTW_GE_OE_LOW		(~((1 << 7) | (1 << 12) \
+					  |(1 << 20) | (1 << 21)))	/*enable GLED,RLED */
+#define MV88F6281GTW_GE_OE_HIGH		(~((1 << 4)|(1 << 6)|(1 << 7)|(1 << 12) \
+					  |(1 << 13)|(1 << 16)|(1 << 17)))
+#define MV88F6281GTW_GE_OE_VAL_LOW	(1 << 20)	/*make GLED on */
+#define MV88F6281GTW_GE_OE_VAL_HIGH	((1 << 6)|(1 << 13)|(1 << 16)|(1 << 17))
+
+
+#endif /* __MV88F6281GTW_GE_H */
diff --git a/include/configs/mv88f6281gtw_ge.h b/include/configs/mv88f6281gtw_ge.h
new file mode 100644
index 0000000..8ace78e
--- /dev/null
+++ b/include/configs/mv88f6281gtw_ge.h
@@ -0,0 +1,187 @@
+/*
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <prafulla@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
+ */
+
+#ifndef _CONFIG_MV88F6281GTW_GE_H
+#define _CONFIG_MV88F6281GTW_GE_H
+
+/*
+ * Version number information
+ */
+#define CONFIG_IDENT_STRING	"\nMarvell-MV88F6281GTW_GE-A0"
+
+/*
+ * High Level Configuration Options (easy to change)
+ */
+#define CONFIG_MARVELL		1
+#define CONFIG_ARM926EJS	1	/* Basic Architecture */
+#define CONFIG_FEROCEON_88FR131	1	/* CPU Core subversion */
+#define CONFIG_KIRKWOOD		1	/* SOC Family Name */
+#define CONFIG_KW88F6281	1	/* SOC Name */
+#define CONFIG_MACH_MV88F6281GTW_GE	/* Machine type */
+
+#define CONFIG_MD5	/* get_random_hex on krikwood needs MD5 support */
+#define CONFIG_SKIP_LOWLEVEL_INIT	/* disable board lowlevel_init */
+#define CONFIG_KIRKWOOD_EGIGA_INIT	/* Enable GbePort0/1 for kernel */
+#define CONFIG_KIRKWOOD_PCIE_INIT	/* Enable PCIE Port0 for kernel */
+#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII Pad voltage to 1.8V */
+
+/*
+ * CLKs configurations
+ */
+#define CONFIG_SYS_HZ		1000
+
+/*
+ * NS16550 Configuration
+ */
+#define CONFIG_SYS_NS16550
+#define CONFIG_SYS_NS16550_SERIAL
+#define CONFIG_SYS_NS16550_REG_SIZE	(-4)
+#define CONFIG_SYS_NS16550_CLK		CONFIG_SYS_TCLK
+#define CONFIG_SYS_NS16550_COM1		KW_UART0_BASE
+
+/*
+ * Serial Port configuration
+ * The following definitions let you select what serial you want to use
+ * for your console driver.
+ */
+
+#define CONFIG_CONS_INDEX	1	/*Console on UART0 */
+#define CONFIG_BAUDRATE			115200	/* console baudrate */
+#define CONFIG_SYS_BAUDRATE_TABLE	{ 9600, 19200, 38400, 57600, \
+					  115200,230400, 460800, 921600 }
+/* auto boot */
+#define CONFIG_BOOTDELAY	3	/* default enable autoboot */
+
+/*
+ * For booting Linux, the board info and command line data
+ * have to be in the first 8 MB of memory, since this is
+ * the maximum mapped by the Linux kernel during initialization.
+ */
+#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 */
+/*
+ * 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
+#define CONFIG_CMD_MEMORY
+#define CONFIG_CMD_BOOTD
+#define CONFIG_CMD_SAVEENV
+#define CONFIG_CMD_SF
+
+/*
+ * Flash configuration
+ */
+#ifdef CONFIG_CMD_SF
+#define CONFIG_SYS_NO_FLASH		1	/* Declare no NOR flash */
+#define CONFIG_SPI_FLASH		1
+#define CONFIG_HARD_SPI			1
+#define CONFIG_KIRKWOOD_SPI		1
+#define CONFIG_SPI_FLASH_MACRONIX	1
+#define CONFIG_ENV_SPI_BUS		0
+#define CONFIG_ENV_SPI_CS		0
+#define CONFIG_ENV_SPI_MAX_HZ		50000000	/*50Mhz */
+#endif
+
+/*
+ *  Environment variables configurations
+ */
+#ifdef CONFIG_SPI_FLASH
+#define CONFIG_ENV_IS_IN_SPI_FLASH	1
+#define CONFIG_ENV_SECT_SIZE		0x10000	/* 64K */
+#else
+#define CONFIG_ENV_IS_NOWHERE		1	/* if env in SDRAM */
+#endif
+#define CONFIG_ENV_SIZE			0x1000	/* 4k */
+#define CONFIG_ENV_ADDR			0x20000
+#define CONFIG_ENV_OFFSET		0x20000	/* env starts here */
+
+/*
+ * Default environment variables
+ */
+#define CONFIG_BOOTCOMMAND		"${x_bootcmd_kernel}; "	\
+	"setenv bootargs ${x_bootargs} ${x_bootargs_root}; "	\
+	"bootm 0x6400000;"
+
+#define CONFIG_MTDPARTS			"spi0.0:512k(uboot),"	\
+	"512k at 512k(psm), 2m at 1m(kernel),13m at 3m(rootfs)\0"
+
+#define CONFIG_EXTRA_ENV_SETTINGS	"x_bootargs=console"	\
+	"=ttyS0,115200 mtdparts="CONFIG_MTDPARTS	\
+	"x_bootcmd_kernel=cp.b 0xf8100000 0x6400000 0x200000\0" \
+	"x_bootargs_root=root=/dev/mtdblock3 ro rootfstype=squashfs\0"
+
+/*
+ * Size of malloc() pool
+ */
+#define CONFIG_SYS_MALLOC_LEN	(1024 * 128) /* 128kB for malloc() */
+/* size in bytes reserved for initial data */
+#define CONFIG_SYS_GBL_DATA_SIZE	128
+
+/*
+ * Other required minimal configurations
+ */
+#define CONFIG_CONSOLE_INFO_QUIET	/* some code reduction */
+#define CONFIG_ARCH_CPU_INIT	/* call arch_cpu_init() */
+#define CONFIG_ARCH_MISC_INIT	/* call arch_misc_init() */
+#define CONFIG_DISPLAY_CPUINFO	/* Display cpu info */
+#define CONFIG_NR_DRAM_BANKS	4
+#define CONFIG_STACKSIZE	0x00100000	/* regular stack- 1M */
+#define CONFIG_SYS_LOAD_ADDR	0x00800000	/* default load adr- 8M */
+#define CONFIG_SYS_MEMTEST_START 0x00400000	/* 4M */
+#define CONFIG_SYS_MEMTEST_END	0x007fffff	/*(_8M -1) */
+#define CONFIG_SYS_RESET_ADDRESS 0xffff0000	/* Rst Vector Adr */
+#define CONFIG_SYS_MAXARGS	16	/* max number of command args */
+
+/*
+ * Ethernet Driver configuration
+ */
+#ifdef CONFIG_CMD_NET
+#define CONFIG_NETCONSOLE	/* include NetConsole support   */
+#define CONFIG_NET_MULTI	/* specify more that one ports available */
+#define	CONFIG_MII		/* expose smi ove miiphy interface */
+#define CONFIG_KIRKWOOD_EGIGA	/* Enable kirkwood Gbe Controller Driver */
+#define CONFIG_SYS_FAULT_ECHO_LINK_DOWN	/* detect link using phy */
+#define CONFIG_KIRKWOOD_EGIGA_PORTS	{1,0}	/* enable port 0 only */
+#define CONFIG_ENV_OVERWRITE	/* ethaddr can be reprogrammed */
+#endif /* CONFIG_CMD_NET */
+
+/*
+ * Marvell 88Exxxx Switch configurations
+ */
+#define CONFIG_RESET_PHY_R	/* use reset_phy() to init phy/swtich */
+#define CONFIG_MV88E61XX_SWITCH	/* Enable mv88e61xx switch driver */
+
+#endif /* _CONFIG_MV88F6281GTW_GE_H */
-- 
1.5.3.3

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

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1242865967-7179-1-git-send-email-prafulla@marvell.com> you wrote:
> 
> This is Marvell's 88F6281_A0 based custom board developed
> for wireless access point product

> --- /dev/null
> +++ b/board/Marvell/mv88f6281gtw_ge/Makefile
> @@ -0,0 +1,51 @@
> +#
> +# (C) Copyright 2009
> +# Marvell Semiconductor <www.marvell.com>
> +# Prafulla Wadaskar <prafulla@marvell.com>

As mentioned before, this makes no sense. Either Marvell holds the
copyright, or you do.

...
> +#define CONFIG_MD5	/* get_random_hex on krikwood needs MD5 support */
> +#define CONFIG_SKIP_LOWLEVEL_INIT	/* disable board lowlevel_init */
> +#define CONFIG_KIRKWOOD_EGIGA_INIT	/* Enable GbePort0/1 for kernel */
> +#define CONFIG_KIRKWOOD_PCIE_INIT	/* Enable PCIE Port0 for kernel */
> +#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII Pad voltage to 1.8V */

Line too long.

...
> +/*
> + * For booting Linux, the board info and command line data
> + * have to be in the first 8 MB of memory, since this is
> + * the maximum mapped by the Linux kernel during initialization.
> + */
> +#define	CONFIG_BOOTMAPSZ	(8 << 20)	/* Initial Memmap for Linux */

Ditto. Please also check other files.


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@denx.de
Vor allem kein Gedanke! Nichts ist kompromittierender als ein  Gedan-
ke!            - Friedrich Wilhelm Nietzsche _Der Fall Wagner_ (1888)

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

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Friday, May 22, 2009 4:37 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; 
> Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH v10] Marvell MV88F6281GTW_GE 
> Board support
> 
> Dear Prafulla Wadaskar,
> 
> In message 
> <1242865967-7179-1-git-send-email-prafulla@marvell.com> you wrote:
> > 
> > This is Marvell's 88F6281_A0 based custom board developed 
> for wireless 
> > access point product
> 
> > --- /dev/null
> > +++ b/board/Marvell/mv88f6281gtw_ge/Makefile
> > @@ -0,0 +1,51 @@
> > +#
> > +# (C) Copyright 2009
> > +# Marvell Semiconductor <www.marvell.com> # Prafulla Wadaskar 
> > +<prafulla@marvell.com>
> 
> As mentioned before, this makes no sense. Either Marvell 
> holds the copyright, or you do.
Marvell holds copyright, I will remove "prafulla at marvell.com"
This is there from first submission in all the patches
such comments in early feedback is more helpful....
I will correct and re-submit all patches...

> > +#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII 
> Pad voltage to 1.8V */
> 
> Line too long.
BTW what is max allowed line size? All line are below 80 chars.

Regards..
Prafulla . .

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

[Prafulla Wadaskar]

> > 
> > As mentioned before, this makes no sense. Either Marvell holds the 
> > copyright, or you do.
> Marvell holds copyright, I will remove "prafulla at marvell.com"
> This is there from first submission in all the patches such 
> comments in early feedback is more helpful....
> I will correct and re-submit all patches...
Dear Woldgang Denk

I have posted two patches for with copyright statements corrected.
Whereas I have updated them for all patches at my end.
It make no sense to post a patch JUST for this change
I will post other patches with other (expected..) review feedback

Regards..
Prafulla ..

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

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E201CF9E687F@SC-VEXCH1.marvell.com> you wrote:
> 
> > As mentioned before, this makes no sense. Either Marvell 
> > holds the copyright, or you do.
> Marvell holds copyright, I will remove "prafulla at marvell.com"
> This is there from first submission in all the patches
> such comments in early feedback is more helpful....

Yes, I am aware of this - sorry. You know how this works - you submit
something, and the second after you hit the "send"  botton  you  will
see  a blatant typo. Same here - I've scanned several versions of the
files and did not realize this, but not it hit me.

> I will correct and re-submit all patches...

Thanks.

> > > +#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII 
> > Pad voltage to 1.8V */
> > 
> > Line too long.
> BTW what is max allowed line size? All line are below 80 chars.

The maximum line is 80 characters. Please keep in mind that one TAB
character may occupy several columns - I guess you measured the
character count, and not the line length.

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
Use C++ to confuse your enemies; use C to produce stable code.

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

[Jean-Christophe PLAGNIOL-VILLARD]

> + */
> +
> +#include <common.h>
> +#include <netdev.h>
> +#include <asm/arch/kirkwood.h>
> +#include <asm/arch/mpp.h>
> +#include "mv88f6281gtw_ge.h"
please move the mv88f6281gtw_ge.h
to include/asm-arm/arch-kirkwood/
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int board_init(void)
> +{
> +	/*
> +	 * default gpio configuration
> +	 * There are maximum 64 gpios controlled through 2 sets of registers
> +	 * the  below configuration configures mainly initial LED status
> +	 */
> +	kw_config_gpio(MV88F6281GTW_GE_OE_VAL_LOW,
> +			MV88F6281GTW_GE_OE_VAL_HIGH,
> +			MV88F6281GTW_GE_OE_LOW, MV88F6281GTW_GE_OE_HIGH);
> +
> +	/* Multi-Purpose Pins Functionality configuration */
> +	u32 kwmpp_config[] = {
do you really need to configure all of this IO?
> +		MPP0_SPI_SCn,
> +		MPP1_SPI_MOSI,
> +		MPP2_SPI_SCK,
> +		MPP3_SPI_MISO,
please add a device init api as done for davinci or at91
(could be done in an other patch)
> +		MPP4_GPIO,
> +		MPP5_GPO,
> +		MPP6_SYSRST_OUTn,
> +		MPP7_SPI_SCn,
> +		MPP8_TW_SDA,
Best Regards,
J.

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

[Prafulla Wadaskar]

> > > > +#define CONFIG_KIRKWOOD_RGMII_PAD_1V8	/* Set RGMII 
> > > Pad voltage to 1.8V */
> > > 
> > > Line too long.
> > BTW what is max allowed line size? All line are below 80 chars.
> 
> The maximum line is 80 characters. Plpeease keep in mind that 
> one TAB character may occupy several columns - I guess you 
> measured the character count, and not the line length.

Hi Wolfgang Denk
I have counted 80 considering tabs as 8chars, and that's every where in the code,
Hopefully your tool is having some different settings (ex. Tab size >8) then may be this is valid comment.

If you are using any specific opensource tool/script to fix/check indentation code (other than Lindent) and If you can share with me. That would help me to provide code in sync with your requirements..

Thanks and Regards..
Prafulla . .

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

[Prafulla Wadaskar]

> > + */
> > +
> > +#include <common.h>
> > +#include <netdev.h>
> > +#include <asm/arch/kirkwood.h>
> > +#include <asm/arch/mpp.h>
> > +#include "mv88f6281gtw_ge.h"
> please move the mv88f6281gtw_ge.h
> to include/asm-arm/arch-kirkwood/
Sure... I will do it :-)

> > +int board_init(void)
> > +{
> > +	/*
> > +	 * default gpio configuration
> > +	 * There are maximum 64 gpios controlled through 2 sets 
> of registers
> > +	 * the  below configuration configures mainly initial LED status
> > +	 */
> > +	kw_config_gpio(MV88F6281GTW_GE_OE_VAL_LOW,
> > +			MV88F6281GTW_GE_OE_VAL_HIGH,
> > +			MV88F6281GTW_GE_OE_LOW, 
> MV88F6281GTW_GE_OE_HIGH);
> > +
> > +	/* Multi-Purpose Pins Functionality configuration */
> > +	u32 kwmpp_config[] = {
> do you really need to configure all of this IO?
Yes
I device needs all IO initializations done to make sure unused MPPs must be configured to GPIOs.
This is required to reduce overall boards noise that we have identified through h/w test in the lab
 
> > +		MPP0_SPI_SCn,
> > +		MPP1_SPI_MOSI,
> > +		MPP2_SPI_SCK,
> > +		MPP3_SPI_MISO,
> please add a device init api as done for davinci or at91 
> (could be done in an other patch)
I could not get any reference for davinci or at91 for "device_init" 
Can you please provide me a specific pointer ?

OR do you mean to create new api "device_init" to do soc/device specific init ?
I think this is really not required here since it is specific to kirkwood implementation and may not apply for other architectures.

In my opinion we do not need any specific init here in this code... Pls clarify...

Regards..
Prafulla . .

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

[Stefan Roese]

On Monday 25 May 2009 06:48:40 Prafulla Wadaskar wrote:
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <netdev.h>
> > > +#include <asm/arch/kirkwood.h>
> > > +#include <asm/arch/mpp.h>
> > > +#include "mv88f6281gtw_ge.h"
> >
> > please move the mv88f6281gtw_ge.h
> > to include/asm-arm/arch-kirkwood/
>
> Sure... I will do it :-)

Sorry for the late reply, but for my taste such board specific headers don't 
belong into the "common" include/asm-arch/arch-kirkwood directory. The 
original location in the board directory is the best place (IMHO).

> > > +int board_init(void)
> > > +{
> > > +	/*
> > > +	 * default gpio configuration
> > > +	 * There are maximum 64 gpios controlled through 2 sets
> >
> > of registers
> >
> > > +	 * the  below configuration configures mainly initial LED status
> > > +	 */
> > > +	kw_config_gpio(MV88F6281GTW_GE_OE_VAL_LOW,
> > > +			MV88F6281GTW_GE_OE_VAL_HIGH,
> > > +			MV88F6281GTW_GE_OE_LOW,
> >
> > MV88F6281GTW_GE_OE_HIGH);
> >
> > > +
> > > +	/* Multi-Purpose Pins Functionality configuration */
> > > +	u32 kwmpp_config[] = {
> >
> > do you really need to configure all of this IO?
>
> Yes
> I device needs all IO initializations done to make sure unused MPPs must be
> configured to GPIOs. This is required to reduce overall boards noise that
> we have identified through h/w test in the lab
>
> > > +		MPP0_SPI_SCn,
> > > +		MPP1_SPI_MOSI,
> > > +		MPP2_SPI_SCK,
> > > +		MPP3_SPI_MISO,
> >
> > please add a device init api as done for davinci or at91
> > (could be done in an other patch)
>
> I could not get any reference for davinci or at91 for "device_init"
> Can you please provide me a specific pointer ?
>
> OR do you mean to create new api "device_init" to do soc/device specific
> init ? I think this is really not required here since it is specific to
> kirkwood implementation and may not apply for other architectures.
>
> In my opinion we do not need any specific init here in this code... Pls
> clarify...

I think the current code version is fine. We don't need to create an over-
complex API here.

Thanks.

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
=====================================================================