add new board nas62x0

Add support for new boards RaidSonic ICY BOX NAS6210 and NAS6220 boards.

Only difference between boards is number of SATA ports. By default we use only one SATA port.

Add support for new boards RaidSonic ICY BOX NAS6210 and NAS6220 boards.

Add support for new boards RaidSonic ICY BOX NAS6210 and NAS6220 boards.

Only difference between boards is number of SATA ports. By default we use only one SATA port.

board/Marvell/ib62x0/ib62x0.h new file +15 -0

+ *
+ * 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

board/Marvell/ib62x0/ib62x0.h new file +12 -0

+#define IB62x0_OE_VAL_LOW	(1 << 29)	/* USB_PWEN low */
+#define IB62x0_OE_VAL_HIGH	(1 << 17)	/* LED pin high */
+
+/* PHY related */
+#define MV88E1116_LED_FCTRL_REG		10
+#define MV88E1116_CPRSP_CR3_REG		21
+#define MV88E1116_MAC_CTRL_REG		21
+#define MV88E1116_PGADR_REG		22
+#define MV88E1116_RGMII_TXTM_CTRL	(1 << 4)
+#define MV88E1116_RGMII_RXTM_CTRL	(1 << 5)
+
+#endif /* __IB62x0_H */

(quoted diff) +0 -0

b/board/Marvell/ib62x0/kwbimage.cfg new file mode 100644 index 0000000..ffa3c18

board/Marvell/ib62x0/kwbimage.cfg new file +27 -0

@@ -0,0 +1,167 @@
+#
+# Copyright (C) 2011 G??rald Kerma <[email protected]>
+#
+# Written-by: G??rald Kerma <[email protected]>
+#
+# 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
+#
+# Refer docs/README.kwimage for more details about how-to configure
+# and create kirkwood boot image
+#
+

board/Marvell/ib62x0/kwbimage.cfg new file +17 -0

+
+#Dram initalization for SINGLE x16 CL=5 @ 400MHz
+DATA 0xFFD01400 0x43000c30	# DDR Configuration register
+# bit13-0:  0xc30 (3120 DDR2 clks refresh rate)
+# bit23-14: zero
+# bit24: 1= enable exit self refresh mode on DDR access
+# bit25: 1 required
+# bit29-26: zero
+# bit31-30: 01
+
+DATA 0xFFD01404 0x37543000	# DDR Controller Control Low
+# bit 4:    0=addr/cmd in smame cycle
+# bit 5:    0=clk is driven during self refresh, we don't care for APX
+# bit 6:    0=use recommended falling edge of clk for addr/cmd
+# bit14:    0=input buffer always powered up
+# bit18:    1=cpu lock transaction enabled
+# bit23-20: 5=recommended value for CL=5 and STARTBURST_DEL disabled

bit31=0 +# bit27-24: 7= CL+2, STARTBURST sample stages, for freqs 400MHz, unbuffered DIMM +# bit30-28: 3 required

board/Marvell/ib62x0/kwbimage.cfg new file +71 -0

+# bit31:    0=no additional STARTBURST delay
+
+DATA 0xFFD01408 0x22125451	# DDR Timing (Low) (active cycles value +1)
+# bit3-0:   TRAS lsbs
+# bit7-4:   TRCD
+# bit11- 8: TRP
+# bit15-12: TWR
+# bit19-16: TWTR
+# bit20:    TRAS msb
+# bit23-21: 0x0
+# bit27-24: TRRD
+# bit31-28: TRTP
+
+DATA 0xFFD0140C 0x00000a33	# DDR Timing (High)
+# bit6-0:   TRFC
+# bit8-7:   TR2R
+# bit10-9:  TR2W
+# bit12-11: TW2W
+# bit31-13: zero required
+
+DATA 0xFFD01410 0x000000CC	# DDR Address Control
+# bit1-0:   00, Cs0width=x8
+# bit3-2:   11, Cs0size=1Gb
+# bit5-4:   00, Cs1width=x8
+# bit7-6:   11, Cs1size=1Gb
+# bit9-8:   00, Cs2width=nonexistent
+# bit11-10: 00, Cs2size =nonexistent
+# bit13-12: 00, Cs3width=nonexistent
+# bit15-14: 00, Cs3size =nonexistent
+# bit16:    0,  Cs0AddrSel
+# bit17:    0,  Cs1AddrSel
+# bit18:    0,  Cs2AddrSel
+# bit19:    0,  Cs3AddrSel
+# bit31-20: 0 required
+
+DATA 0xFFD01414 0x00000000	# DDR Open Pages Control
+# bit0:    0,  OpenPage enabled
+# bit31-1: 0 required
+
+DATA 0xFFD01418 0x00000000	# DDR Operation
+# bit3-0:   0x0, DDR cmd
+# bit31-4:  0 required
+
+DATA 0xFFD0141C 0x00000C52	# DDR Mode
+# bit2-0:   2, BurstLen=2 required
+# bit3:     0, BurstType=0 required
+# bit6-4:   4, CL=5
+# bit7:     0, TestMode=0 normal
+# bit8:     0, DLL reset=0 normal
+# bit11-9:  6, auto-precharge write recovery ????????????
+# bit12:    0, PD must be zero
+# bit31-13: 0 required
+
+DATA 0xFFD01420 0x00000004	# DDR Extended Mode
+# bit0:    0,  DDR DLL enabled
+# bit1:    0,  DDR drive strenght normal
+# bit2:    1,  DDR ODT control lsd (disabled)
+# bit5-3:  000, required
+# bit6:    0,  DDR ODT control msb, (disabled)
+# bit9-7:  000, required
+# bit10:   0,  differential DQS enabled
+# bit11:   0, required
+# bit12:   0, DDR output buffer enabled
+# bit31-13: 0 required
+
+DATA 0xFFD01424 0x0000F17F	# DDR Controller Control High
+# bit2-0:  111, required
+# bit3  :  1  , MBUS Burst Chop disabled
+# bit6-4:  111, required
+# bit7  :  0
+# bit8  :  1  , add writepath sample stage, must be 1 for DDR freq >=

