[U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
From: prafulla_wadaskar 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 Signed-off-by: Prafulla Wadaskar --- 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 +# Prafulla Wadaskar +# +# 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 +# Prafulla Wadaskar +# +# 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 +# F
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
On Wednesday 08 April 2009 12:18:11 Prafulla Wadaskar wrote: > From: prafulla_wadaskar 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
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 > > 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 > Signed-off-by: Prafulla Wadaskar > --- > 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 > +# Prafulla Wadaskar > +# > +# 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 > +# Prafulla Wadaskar > +# > +# See file CREDITS for list of people who contrib
Re: [U-Boot] [PATCH v2] Marvell MV88F6281GTW_GE Board support
On 21:48 Wed 08 Apr , Prafulla Wadaskar wrote:
> From: prafulla_wadaskar
>
> 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
> Signed-off-by: Prafulla Wadaskar
> ---
> 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
> +
> +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;
> +}
> +
>
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
> >
> > 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
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
> > > > + > > > > +#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 Sure... Except one more exception please "mv88f6192gtw_fe" This is next board whose kernel support and board name registry is already done in linux, this will be the next board support patch from me after getting this stuff in. For next upcomming boards I will keep in mind > > > > +#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 Okay I will change accordingly > > > > +#define CONFIG_SYS_MALLOC_LEN 0x0040 /* 4M */ > > > 4M? > > What it should be? > just ask why do you need 4M of malloc? In future to support all SOC drivers, I will keep it 4M if you agree > > > 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 :-) I will keep it 8M at this moment > > > > +#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 ? > it's board instance specific Okay I will remove them 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
On 23:26 Sat 18 Apr , Prafulla Wadaskar wrote: > > > > > + > > > > > +#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 > Sure... Except one more exception please "mv88f6192gtw_fe" > This is next board whose kernel support and board name registry is already > done in linux, > this will be the next board support patch from me after getting this stuff in. > For next upcomming boards I will keep in mind ok > > > > > > +#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 > Okay I will change accordingly > > > > > > +#define CONFIG_SYS_MALLOC_LEN0x0040 /* 4M */ > > > > 4M? > > > What it should be? > > just ask why do you need 4M of malloc? > In future to support all SOC drivers, I will keep it 4M if you agree ok 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
Hi Prafulla, Prafulla Wadaskar wrote: >>> +#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
> >>> +#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
