Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
+#define CONFIG_IPADDR192.168.5.44 +#define CONFIG_SERVERIP 192.168.5.30 +#define CONFIG_NETMASK 255.255.255.0 please remove the IP params Why ? Because these values are only relevant to your lab network, not anybody else who's going to use your boards. Boards should start with a clean slate. This is a hard rule that we've been enforcing for a while now - no address or network parameter initializations in config files. Thanks Ben for clarification. Regards.. Prafulla . . ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
Hi Prafulla, Prafulla Wadaskar wrote: snip +#define CONFIG_IPADDR 192.168.5.44 +#define CONFIG_SERVERIP192.168.5.30 +#define CONFIG_NETMASK 255.255.255.0 please remove the IP params Why ? Because these values are only relevant to your lab network, not anybody else who's going to use your boards. Boards should start with a clean slate. This is a hard rule that we've been enforcing for a while now - no address or network parameter initializations in config files. regards, Ben ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
Hi Jean
Thanks for your comments,
Please see my reply inlined...
-Original Message-
From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagn...@jcrosoft.com]
Sent: Friday, April 17, 2009 1:15 PM
To: Prafulla Wadaskar
Cc: u-boot@lists.denx.de; Ashish Karkare; Ronen Shitrit
Subject: Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
On 21:48 Wed 08 Apr , Prafulla Wadaskar wrote:
From: prafulla_wadaskar prafu...@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
Note: doImage utility needed to convert u-boot.bin to
u-boot-spiflash.bin, DRAM configuration will be part of this utility
btw where is the spi driver?
Drivers/spi/kirkwood_spi.c through Kirkwood SOC support patch :-)
+#define MV88F6281GTW_GE_OE_HIGH
(~((BIT4)|(BIT6)|(BIT7)|(BIT12) \
+ |(BIT13)|(BIT16)|(BIT17)))
+#define MV88F6281GTW_GE_OE_VAL_LOW (BIT20) /*make GLED on */
+#define MV88F6281GTW_GE_OE_VAL_HIGH
((BIT6)|(BIT13)|(BIT16)|(BIT17))
plese remove the BITxx
Okay
+
+/*
+ * Default values for MPP registers
+ */
+#define MV88F6281GTW_GE_MPP0_7 0x0111
+#define MV88F6281GTW_GE_MPP8_150x11103311
+#define MV88F6281GTW_GE_MPP16_23 0x
+#define MV88F6281GTW_GE_MPP24_31 0x
+#define MV88F6281GTW_GE_MPP32_39 0x40440222
+#define MV88F6281GTW_GE_MPP40_47 0x
+#define MV88F6281GTW_GE_MPP48_55 0x
please move all this define to a header
and if possible please use macro to describe the content
Okay I will creat and move them to header
+ /* init serial */
+ gd-baudrate = CONFIG_BAUDRATE;
+ gd-have_console = 1;
+ serial_init();
no need please remove the serial init is done by the lib_arm/board.c
Okay I will remove it
+
+#endif /* CONFIG_MISC_INIT_R */
diff --git a/board/Marvell/mv88f6281gtw_ge/u-boot.lds
b/board/Marvell/mv88f6281gtw_ge/u-boot.lds
is it possible to have a shorter name for the board?
No Jean, not possible, kernel patches also represents the same name and machine
is also register with the same name, pleas bear with this, thanks..
+ .rodata : { *(.rodata) }
please replace by this
.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
Okay I will do it
+ . = ALIGN(4);
+ .data : { *(.data) }
+ . = ALIGN(4);
+ .got : { *(.got) }
+/*
+#define CONFIG_FEROCEON_88FR1311 /* CPU Core
subversion */
+#define LE 1 /* Specify LE/BE operation */
why?
Because SOC can be initialized to work in both the modes.
+#define CONFIG_KIRKWOOD1 /* SOC Family Name */
+#define CONFIG_KW88F6281 1 /* SOC Name */
+#define CONFIG_KW88F6281_A01 /* SOC Revision */
is is not possible to detect it?
I will try to detect it.
+#define CONFIG_BAUDRATE115200 /* console baudrate */
^
whitespace please remove
You mean spaces and tabs combination, I wll remove them
+
+#defineCONFIG_SYS_PROMPT Marvell /*
Command Prompt
why not Marvell or a board name?
This is to sync up with our current u-boot and the automation
tools/documentation based on it
Changing it to Marvell is not a big deal but will involve lot of unwanted
efforts.
+#defineCONFIG_SYS_CBSIZE 1024/* Console I/O
Buff Size */
+#defineCONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE \
+ +sizeof(CONFIG_SYS_PROMPT)+16) /* Print Buff */
please add space before and after '+'
Okay..
+#define CONFIG_SYS_MALLOC_LEN 0x0040 /* 4M */
4M?
What it should be?
+/* 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_MISC_INIT_R 1 /* call misc_init_r() */
+#define CONFIG_NR_DRAM_BANKS 4
^
whitespace please remove
Okay..
+#define CONFIG_STACKSIZE 0x0010 /* regular stack- 1M */
+#define CONFIG_SYS_LOAD_ADDR 0x0080 /*
default load adr- 8M */
+#define CONFIG_SYS_MEMTEST_START 0x0040/* 4M */
+#define CONFIG_SYS_MEMTEST_END 0x007f /*(_8M -1) */
_8M?
What it should be ?
+ */
+#ifdef CONFIG_CMD_NET
+#define CONFIG_NETCONSOLE /* include NetConsole support */
whitespace please remove
Okay ..
+#define CONFIG_NET_MULTI /* specify more that one ports
available */
+#define CONFIG_KIRKWOOD_EGIGA /* Enable SOC specific
Ethernet Gigabit
+ Controller Driver */
please use this style of multiple comment
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
+#define MV88F6281GTW_GE_MPP0_7 0x0111
+#define MV88F6281GTW_GE_MPP8_15 0x11103311
+#define MV88F6281GTW_GE_MPP16_23 0x
+#define MV88F6281GTW_GE_MPP24_31 0x
+#define MV88F6281GTW_GE_MPP32_39 0x40440222
+#define MV88F6281GTW_GE_MPP40_47 0x
+#define MV88F6281GTW_GE_MPP48_55 0x
please move all this define to a header
and if possible please use macro to describe the content
Okay I will creat and move them to header
+ /* init serial */
+ gd-baudrate = CONFIG_BAUDRATE;
+ gd-have_console = 1;
+ serial_init();
no need please remove the serial init is done by the lib_arm/board.c
Okay I will remove it
+
+#endif /* CONFIG_MISC_INIT_R */
diff --git a/board/Marvell/mv88f6281gtw_ge/u-boot.lds
b/board/Marvell/mv88f6281gtw_ge/u-boot.lds
is it possible to have a shorter name for the board?
No Jean, not possible, kernel patches also represents the same name and
machine is also register with the same name, pleas bear with this, thanks..
ok if possible next time try a shorter name
+ .rodata : { *(.rodata) }
please replace by this
.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
Okay I will do it
+ . = ALIGN(4);
+ .data : { *(.data) }
+ . = ALIGN(4);
+ .got : { *(.got) }
+/*
+#define CONFIG_FEROCEON_88FR131 1 /* CPU Core
subversion */
+#define LE 1 /* Specify LE/BE operation */
why?
Because SOC can be initialized to work in both the modes.
so the binary will be compile as LE or BE so __ARMEL__ or __ARMEB__ will be
defined
+#define CONFIG_KIRKWOOD 1 /* SOC Family Name */
+#define CONFIG_KW88F6281 1 /* SOC Name */
+#define CONFIG_KW88F6281_A0 1 /* SOC Revision */
is is not possible to detect it?
I will try to detect it.
+#define CONFIG_BAUDRATE 115200 /* console baudrate */
^
whitespace please remove
You mean spaces and tabs combination, I wll remove them
+
+#define CONFIG_SYS_PROMPT Marvell /*
Command Prompt
why not Marvell or a board name?
This is to sync up with our current u-boot and the automation
tools/documentation based on it
Changing it to Marvell is not a big deal but will involve lot of unwanted
efforts.
ok
+#define CONFIG_SYS_CBSIZE 1024/* Console I/O
Buff Size */
+#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE \
+ +sizeof(CONFIG_SYS_PROMPT)+16) /* Print Buff */
please add space before and after '+'
Okay..
+#define CONFIG_SYS_MALLOC_LEN0x0040 /* 4M */
4M?
What it should be?
just ask why do you need 4M of 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_MISC_INIT_R 1 /* call misc_init_r() */
+#define CONFIG_NR_DRAM_BANKS 4
^
whitespace please remove
Okay..
+#define CONFIG_STACKSIZE 0x0010 /* regular stack- 1M */
+#define CONFIG_SYS_LOAD_ADDR 0x0080 /*
default load adr- 8M */
+#define CONFIG_SYS_MEMTEST_START 0x0040 /* 4M */
+#define CONFIG_SYS_MEMTEST_END 0x007f /*(_8M -1) */
_8M?
What it should be ?
8M maybe
+ */
+#ifdef CONFIG_CMD_NET
+#define CONFIG_NETCONSOLE/* include NetConsole support */
whitespace please remove
Okay ..
+#define CONFIG_NET_MULTI /* specify more that one ports
available */
+#define CONFIG_KIRKWOOD_EGIGA/* Enable SOC specific
Ethernet Gigabit
+Controller Driver */
please use this style of multiple comment
/*
*
*/
Okay..
+#undef CONFIG_PHY_LINK_DETECT/* detect link always on */
+ /* specify ports to be used */
+#define CONFIG_KIRKWOOD_EGIGA_PORTS {TRUE,FALSE}
+ /* phy base addr for multi-chip
addressing */
+#define CONFIG_IPADDR192.168.5.44
+#define CONFIG_SERVERIP 192.168.5.30
+#define CONFIG_NETMASK 255.255.255.0
please remove the IP params
Why ?
it's board instance specific
Best Regards,
J.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
On 21:48 Wed 08 Apr , Prafulla Wadaskar wrote:
From: prafulla_wadaskar prafu...@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
Note: doImage utility needed to convert u-boot.bin to
u-boot-spiflash.bin, DRAM configuration will be part of
this utility
btw where is the spi driver?
Reviewed-by: Ronen Shitrit rshit...@marvell.com
Signed-off-by: Prafulla Wadaskar prafu...@marvell.com
---
Changelog
v2: updated as per first review comments by Wolfgang Denk
debug_prints updated to debug
MAKEALL |1 +
Makefile|3 +
board/Marvell/mv88f6281gtw_ge/Makefile | 52 +++
board/Marvell/mv88f6281gtw_ge/config.mk | 25 +++
board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c | 118 ++
board/Marvell/mv88f6281gtw_ge/u-boot.lds| 53 +++
include/configs/mv88f6281gtw_ge.h | 185
+++
7 files changed, 437 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/u-boot.lds
create mode 100644 include/configs/mv88f6281gtw_ge.h
diff --git a/MAKEALL b/MAKEALL
index e4eb42b..1caf81d 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -504,6 +504,7 @@ LIST_ARM9= \
cp946es \
cp966 \
lpd7a400\
+ mv88f6281gtw_ge \
mx1ads \
mx1fs2 \
netstar \
diff --git a/Makefile b/Makefile
index d2c7c3f..709e4be 100644
--- a/Makefile
+ * 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
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define MV88F6281GTW_GE_OE_LOW (~((BIT7) | (BIT20) \
+ |(BIT21))) /*enable GLED,RLED */
+#define MV88F6281GTW_GE_OE_HIGH (~((BIT4)|(BIT6)|(BIT7)|(BIT12)
\
+ |(BIT13)|(BIT16)|(BIT17)))
+#define MV88F6281GTW_GE_OE_VAL_LOW (BIT20) /*make GLED on */
+#define MV88F6281GTW_GE_OE_VAL_HIGH ((BIT6)|(BIT13)|(BIT16)|(BIT17))
plese remove the BITxx
+
+/*
+ * Default values for MPP registers
+ */
+#define MV88F6281GTW_GE_MPP0_7 0x0111
+#define MV88F6281GTW_GE_MPP8_15 0x11103311
+#define MV88F6281GTW_GE_MPP16_23 0x
+#define MV88F6281GTW_GE_MPP24_31 0x
+#define MV88F6281GTW_GE_MPP32_39 0x40440222
+#define MV88F6281GTW_GE_MPP40_47 0x
+#define MV88F6281GTW_GE_MPP48_55 0x
please move all this define to a header
and if possible please use macro to describe the content
+
+/*
+ * function definitations
+ */
+#ifdef CONFIG_SWITCH_88E61XX
+extern int mv_switch_88e61xx_init(u32 eth_port_num);
+#endif
please move it to a header
+
+int board_init(void)
+{
+ unsigned int i;
+
+ /* Board Parameters initializations */
+ kw_window_ctrl_reg_init();
+ kw_gpio_init(MV88F6281GTW_GE_OE_VAL_LOW,
+ MV88F6281GTW_GE_OE_VAL_HIGH,
+ MV88F6281GTW_GE_OE_LOW,
+ MV88F6281GTW_GE_OE_HIGH);
+
+ kw_mpp_control_init(MV88F6281GTW_GE_MPP0_7,
+ MV88F6281GTW_GE_MPP8_15,
+ MV88F6281GTW_GE_MPP16_23,
+ MV88F6281GTW_GE_MPP24_31,
+ MV88F6281GTW_GE_MPP32_39,
+ MV88F6281GTW_GE_MPP40_47,
+ MV88F6281GTW_GE_MPP48_55);
+
+ /* init serial */
+ gd-baudrate = CONFIG_BAUDRATE;
+ gd-have_console = 1;
+ serial_init();
no need please remove the serial init is done by the lib_arm/board.c
+ /*
+ * arch number of USED SOC
+ */
+ gd-bd-bi_arch_number = MACH_TYPE_MV88F6281GTW_GE;
+
+ /* adress of boot parameters */
+ gd-bd-bi_boot_params = 0x0100;
+
+ return 0;
+}
+
+int dram_init(void)
+{
+ int i;
+
+ for (i = 0; i
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
Hi all, This is much cleaner and smaller patch. Any comments or acceptance or flag to resend are welcomed. Regards.. Prafulla . . -Original Message- From: Prafulla Wadaskar [mailto:prafu...@marvell.com] Sent: Wednesday, April 08, 2009 9:48 PM To: u-boot@lists.denx.de Cc: Ronen Shitrit; Ashish Karkare; Prafulla Wadaskar Subject: [PATCH v2] Marvell MV88F6281GTW_GE Board support From: prafulla_wadaskar prafu...@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 Note: doImage utility needed to convert u-boot.bin to u-boot-spiflash.bin, DRAM configuration will be part of this utility Reviewed-by: Ronen Shitrit rshit...@marvell.com Signed-off-by: Prafulla Wadaskar prafu...@marvell.com --- Changelog v2: updated as per first review comments by Wolfgang Denk debug_prints updated to debug MAKEALL |1 + Makefile|3 + board/Marvell/mv88f6281gtw_ge/Makefile | 52 +++ board/Marvell/mv88f6281gtw_ge/config.mk | 25 +++ board/Marvell/mv88f6281gtw_ge/mv88f6281gtw_ge.c | 118 ++ board/Marvell/mv88f6281gtw_ge/u-boot.lds| 53 +++ include/configs/mv88f6281gtw_ge.h | 185 +++ 7 files changed, 437 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/u-boot.lds create mode 100644 include/configs/mv88f6281gtw_ge.h diff --git a/MAKEALL b/MAKEALL index e4eb42b..1caf81d 100755 --- a/MAKEALL +++ b/MAKEALL @@ -504,6 +504,7 @@ LIST_ARM9= \ cp946es \ cp966 \ lpd7a400\ + mv88f6281gtw_ge \ mx1ads \ mx1fs2 \ netstar \ diff --git a/Makefile b/Makefile index d2c7c3f..709e4be 100644 --- a/Makefile +++ b/Makefile @@ -2792,6 +2792,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 000..1b8cc6f --- /dev/null +++ b/board/Marvell/mv88f6281gtw_ge/Makefile @@ -0,0 +1,52 @@ +# +# (C) Copyright 2009 +# Marvell Semiconductor www.marvell.com +# Prafulla Wadaskar prafu...@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 +COBJS+= ../common/mv88e61xx.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 000..fb29a1b --- /dev/null +++ b/board/Marvell/mv88f6281gtw_ge/config.mk @@ -0,0 +1,25 @@ +# +# (C) Copyright 2009 +# Marvell Semiconductor www.marvell.com +# Prafulla Wadaskar prafu...@marvell.com +# +# See file CREDITS for list of people who contributed to this
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
On Wednesday 08 April 2009 12:18:11 Prafulla Wadaskar wrote: From: prafulla_wadaskar prafu...@marvell.com this part should be fixed as well -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