300MHz +# bit9 : 0 , no half clock cycle addition to dataout

board/Marvell/ib62x0/kwbimage.cfg new file +39 -0

+# bit10 :  0  , 1/4 clock cycle skew enabled for addr/ctl signals
+# bit11 :  0  , 1/4 clock cycle skew disabled for write mesh
+# bit15-12: 1111 required
+# bit31-16: 0    required
+
+DATA 0xFFD01428 0x00085520	# DDR2 ODT Read Timing (default values)
+DATA 0xFFD0147C 0x00008552	# DDR2 ODT Write Timing (default values)
+
+DATA 0xFFD01500 0x00000000	# CS[0]n Base address to 0x0
+DATA 0xFFD01504 0x0FFFFFF1	# CS[0]n Size
+# bit0:    1,  Window enabled
+# bit1:    0,  Write Protect disabled
+# bit3-2:  00, CS0 hit selected
+# bit23-4: ones, required
+# bit31-24: 0x0F, Size (i.e. 256MB)
+
+DATA 0xFFD01508 0x10000000	# CS[1]n Base address to 256Mb
+DATA 0xFFD0150C 0x00000000	# CS[1]n Size, window disabled
+
+DATA 0xFFD01514 0x00000000	# CS[2]n Size, window disabled
+DATA 0xFFD0151C 0x00000000	# CS[3]n Size, window disabled
+
+DATA 0xFFD01494 0x00120012	# DDR ODT Control (Low)
+# bit3-0:  2, ODT0Rd, MODT[0] asserted during read from DRAM CS1
+# bit7-4:  1, ODT0Rd, MODT[0] asserted during read from DRAM CS0
+# bit19-16:2, ODT0Wr, MODT[0] asserted during write to DRAM CS1
+# bit23-20:1, ODT0Wr, MODT[0] asserted during write to DRAM CS0
+
+DATA 0xFFD01498 0x00000000	# DDR ODT Control (High)
+# bit1-0:  00, ODT0 controlled by ODT Control (low) register above
+# bit3-2:  01, ODT1 active NEVER!
+# bit31-4: zero, required
+
+DATA 0xFFD0149C 0x0000E40F	# CPU ODT Control
+DATA 0xFFD01480 0x00000001	# DDR Initialization Control
+#bit0=1, enable DDR init upon this register write
+
+# End of Header extension
+DATA 0x0 0x0

boards.cfg +0 -0

index 28cc345..a4e8852 100644
@@ -147,6 +147,8 @@ netspace_max_v2              arm         arm926ejs  

netspace_v2 LaCie netspace_v2 arm
arm926ejs netspace_v2 LaCie
kirkwood lacie_kw:NETSPACE_V2 dreamplug arm
arm926ejs - Marvell kirkwood guruplug
arm arm926ejs - Marvell
kirkwood +ib_nas6210 arm arm926ejs ib62x0
Marvell kirkwood ib62x0:BOARD_IS_IB_NAS6210
+ib_nas6220 arm arm926ejs ib62x0
Marvell kirkwood ib62x0:BOARD_IS_IB_NAS6220 mv88f6281gtw_ge
arm arm926ejs - Marvell
kirkwood openrd_base arm arm926ejs openrd
Marvell kirkwood openrd:BOARD_IS_OPENRD_BASE
openrd_client arm arm926ejs openrd
Marvell kirkwood openrd:BOARD_IS_OPENRD_CLIENT diff --git
a/include/configs/ib62x0.h b/include/configs/ib62x0.h new file mode 100644
index 0000000..5981b65

include/configs/ib62x0.h new file +36 -0

@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2011-2012
+ * G??rald Kerma <[email protected]>
+ * Luka Perkov <[email protected]>
+ *
+ * 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_IB62x0_H
+#define _CONFIG_IB62x0_H
+
+/*
+ * Version number information
+ */
+#ifdef CONFIG_BOARD_IS_IB_NAS6210
+# define CONFIG_IDENT_STRING	" RaidSonic ICY BOX IB-NAS6210"
+#elif CONFIG_BOARD_IS_IB_NAS6220
+# define CONFIG_IDENT_STRING	" RaidSonic ICY BOX IB-NAS6220"
+#else
+# error Unknown RaidSonic ICY BOX board specified

include/configs/ib62x0.h new file +30 -0

+
+/*
+ * Other required minimal configurations
+ */
+#define CONFIG_DISPLAY_CPUINFO		/* Display cpu info */
+#define CONFIG_SYS_ALT_MEMTEST
+
+/*
+ * Compression configuration
+ */
+#define CONFIG_BZIP2
+#define CONFIG_LZMA
+#define CONFIG_LZO
+
+/*
+ * Commands configuration
+ */
+#define CONFIG_SYS_NO_FLASH		/* Declare no flash (NOR/SPI) */
+#define CONFIG_SYS_MVFS
+#include <config_cmd_default.h>
+#define CONFIG_CMD_ENV
+#define CONFIG_CMD_IDE
+#define CONFIG_CMD_MII
+#define CONFIG_CMD_NAND
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_USB
+/*
+ * mv-common.h should be defined after CMD configs since it used them
+ * to enable certain macros
+ */

include/configs/ib62x0.h new file +58 -0

+
+/*
+ * Environment variables configurations
+ */
+#ifdef CONFIG_CMD_NAND
+#define CONFIG_ENV_IS_IN_NAND
+#define CONFIG_ENV_SECT_SIZE	0x20000
+#else
+#define CONFIG_ENV_IS_NOWHERE
+#endif
+#define CONFIG_ENV_SIZE		0x20000
+#define CONFIG_ENV_OFFSET	0x80000
+
+/*
+ * Default environment variables
+ */
+#define CONFIG_BOOTCOMMAND \
+	"setenv bootargs ${console} ${mtdparts} ${bootargs_root}; "	\
+	"ubi part root; "						\
+	"ubifsmount root; "						\
+	"ubifsload 0x800000 ${kernel}; "				\
+	"ubifsload 0x1100000 ${initrd}; "				\
+	"bootm 0x800000 0x1100000"
+
+#define CONFIG_MTDPARTS				\
+	"mtdparts=orion_nand:"			\
+	"0x80000 at 0x0(uboot),"			\
+	"0x20000 at 0x80000(uboot_env),"		\
+	"- at 0xa0000(root)\0"
+
+#define CONFIG_EXTRA_ENV_SETTINGS					\
+	"console=console=ttyS0,115200\0"				\
+	"mtdids=nand0=orion_nand\0"					\
+	"mtdparts="CONFIG_MTDPARTS					\
+	"kernel=/boot/uImage\0"						\
+	"initrd=/boot/uInitrd\0"					\
+	"bootargs_root=ubi.mtd=2 root=ubi0:root rootfstype=ubifs\0"
+
+/*
+ * Ethernet Driver configuration
+ */
+#ifdef CONFIG_CMD_NET
+#define CONFIG_MVGBE_PORTS	{1, 0}	/* enable port 0 only */
+#define CONFIG_PHY_BASE_ADR	0
+#undef CONFIG_RESET_PHY_R
+#endif /* CONFIG_CMD_NET */
+
+/*
+ * SATA Driver configuration
+ */
+#ifdef CONFIG_CMD_IDE
+#define __io
+#define CONFIG_IDE_PREINIT
+#define CONFIG_DOS_PARTITION
+#define CONFIG_MVSATA_IDE_USE_PORT0
+# ifdef CONFIG_BOARD_IS_IB_NAS6210
+#  undef CONFIG_SYS_IDE_MAXBUS
+#  define CONFIG_SYS_IDE_MAXBUS		1

include/configs/ib62x0.h new file +12 -0

+
+/*
+ * RTC driver configuration
+ */
+#ifdef CONFIG_CMD_DATE
+#define CONFIG_RTC_MV
+#endif /* CONFIG_CMD_DATE */
+
+/*
+ * File system
+ */
+#define CONFIG_CMD_EXT2

Quoted 1 line

+ * Copyright (C) 2011 G??rald Kerma <[email protected]>

Can you please fix your name here?

board/Marvell/ib62x0/ib62x0.h line 29

Quoted 2 lines

board/Marvell/ib62x0/ib62x0.h new file +2 -0

+#define IB62x0_OE_LOW		(~(0))
+#define IB62x0_OE_HIGH		(~(0))

Fix this constant please (0xffffffff) and remove those parenthesis ... btw OE_HIGH and OE_LOW have both the same value?

board/Marvell/ib62x0/kwbimage.cfg line 37

Quoted 2 lines

board/Marvell/ib62x0/kwbimage.cfg new file +2 -0

+# Configure RGMII-0 interface pad voltage to 1.8V
+DATA 0xFFD100e0 0x1b1b1b9b

Make usage of upper/lower case consistent across files in your patch please (lowercase prefered).

Quoted 1 line

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init */

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

include/configs/ib62x0.h line 86

Quoted 6 lines

include/configs/ib62x0.h new file +6 -0

+# define CONFIG_SYS_PROMPT	"IB-NAS6210 # "
+#elif CONFIG_BOARD_IS_IB_NAS6220
+# define CONFIG_SYS_PROMPT	"IB-NAS6220 # "
+#else
+# error Unknown RaidSonic ICY BOX board specified
+#endif

Please make the prompt like "=> " so we can run tests on this :)

Quoted 8 lines

include/configs/ib62x0.h new file +8 -0

+#define CONFIG_MVSATA_IDE_USE_PORT0
+# ifdef CONFIG_BOARD_IS_IB_NAS6210
+#  undef CONFIG_SYS_IDE_MAXBUS
+#  define CONFIG_SYS_IDE_MAXBUS		1
+#  undef CONFIG_SYS_IDE_MAXDEVICE
+#  define CONFIG_SYS_IDE_MAXDEVICE	1
+# elif CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_MVSATA_IDE_USE_PORT1

include/configs/ib62x0.h line 154

Quoted 6 lines

include/configs/ib62x0.h new file +6 -0

+# endif
+#define CONFIG_SYS_ATA_IDE0_OFFSET	KW_SATA_PORT0_OFFSET
+# ifdef CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_SYS_ATA_IDE1_OFFSET	KW_SATA_PORT1_OFFSET
+# endif
+#endif /* CONFIG_CMD_IDE */

please don't use this "#[space][space]define" convention.

The patch looks good otherwise )

Hi Marek,

Quoted 3 lines · Marek Vasut

Quoted 1 line

+ * Copyright (C) 2011 G??rald Kerma <[email protected]>

Can you please fix your name here?

I think your mail agent is not displaying UTF8 characters correctly. If this is a problem we could change it if Gerald is ok with that. Nobody else had problems with this...

Quoted 5 lines

board/Marvell/ib62x0/ib62x0.h line 29

Quoted 2 lines

board/Marvell/ib62x0/ib62x0.h new file +2 -0

+#define IB62x0_OE_LOW		(~(0))
+#define IB62x0_OE_HIGH		(~(0))

Fix this constant please (0xffffffff) and remove those parenthesis ... btw OE_HIGH and OE_LOW have both the same value?

Are you sure? It's done this way in:

board/Marvell/dreamplug/dreamplug.h
board/Marvell/sheevaplug/sheevaplug.h
board/Seagate/dockstar/dockstar.h

Quoted 6 lines

board/Marvell/ib62x0/kwbimage.cfg line 37

Quoted 3 lines

board/Marvell/ib62x0/kwbimage.cfg new file +2 -0

+# Configure RGMII-0 interface pad voltage to 1.8V
+DATA 0xFFD100e0 0x1b1b1b9b

Make usage of upper/lower case consistent across files in your patch please (lowercase prefered).

Ok. I will send this in v4 once I get your feedback on other items.

Quoted 2 lines

Quoted 2 lines

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init

Quoted 3 lines

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

I will retest and send v4 once I get your feedback on other items.

Quoted 9 lines

include/configs/ib62x0.h line 86

Quoted 7 lines

include/configs/ib62x0.h new file +6 -0

+# define CONFIG_SYS_PROMPT	"IB-NAS6210 # "
+#elif CONFIG_BOARD_IS_IB_NAS6220
+# define CONFIG_SYS_PROMPT	"IB-NAS6220 # "
+#else
+# error Unknown RaidSonic ICY BOX board specified
+#endif

Please make the prompt like "=> " so we can run tests on this :)

Ok.

Quoted 8 lines

Quoted 8 lines

include/configs/ib62x0.h new file +8 -0

+#define CONFIG_MVSATA_IDE_USE_PORT0
+# ifdef CONFIG_BOARD_IS_IB_NAS6210
+#  undef CONFIG_SYS_IDE_MAXBUS
+#  define CONFIG_SYS_IDE_MAXBUS		1
+#  undef CONFIG_SYS_IDE_MAXDEVICE
+#  define CONFIG_SYS_IDE_MAXDEVICE	1
+# elif CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_MVSATA_IDE_USE_PORT1

Quoted 8 lines

include/configs/ib62x0.h line 154

Quoted 6 lines

include/configs/ib62x0.h new file +6 -0

+# endif
+#define CONFIG_SYS_ATA_IDE0_OFFSET	KW_SATA_PORT0_OFFSET
+# ifdef CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_SYS_ATA_IDE1_OFFSET	KW_SATA_PORT1_OFFSET
+# endif
+#endif /* CONFIG_CMD_IDE */

please don't use this "#[space][space]define" convention.

I must disagree here. This is more readable and there are many examples in u-boot that use that syntax:

$ egrep '#[ ]+define' * -r | wc -l
12557

Quoted 2 lines

The patch looks good otherwise )

Cool. Thanks for reviewing.

Bye,
Luka

Quoted 6 lines

Quoted 5 lines

board/Marvell/ib62x0/ib62x0.h line 29

Quoted 2 lines

board/Marvell/ib62x0/ib62x0.h new file +2 -0

+#define IB62x0_OE_LOW		(~(0))
+#define IB62x0_OE_HIGH		(~(0))

Fix this constant please (0xffffffff) and remove those parenthesis ... btw OE_HIGH and OE_LOW have both the same value?

Quoted 5 lines

Are you sure? It's done this way in:

board/Marvell/dreamplug/dreamplug.h
board/Marvell/sheevaplug/sheevaplug.h
board/Seagate/dockstar/dockstar.h

Does that mean it's inherently correct and not just a duplicated bug?

Quoted 1 line

Quoted 1 line

Quoted 1 line

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init

*/

Quoted 5 lines

Quoted 3 lines

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

I will retest and send v4 once I get your feedback on other items.

Ok, what's the result? From IRC I take it you must define this ... why?

Quoted 20 lines

Quoted 8 lines

Quoted 8 lines

include/configs/ib62x0.h new file +8 -0

+#define CONFIG_MVSATA_IDE_USE_PORT0
+# ifdef CONFIG_BOARD_IS_IB_NAS6210
+#  undef CONFIG_SYS_IDE_MAXBUS
+#  define CONFIG_SYS_IDE_MAXBUS		1
+#  undef CONFIG_SYS_IDE_MAXDEVICE
+#  define CONFIG_SYS_IDE_MAXDEVICE	1
+# elif CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_MVSATA_IDE_USE_PORT1

Quoted 8 lines

include/configs/ib62x0.h line 154

Quoted 6 lines

include/configs/ib62x0.h new file +6 -0

+# endif
+#define CONFIG_SYS_ATA_IDE0_OFFSET	KW_SATA_PORT0_OFFSET
+# ifdef CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_SYS_ATA_IDE1_OFFSET	KW_SATA_PORT1_OFFSET
+# endif
+#endif /* CONFIG_CMD_IDE */

please don't use this "#[space][space]define" convention.

I must disagree here. This is more readable and there are many examples in u-boot that use that syntax:

Quoted 2 lines

$ egrep '#[ ]+define' * -r | wc -l
12557

Again, the fact that it's used doesn't mean it's correct. It's not more readable actually, it's readable in the same way given you have good editor. Also, doesn't checkpatch.pl caugh on this stuff ?

Hi Marek,

Quoted 12 lines · Marek Vasut

Quoted 11 lines

Quoted 5 lines

board/Marvell/ib62x0/ib62x0.h line 29

Quoted 2 lines

board/Marvell/ib62x0/ib62x0.h new file +2 -0

+#define IB62x0_OE_LOW		(~(0))
+#define IB62x0_OE_HIGH		(~(0))

Fix this constant please (0xffffffff) and remove those parenthesis ... btw OE_HIGH and OE_LOW have both the same value?

Are you sure? It's done this way in:

board/Marvell/dreamplug/dreamplug.h
board/Marvell/sheevaplug/sheevaplug.h
board/Seagate/dockstar/dockstar.h

Quoted 1 line

Does that mean it's inherently correct and not just a duplicated bug?

Well I really dont know. Judging by your comments it looks like kirkwood target could use some optimizations in "core" and not only in the board code.

Quoted 10 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init

*/

Quoted 4 lines

Quoted 2 lines

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

I will retest and send v4 once I get your feedback on other items.

Quoted 1 line

Ok, what's the result? From IRC I take it you must define this ... why?

It generates error when building without it:

/home/luka/uboot/arch/arm/cpu/arm926ejs/start.S:393: undefined reference to `lowlevel_init' arm-openwrt-linux-ld: BFD (GNU Binutils) 2.22 assertion fail elf32-arm.c:13830

All other kirkwood targets I looked at define CONFIG_SKIP_LOWLEVEL_INIT, including the ones mentioned above; here are their configs for comparison:

include/configs/dreamplug.h
include/configs/sheevaplug.h
include/configs/dockstar.h

Quoted 24 lines

Quoted 23 lines

Quoted 17 lines

Quoted 5 lines

include/configs/ib62x0.h new file +4 -0

+#define CONFIG_MVSATA_IDE_USE_PORT0
+# ifdef CONFIG_BOARD_IS_IB_NAS6210
+#  undef CONFIG_SYS_IDE_MAXBUS
+#  define CONFIG_SYS_IDE_MAXBUS		1

include/configs/ib62x0.h line 154

Quoted 10 lines

include/configs/ib62x0.h new file +10 -0

+#  undef CONFIG_SYS_IDE_MAXDEVICE
+#  define CONFIG_SYS_IDE_MAXDEVICE	1
+# elif CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_MVSATA_IDE_USE_PORT1
+# endif
+#define CONFIG_SYS_ATA_IDE0_OFFSET	KW_SATA_PORT0_OFFSET
+# ifdef CONFIG_BOARD_IS_IB_NAS6220
+#  define CONFIG_SYS_ATA_IDE1_OFFSET	KW_SATA_PORT1_OFFSET
+# endif
+#endif /* CONFIG_CMD_IDE */

please don't use this "#[space][space]define" convention.

I must disagree here. This is more readable and there are many examples in u-boot that use that syntax:

$ egrep '#[ ]+define' * -r | wc -l
12557

Quoted 3 lines

Again, the fact that it's used doesn't mean it's correct. It's not more readable actually, it's readable in the same way given you have good editor. Also, doesn't checkpatch.pl caugh on this stuff ?

checkpatch.pl is ok with this. It's readable only if your editor uses different colors for this, and I would not like to go into editor fight here. I use vim probably as you but that is not important.

I'll resend
v4 with this indentation unless Wolfgang has some objections.

If indentation is wrong in all other places in u-boot we can easily fix that with some sed magic.

This is my proposal - I'll resend v4 and it should be ok to commit without fixes for:

1) IB62x0_OE_LOW and IB62x0_OE_HIGH
2) CONFIG_SKIP_LOWLEVEL_INIT
3) ifdef indentation

Because fixing the 1) and 2) is more than adding support for this new board, and if it was in the same patch I would need to separate it. That is a different issue.

I'll put on my TODO list, and work on this after commit:

* replace tabs with spaces in boards.config
* look at IB62x0_OE_LOW and IB62x0_OE_HIGH issue
* look at CONFIG_SKIP_LOWLEVEL_INIT issue

If nobody has objections I'll resend v4...

Bye,
Luka

Quoted 11 lines

Quoted 10 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board lowlevel_init

*/

Quoted 4 lines

Quoted 2 lines

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

I will retest and send v4 once I get your feedback on other items.

Ok, what's the result? From IRC I take it you must define this ... why?

Quoted 5 lines

It generates error when building without it:

/home/luka/uboot/arch/arm/cpu/arm926ejs/start.S:393: undefined reference to `lowlevel_init' arm-openwrt-linux-ld: BFD (GNU Binutils) 2.22 assertion fail elf32-arm.c:13830

Define it empty in your arch/arm/cpu/..../kirkwood.c and be done with it ;-)

Quoted 7 lines

All other kirkwood targets I looked at define CONFIG_SKIP_LOWLEVEL_INIT, including the ones mentioned above; here are their configs for comparison:

include/configs/dreamplug.h
include/configs/sheevaplug.h
include/configs/dockstar.h

Why do you need to skip it? Does it hang or something?

Quoted 7 lines

This is my proposal - I'll resend v4 and it should be ok to commit without fixes for:

1) IB62x0_OE_LOW and IB62x0_OE_HIGH
2) CONFIG_SKIP_LOWLEVEL_INIT
3) ifdef indentation

Quoted 3 lines

Because fixing the 1) and 2) is more than adding support for this new board, and if it was in the same patch I would need to separate it. That is a different issue.

You can wait for Prafulla with #1 and #2, also for #2 check my comment. But we have two bugs going on for granted here at least and they're not your boards fault. On the other hand, it'd be cool if you could fix them prior to adding your board ;-)

Quoted 5 lines

I'll put on my TODO list, and work on this after commit:

* replace tabs with spaces in boards.config
* look at IB62x0_OE_LOW and IB62x0_OE_HIGH issue
* look at CONFIG_SKIP_LOWLEVEL_INIT issue

Hi Marek,

Quoted 13 lines · Marek Vasut

Quoted 13 lines

Quoted 12 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

+#define CONFIG_SKIP_LOWLEVEL_INIT /* disable board
lowlevel_init

*/

Quoted 4 lines

Quoted 2 lines

Are you sure you want to skip lowlevel init? It'll break cache setup etc. I believe.

I will retest and send v4 once I get your feedback on other items.

Ok, what's the result? From IRC I take it you must define this ... why?

Quoted 8 lines

Quoted 5 lines

It generates error when building without it:

/home/luka/uboot/arch/arm/cpu/arm926ejs/start.S:393: undefined reference to `lowlevel_init' arm-openwrt-linux-ld: BFD (GNU Binutils) 2.22 assertion fail elf32-arm.c:13830

Define it empty in your arch/arm/cpu/..../kirkwood.c and be done with it ;-)

Yes, this seems to fix it:

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c index fba5e01..ec2026c 100644

arch/arm/cpu/arm926ejs/kirkwood/cpu.c +2 -0

@@ -33,6 +33,8 @@
 
 #define BUFLEN 16
 
+inline void lowlevel_init(void) {}
+
 void reset_cpu(unsigned long ignored)
 {
        struct kwcpu_registers *cpureg =

include/configs/ib62x0.h +0 -1

index 1c4778d..9808a04 100644
@@ -43,7 +43,6 @@
 #define CONFIG_KIRKWOOD                        /* SOC Family Name */
 #define CONFIG_KW88F6281               /* SOC Name */
 #define CONFIG_MACH_NAS6210            /* Machine type */
-#define CONFIG_SKIP_LOWLEVEL_INIT      /* disable board lowlevel_init */
 
 /*
  * Other required minimal configurations

I'll clean this up and resend after we commit this...

Quoted 5 lines

Quoted 5 lines

All other kirkwood targets I looked at define CONFIG_SKIP_LOWLEVEL_INIT, including the ones mentioned above; here are their configs for comparison:

Quoted 5 lines

Quoted 3 lines

include/configs/dreamplug.h
include/configs/sheevaplug.h
include/configs/dockstar.h

Why do you need to skip it? Does it hang or something?

See above. I guess compile error also for other boards.

Quoted 8 lines

Quoted 8 lines

This is my proposal - I'll resend v4 and it should be ok to commit

without fixes for:
1) IB62x0_OE_LOW and IB62x0_OE_HIGH
2) CONFIG_SKIP_LOWLEVEL_INIT
3) ifdef indentation

Quoted 8 lines

Quoted 3 lines

Because fixing the 1) and 2) is more than adding support for this new board, and if it was in the same patch I would need to separate it. That is a different issue.

You can wait for Prafulla with #1 and #2, also for #2 check my comment. But we have two bugs going on for granted here at least and they're not your boards fault. On the other hand, it'd be cool if you could fix them prior to adding your board ;-)

I'll resend v4 now and work on patches for this stuff later.

Quoted 4 lines

Quoted 4 lines

I'll put on my TODO list, and work on this after commit:
* replace tabs with spaces in boards.config
* look at IB62x0_OE_LOW and IB62x0_OE_HIGH issue
* look at CONFIG_SKIP_LOWLEVEL_INIT issue

For this one we have a patch now :)

Thank you Marek.

Bye,
Luka

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 12:52
To: Luka Perkov
Cc: u-boot at lists.denx.de; dreagle at doukki.net; Wolfgang Denk; Prafulla Wadaskar
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Luka Perkov,

Quoted 2 lines

Hi Marek,

Quoted 1 line

Quoted 1 line

Quoted 1 line

This is my proposal - I'll resend v4 and it should be ok to

commit

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

without fixes for:
1) IB62x0_OE_LOW and IB62x0_OE_HIGH
2) CONFIG_SKIP_LOWLEVEL_INIT
3) ifdef indentation

Because fixing the 1) and 2) is more than adding support for

this new

Quoted 1 line

Quoted 1 line

Quoted 1 line

board, and if it was in the same patch I would need to separate

it.

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

That is a different issue.

You can wait for Prafulla with #1 and #2, also for #2 check my

comment.

Quoted 1 line

Quoted 1 line

But we have two bugs going on for granted here at least and

they're not

Quoted 1 line

Quoted 1 line

your boards fault. On the other hand, it'd be cool if you could

fix them

Quoted 1 line

Quoted 1 line

prior to adding your board ;-)

Quoted 11 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 12:52
To: Luka Perkov
Cc: u-boot at lists.denx.de; dreagle at doukki.net; Wolfgang Denk; Prafulla Wadaskar
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Luka Perkov,

Quoted 1 line

Hi Marek,

...snip...

Quoted 23 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

This is my proposal - I'll resend v4 and it should be ok to

commit

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

without fixes for:
1) IB62x0_OE_LOW and IB62x0_OE_HIGH
2) CONFIG_SKIP_LOWLEVEL_INIT
3) ifdef indentation

Because fixing the 1) and 2) is more than adding support for

this new

Quoted 1 line

Quoted 1 line

Quoted 1 line

board, and if it was in the same patch I would need to separate

it.

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

That is a different issue.

You can wait for Prafulla with #1 and #2, also for #2 check my

comment.

Quoted 9 lines

Quoted 1 line

Quoted 1 line

But we have two bugs going on for granted here at least and

they're not

Quoted 1 line

Quoted 1 line

your boards fault. On the other hand, it'd be cool if you could

fix them

Quoted 1 line

Quoted 1 line

prior to adding your board ;-)

Hi Luka

#1: Defining these values as 0xffffffff, indicates that all GPIOs are configured high by default. so this configuration solely depends upon your board requirement.

#2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since lowlevel_init is not needed on Kirkwood platforms. (ref:
doc/README.kwbimage)

Regards..
Prafulla . . .

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 15:33
To: Prafulla Wadaskar
Cc: Luka Perkov; u-boot at lists.denx.de; dreagle at doukki.net; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Prafulla Wadaskar,

Quoted 4 lines

Hi Luka

#1: Defining these values as 0xffffffff, indicates that all GPIOs

are

Quoted 1 line

configured high by default. so this configuration solely depends

upon your

Quoted 2 lines

board requirement.

Quoted 3 lines

#2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since lowlevel_init is not needed on Kirkwood platforms. (ref:
doc/README.kwbimage)

Prafulla, you're then missing the fiddling with CPSR bits, which might be quite
necessary.

Quoted 9 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 15:33
To: Prafulla Wadaskar
Cc: Luka Perkov; u-boot at lists.denx.de; dreagle at doukki.net; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Prafulla Wadaskar,

...snip...

Quoted 16 lines

Quoted 3 lines

Hi Luka

#1: Defining these values as 0xffffffff, indicates that all GPIOs

are

Quoted 1 line

configured high by default. so this configuration solely depends

upon your

Quoted 5 lines

board requirement.

#2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT since lowlevel_init is not needed on Kirkwood platforms. (ref:
doc/README.kwbimage)

Quoted 3 lines

Prafulla, you're then missing the fiddling with CPSR bits, which might be quite
necessary.

Hi Marek.
May be, may you please explain these bits? Or any pointers? Can't these be addressed in kwimage.cfg?

Regards..
Prafulla . . .

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 16:27
To: Prafulla Wadaskar
Cc: Luka Perkov; u-boot at lists.denx.de; dreagle at doukki.net; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Prafulla Wadaskar,

Quoted 5 lines

Quoted 5 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: 21 March 2012 15:33
To: Prafulla Wadaskar
Cc: Luka Perkov; u-boot at lists.denx.de; dreagle at doukki.net;

Wolfgang

Quoted 10 lines

Quoted 4 lines

Denk
Subject: Re: [U-Boot] [PATCH v2] add new board nas62x0

Dear Prafulla Wadaskar,

...snip...

Quoted 3 lines

Quoted 3 lines

Hi Luka

#1: Defining these values as 0xffffffff, indicates that all

GPIOs

Quoted 10 lines

Quoted 10 lines

are

Quoted 1 line

configured high by default. so this configuration solely depends

upon your

Quoted 3 lines

board requirement.

#2: on kirkwood, you should define CONFIG_SKIP_LOWLEVEL_INIT

since

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

lowlevel_init is not needed on Kirkwood platforms. (ref:
doc/README.kwbimage)

Prafulla, you're then missing the fiddling with CPSR bits, which

might

Quoted 3 lines

Quoted 2 lines

be quite
necessary.

Quoted 2 lines

Hi Marek.
May be, may you please explain these bits? Or any pointers? Can't

these be

Quoted 1 line

addressed in kwimage.cfg?

I have no idea, I'm no kirkwood expert. And about these bits, check start.S,
what it does with them.

Dear Luka Perkov,

Quoted 10 lines

Hi Marek,

Quoted 3 lines · Marek Vasut

Quoted 1 line

+ * Copyright (C) 2011 G??rald Kerma <[email protected]>

Can you please fix your name here?

I think your mail agent is not displaying UTF8 characters correctly. If this is a problem we could change it if Gerald is ok with that. Nobody else had problems with this...

Hi Luka,

Le 19/03/2012 16:50, Marek Vasut a ?crit :

Quoted 9 lines

Dear Luka Perkov,

Quoted 7 lines

Hi Marek,

Quoted 3 lines · Marek Vasut

Quoted 1 line

+ * Copyright (C) 2011 G??rald Kerma <[email protected]>

Can you please fix your name here?

Quoted 3 lines

Quoted 3 lines

I think your mail agent is not displaying UTF8 characters correctly. If this is a problem we could change it if Gerald is ok with that. Nobody else had problems with this...

Okay for me.

Please, replace by ... Gerald Kerma <[email protected]>

Regards,
---
Gk2

:-]