Skip to contentSkip to conversation

83xx: add support for ve8313 board

U-Boot New Heiko Schocher 5 participants 11 messages Jul 5, 2010 – Jul 9, 2010

lore
Share & tools
lore
#0 lore raw

This patch add support for the ve8313 board based on Freescale MPC8313 CPU.

- serial console on UART 1
- 128 MB DDR RAM
- 32 MB NOR Flash
- 16 MB NAND Flash
- Ethernet MII Mode on TSEC0
- micrel ksz804 phy
- Hardware WDT MAX824

---
 board/ve8313/Makefile    |   50 +++++
 board/ve8313/config.mk   |   10 +
 board/ve8313/ve8313.c    |  212 ++++++++++++++++++
 boards.cfg               |    1 +
 include/configs/ve8313.h |  534 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 807 insertions(+), 0 deletions(-)
 create mode 100644 board/ve8313/Makefile
 create mode 100644 board/ve8313/config.mk
 create mode 100644 board/ve8313/ve8313.c
 create mode 100644 include/configs/ve8313.h
board/ve8313/Makefile new file +50 -0
new file mode 100644
index 0000000..c95f90e
@@ -0,0 +1,50 @@
+#
+# (C) Copyright 2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# 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., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)lib$(BOARD).a
+
+COBJS	:= $(BOARD).o
+
+SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+SOBJS	:= $(addprefix $(obj),$(SOBJS))
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(AR) $(ARFLAGS) $@ $(OBJS)
+
+clean:
+	rm -f $(SOBJS) $(OBJS)
+
+distclean:	clean
+	rm -f $(LIB) core *.bak $(obj).depend
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
board/ve8313/config.mk new file +10 -0
new file mode 100644
index 0000000..66fbc10
@@ -0,0 +1,10 @@
+ifndef NAND_SPL
+sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
+endif
+
+ifndef TEXT_BASE
+#TEXT_BASE = 0x100000
+TEXT_BASE = 0xfe000000
+endif
+
+#PLATFORM_CPPFLAGS += -DDEBUG
board/ve8313/ve8313.c new file +212 -0

212 lines not on this page · raw message

boards.cfg +1 -0
index 1a5cfb1..0de35bf 100644
@@ -135,6 +135,7 @@ TQM8272		powerpc	mpc8260		tqm8272		tqc
 kmeter1		powerpc	mpc83xx		kmeter1		keymile
 MVBLM7		powerpc	mpc83xx		mvblm7		matrix_vision
 TQM834x		powerpc	mpc83xx		tqm834x		tqc
+ve8313		powerpc	mpc83xx		ve8313
 PM854		powerpc	mpc85xx		pm854
 PM856		powerpc	mpc85xx		pm856
 stxgp3		powerpc	mpc85xx		stxgp3		stx
include/configs/ve8313.h new file +534 -0

534 lines not on this page · raw message

Signature · 5 lines
1.6.2.5

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#1 lore raw
Quoted 7 lines · On Mon, 5 Jul 2010 12:23:10 +0200 Heiko Schocher <[email protected]> wrote:
---
 board/ve8313/Makefile    |   50 +++++
 board/ve8313/config.mk   |   10 +
 board/ve8313/ve8313.c    |  212 ++++++++++++++++++
 boards.cfg               |    1 +
 include/configs/ve8313.h |  534 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 807 insertions(+), 0 deletions(-)

missing MAKEALL, MAINTAINERS entries, and, if there's anything unique about this board, a doc/README.ve8313.

board/ve8313/config.mk line 10

Quoted 8 lines
board/ve8313/config.mk +6 -0
 
+ifndef TEXT_BASE
+#TEXT_BASE = 0x100000
+TEXT_BASE = 0xfe000000
+endif
+
+#PLATFORM_CPPFLAGS += -DDEBUG

please clean this up - remove commented out code.

board/ve8313/ve8313.c line 3

Quoted 9 lines
board/ve8313/ve8313.c new file +3 -0
new file mode 100644
index 0000000..b13d1f3
@@ -0,0 +1,212 @@
+/*
+ * (C) Copyright 2010
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.

this file contains code that looks like it was copied from files with Freescale copyrights; why are you deleting Freescale's copyrights?

board/ve8313/ve8313.c line 27

Quoted 3 lines
board/ve8313/ve8313.c new file +3 -0
+#if defined(CONFIG_OF_LIBFDT)
+#include <libfdt.h>
+#endif

I realise this was copied in, but we don't need the ifdef anymore.

board/ve8313/ve8313.c line 51

Quoted 5 lines
board/ve8313/ve8313.c new file +5 -0
+/* Fixed sdram init -- doesn't use serial presence detect.
+ *
+ * This is useful for faster booting in configs where the RAM is unlikely
+ * to be changed, or for things like NAND booting where space is tight.
+ */

Is the memory soldered on to the board? If not, does the serial presence detect code not work for you?

board/ve8313/ve8313.c line 139

Quoted 7 lines
board/ve8313/ve8313.c new file +7 -0
+#if defined(CONFIG_HW_WATCHDOG)
+	clrbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));
+	/* set WDT pins as output */
+	setbits_be32(&gpio->dir, (VE8313_WDT_EN | VE8313_WDT_TRIG));
+#else
+	/* disable WDT */
+	setbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));

inner parens not necessary, also the last WDT_EN bit setting doesn't match the comment above it (ENable vs. disable) - perhaps nothing need be done if HW_WATCHDOG is not set?

board/ve8313/ve8313.c line 151

Quoted 2 lines
board/ve8313/ve8313.c new file +2 -0
+	setbits_be32(&gpio->dat, (VE8313_WDT_TRIG));
+	clrbits_be32(&gpio->dat, (VE8313_WDT_TRIG));

inner parens not necessary.

board/ve8313/ve8313.c line 21

Quoted 1 line
board/ve8313/ve8313.c new file +1 -0
+#undef CONFIG_HW_WATCHDOG

no #undefs please.

board/ve8313/ve8313.c line 22

Quoted 1 line
board/ve8313/ve8313.c new file +1 -0
+#define CONFIG_SYS_LBC_LBCR	(0x00040000)

parens not necessary.

board/ve8313/ve8313.c line 23

Quoted 1 line
board/ve8313/ve8313.c new file +1 -0
+#define CONFIG_SYS_PCI_SUBSYS_VENDORID 0x1057	/* Motorola */

It's Freescale: 0x1957.

Thanks,

Kim

#2 lore raw

Hello Kim,

Quoted 11 lines · Kim Phillips
Quoted 7 lines · On Mon, 5 Jul 2010 12:23:10 +0200 Heiko Schocher <[email protected]> wrote:
---
 board/ve8313/Makefile    |   50 +++++
 board/ve8313/config.mk   |   10 +
 board/ve8313/ve8313.c    |  212 ++++++++++++++++++
 boards.cfg               |    1 +
 include/configs/ve8313.h |  534 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 807 insertions(+), 0 deletions(-)
Quoted 2 lines

missing MAKEALL, MAINTAINERS entries, and, if there's anything unique about this board, a doc/README.ve8313.

There is nothing unique on this board, so don;t think a doc/README.ve8313 is useful.

Quoted 10 lines

board/ve8313/config.mk line 10

Quoted 8 lines
board/ve8313/config.mk +6 -0
 
+ifndef TEXT_BASE
+#TEXT_BASE = 0x100000
+TEXT_BASE = 0xfe000000
+endif
+
+#PLATFORM_CPPFLAGS += -DDEBUG

please clean this up - remove commented out code.

fixed.

Quoted 4 lines
Quoted 4 lines
board/ve8313/ve8313.c new file +0 -1
new file mode 100644
index 0000000..b13d1f3
--- /dev/null
Quoted 8 lines

board/ve8313/ve8313.c line 3

Quoted 5 lines

+++ b/board/ve8313/ve8313.c

board/ve8313/ve8313.c new file +3 -0
@@ -0,0 +1,212 @@
+/*
+ * (C) Copyright 2010
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.

this file contains code that looks like it was copied from files with Freescale copyrights; why are you deleting Freescale's copyrights?

Hups, Sorry for that.

Quoted 5 lines

board/ve8313/ve8313.c line 27

Quoted 3 lines
board/ve8313/ve8313.c new file +3 -0
+#if defined(CONFIG_OF_LIBFDT)
+#include <libfdt.h>
+#endif

I realise this was copied in, but we don't need the ifdef anymore.

done.

Quoted 8 lines

board/ve8313/ve8313.c line 51

Quoted 5 lines
board/ve8313/ve8313.c new file +5 -0
+/* Fixed sdram init -- doesn't use serial presence detect.
+ *
+ * This is useful for faster booting in configs where the RAM is unlikely
+ * to be changed, or for things like NAND booting where space is tight.
+ */

Is the memory soldered on to the board? If not, does the serial presence detect code not work for you?

It is soldered.

Quoted 11 lines

board/ve8313/ve8313.c line 139

Quoted 7 lines
board/ve8313/ve8313.c new file +7 -0
+#if defined(CONFIG_HW_WATCHDOG)
+	clrbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));
+	/* set WDT pins as output */
+	setbits_be32(&gpio->dir, (VE8313_WDT_EN | VE8313_WDT_TRIG));
+#else
+	/* disable WDT */
+	setbits_be32(&gpio->dat, (VE8313_WDT_EN | VE8313_WDT_TRIG));

inner parens not necessary, also the last WDT_EN bit setting doesn't match the comment above it (ENable vs. disable) - perhaps nothing need be done if HW_WATCHDOG is not set?

I must explicitely disable the WDT.

Quoted 4 lines

board/ve8313/ve8313.c line 151

Quoted 2 lines
board/ve8313/ve8313.c new file +2 -0
+	setbits_be32(&gpio->dat, (VE8313_WDT_TRIG));
+	clrbits_be32(&gpio->dat, (VE8313_WDT_TRIG));

inner parens not necessary.

done.

Quoted 4 lines

board/ve8313/ve8313.c line 21

Quoted 2 lines
board/ve8313/ve8313.c new file +1 -0
+#undef CONFIG_HW_WATCHDOG

no #undefs please.

removed.

Quoted 3 lines

board/ve8313/ve8313.c line 22

Quoted 1 line
board/ve8313/ve8313.c new file +1 -0
+#define CONFIG_SYS_LBC_LBCR	(0x00040000)

parens not necessary.

fixed.

Quoted 3 lines

board/ve8313/ve8313.c line 23

Quoted 1 line
board/ve8313/ve8313.c new file +1 -0
+#define CONFIG_SYS_PCI_SUBSYS_VENDORID 0x1057	/* Motorola */

It's Freescale: 0x1957.

fixed.

Thanks for your review.

bye
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#3 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

In reply to Kim Phillips

This patch add support for the ve8313 board based on Freescale MPC8313 CPU.

- serial console on UART 1
- 128 MB DDR RAM
- 32 MB NOR Flash
- 16 MB NAND Flash
- Ethernet MII Mode over on TSEC0
- micrel ksz804 phy
- Hardware WDT MAX824

---
changes since v1
- environment size = sector size
- use red. environment
- add comments from Kim Phillips
- add MAKEALL, MAINTAINERS entry
- Codingstyle issues fixed
- inserted original Copyrights
- PCI subsys vendor ID changed from 0x1057 (Motorola)

    to 0x1957 (Freescale)
 MAINTAINERS              |    1 +
 MAKEALL                  |    1 +
 board/ve8313/Makefile    |   50 +++++
 board/ve8313/config.mk   |    7 +
 board/ve8313/ve8313.c    |  215 +++++++++++++++++++
 boards.cfg               |    1 +
 include/configs/ve8313.h |  534 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 809 insertions(+), 0 deletions(-)
 create mode 100644 board/ve8313/Makefile
 create mode 100644 board/ve8313/config.mk
 create mode 100644 board/ve8313/ve8313.c
 create mode 100644 include/configs/ve8313.h
MAINTAINERS +1 -0
index d7aec98..88a3d79 100644
@@ -427,6 +427,7 @@ Heiko Schocher <[email protected]>
 	sc3		PPC405GP
 	suen3		ARM926EJS (Kirkwood SoC)
 	uc101		MPC5200
+	ve8313		MPC8313
 
 Peter De Schrijver <[email protected]>
 
MAKEALL +1 -0
index 27f8bd1..eedae60 100755
@@ -379,6 +379,7 @@ LIST_83xx="		\
 	sbc8349		\
 	SIMPC8313_LP	\
 	TQM834x		\
+	ve8313		\
 	vme8349		\
 "
 
board/ve8313/Makefile new file +50 -0

50 lines not on this page · raw message

board/ve8313/config.mk new file +7 -0
new file mode 100644
index 0000000..02dd33e
@@ -0,0 +1,7 @@
+ifndef NAND_SPL
+sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
+endif
+
+ifndef TEXT_BASE
+TEXT_BASE = 0xfe000000
+endif
board/ve8313/ve8313.c new file +215 -0

215 lines not on this page · raw message

boards.cfg +1 -0
index 1a5cfb1..0de35bf 100644
@@ -135,6 +135,7 @@ TQM8272		powerpc	mpc8260		tqm8272		tqc
 kmeter1		powerpc	mpc83xx		kmeter1		keymile
 MVBLM7		powerpc	mpc83xx		mvblm7		matrix_vision
 TQM834x		powerpc	mpc83xx		tqm834x		tqc
+ve8313		powerpc	mpc83xx		ve8313
 PM854		powerpc	mpc85xx		pm854
 PM856		powerpc	mpc85xx		pm856
 stxgp3		powerpc	mpc85xx		stxgp3		stx
include/configs/ve8313.h new file +534 -0

534 lines not on this page · raw message

Signature · 1 line
1.6.2.5
#4 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

Hi Heiko,

Quoted 7 lines · On Wednesday 07 July 2010 06:28:21 Heiko Schocher wrote:

This patch add support for the ve8313 board based on Freescale MPC8313 CPU.

- serial console on UART 1
- 128 MB DDR RAM
- 32 MB NOR Flash
- 16 MB NAND Flash

Just checking: Is this really *only* 16 MiB NAND flash?

Cheers,
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

#5 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

Hello Stefan,

Quoted 10 lines · Stefan Roese
Quoted 7 lines · On Wednesday 07 July 2010 06:28:21 Heiko Schocher wrote:

This patch add support for the ve8313 board based on Freescale MPC8313 CPU.

- serial console on UART 1
- 128 MB DDR RAM
- 32 MB NOR Flash
- 16 MB NAND Flash

Just checking: Is this really *only* 16 MiB NAND flash?

Yep, it is a NAND128W3A2BN6E from Numonyx

bye
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#6 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

In reply to Heiko Schocher

Dear Heiko Schocher,

Quoted 3 lines · In message <[email protected]> you wrote:

changes since v1
- environment size = sector size

Argh. While the previous environment size of 8 KiB was indeed a bit smallish, using 128 KiB is overkill. Keep in mind that we then have to compute the checksum over 18 KiB as well, which just costs time - at booting, and with every setenv you run.

I suggest to change this to a more resaobale size - say 16 KiB ?

Quoted 5 lines

+/* Fixed sdram init -- doesn't use serial presence detect.
+ *
+ * This is useful for faster booting in configs where the RAM is unlikely
+ * to be changed, or for things like NAND booting where space is tight.
+ */

Incorrect multiline comment style.

The first line of the comment is misleading as one might think there was SPD information available but we decide not to use it - actually there is none and we cannot use it. Please fix. And since there are no other RAM configurations on this board, I recommend to remove the last 2 lines of the comment as well.

board/ve8313/ve8313.c line 144

Quoted 10 lines
board/ve8313/ve8313.c new file +10 -0
+	/* set WDT pins as output */
+	setbits_be32(&gpio->dir, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#if defined(CONFIG_HW_WATCHDOG)
+	/* enable WDT */
+	clrbits_be32(&gpio->dat, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#else
+	/* disable WDT */
+	setbits_be32(&gpio->dat, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#endif
+	return 0;

Is this the correct order? Would it not make sense first to define the state of the data bit before you enable the output? Otherwise this might result in short spikes at the wrong signal level, which here might start the watchdog by accident?

board/ve8313/ve8313.c line 155

Quoted 9 lines
board/ve8313/ve8313.c new file +9 -0
+#if defined(CONFIG_HW_WATCHDOG)
+void hw_watchdog_reset(void)
+{
+	volatile immap_t *im = (volatile immap_t *)CONFIG_SYS_IMMR;
+	volatile gpio83xx_t *gpio = (volatile gpio83xx_t *)im->gpio;
+
+	setbits_be32(&gpio->dat, VE8313_WDT_TRIG);
+	clrbits_be32(&gpio->dat, VE8313_WDT_TRIG);
+}

Is there a minimal length of the trigger pulse defined for this watchdog? Maybe some udelay() is needed here?

boards.cfg TQM8272 powerpc mpc8260 tqm8272 tqc · line 141

Quoted 10 lines
boards.cfg +1 -0
@@ -135,6 +135,7 @@ TQM8272		powerpc	mpc8260		tqm8272		tqc
 kmeter1		powerpc	mpc83xx		kmeter1		keymile
 MVBLM7		powerpc	mpc83xx		mvblm7		matrix_vision
 TQM834x		powerpc	mpc83xx		tqm834x		tqc
+ve8313		powerpc	mpc83xx		ve8313
 PM854		powerpc	mpc85xx		pm854
 PM856		powerpc	mpc85xx		pm856
 stxgp3		powerpc	mpc85xx		stxgp3		stx

This is not sorted correctly. It should go here (because of the empty vendor field).

 SCM            powerpc mpc8260         -               siemens
 TQM8272        powerpc mpc8260         tqm8272         tqc
+ve8313         powerpc mpc83xx         ve8313
 kmeter1        powerpc mpc83xx         kmeter1         keymile
 MVBLM7         powerpc mpc83xx         mvblm7          matrix_vision

The comment in the header of the file describes how to correctly sort this file; if you cannot parse this, just run the command in vi :-)

Quoted 4 lines

+/*
+ * Manually set up DDR parameters, as this board does not
+ * seem to have the SPD connected to I2C.
+ */

"does not seem to have the SPD connected to I2C"? You know this for sure, so please fix the comment.

Quoted 1 line

+#if !defined(CONFIG_SYS_NO_FLASH)

...

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+#else
+#define CONFIG_SYS_FLASH_BASE		0xFE000000
+#define CONFIG_SYS_FLASH_SIZE		32	/* size in MB */
+#endif

I think there are no configurations of this board without NOR flash either; please drop this as well.

boards.cfg

Quoted 8 lines
boards.cfg +8 -0
+/*
+ * TSEC
+ */
+#define CONFIG_TSEC_ENET		/* TSEC ethernet support */
+
+#define CONFIG_NET_MULTI
+
+#define CONFIG_TSEC1

Please drop these empty lines.

boards.cfg

Quoted 3 lines
boards.cfg +3 -0
+#ifdef CONFIG_TSEC1
+#define CONFIG_HAS_ETH0
+#define CONFIG_TSEC1_NAME      "TSEC0"

Oops? This is TSEC1, so why do you name it "TSEC0" ?

boards.cfg

Quoted 8 lines
boards.cfg +8 -0
+/*
+ * Environment
+ */
+#if !defined(CONFIG_SYS_RAMBOOT)
+	#define CONFIG_ENV_IS_IN_FLASH	1
+	#define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE + \
+						CONFIG_SYS_MONITOR_LEN)
+	#define CONFIG_ENV_SECT_SIZE	0x20000	/* 64K(one sector) for env */

Comment and code don't match - the comment is wrong - actual sector size is 128 KiB.

boards.cfg

Quoted 6 lines
boards.cfg +6 -0
+	/* Address and size of Redundant Environment Sector */
+	#define CONFIG_ENV_OFFSET_REDUND	(CONFIG_ENV_OFFSET + \
+						CONFIG_ENV_SECT_SIZE)
+	#define CONFIG_ENV_SIZE_REDUND	(CONFIG_ENV_SIZE)
+#else
+	#define CONFIG_SYS_NO_FLASH	1	/* Flash is not usable now */

Why would flash not be usable when booting from RAM?

boards.cfg

Quoted 3 lines
boards.cfg +3 -0
+	#define CONFIG_ENV_IS_NOWHERE	1	/* Store ENV in mem only */
+	#define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE - 0x1000)
+	#define CONFIG_ENV_SIZE		0x2000

I recommend to use the same CONFIG_ENV_SIZE setting for both configurations (suggestion: use (16 << 10)).

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+#if defined(CONFIG_SYS_RAMBOOT) && !defined(CONFIG_NAND_U_BOOT)
+    #undef CONFIG_CMD_SAVEENV
+    #undef CONFIG_CMD_LOADS
+#endif

Why do you disable the loads command here? This makes no sense to me.

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+/*
+ * Environment Configuration
+ */
+#define CONFIG_ENV_OVERWRITE

Please remove.

Quoted 1 line

+#define CONFIG_SYS_LOAD_ADDR 0x2000000 /* default load address */

...

Quoted 1 line

+#define CONFIG_LOADADDR 800000

Is this 800000 (= 0xc3500) or 0x800000?
and why is it different from the CONFIG_SYS_LOAD_ADDR definition above?

boards.cfg

Quoted 9 lines
boards.cfg +9 -0
+#define CONFIG_BOOTDELAY	6	/* -1 disables auto-boot */
+#define CONFIG_BAUDRATE		115200
+
+#define XMK_STR(x)	#x
+#define MK_STR(x)	XMK_STR(x)
+
+#define CONFIG_EXTRA_ENV_SETTINGS \
+	"netdev=" MK_STR(CONFIG_NETDEV) "\0"				\
+	"ethprime=TSEC0\0"						\

Use CONFIG_TSEC1_NAME here?

Best regards,

Wolfgang Denk

Signature · 7 lines
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
Severe culture shock results when experts from another protocol suite
[...] try to read OSI documents. The term "osified" is used to  refer
to  such  documents. [...] Any relationship to the word "ossified" is
purely intentional.                                - Marshall T. Rose
#7 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

Hello Wolfgang,

Quoted 10 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:

changes since v1
- environment size = sector size

Argh. While the previous environment size of 8 KiB was indeed a bit smallish, using 128 KiB is overkill. Keep in mind that we then have to compute the checksum over 18 KiB as well, which just costs time - at booting, and with every setenv you run.

I suggest to change this to a more resaobale size - say 16 KiB ?

Ok, done.

Quoted 6 lines
Quoted 5 lines

+/* Fixed sdram init -- doesn't use serial presence detect.
+ *
+ * This is useful for faster booting in configs where the RAM is unlikely
+ * to be changed, or for things like NAND booting where space is tight.
+ */

Quoted 7 lines

Incorrect multiline comment style.

The first line of the comment is misleading as one might think there was SPD information available but we decide not to use it - actually there is none and we cannot use it. Please fix. And since there are no other RAM configurations on this board, I recommend to remove the last 2 lines of the comment as well.

removed.

Quoted 7 lines
Quoted 7 lines
board/ve8313/ve8313.c new file +7 -0
+	/* set WDT pins as output */
+	setbits_be32(&gpio->dir, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#if defined(CONFIG_HW_WATCHDOG)
+	/* enable WDT */
+	clrbits_be32(&gpio->dat, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#else
+	/* disable WDT */
Quoted 8 lines

board/ve8313/ve8313.c line 144

Quoted 3 lines
board/ve8313/ve8313.c new file +3 -0
+	setbits_be32(&gpio->dat, VE8313_WDT_EN | VE8313_WDT_TRIG);
+#endif
+	return 0;

Is this the correct order? Would it not make sense first to define the state of the data bit before you enable the output? Otherwise this might result in short spikes at the wrong signal level, which here might start the watchdog by accident?

Yup, fix this.

Quoted 4 lines
Quoted 4 lines
board/ve8313/ve8313.c new file +4 -0
+#if defined(CONFIG_HW_WATCHDOG)
+void hw_watchdog_reset(void)
+{
+	volatile immap_t *im = (volatile immap_t *)CONFIG_SYS_IMMR;
Quoted 8 lines

board/ve8313/ve8313.c line 155

Quoted 5 lines
board/ve8313/ve8313.c new file +5 -0
+	volatile gpio83xx_t *gpio = (volatile gpio83xx_t *)im->gpio;
+
+	setbits_be32(&gpio->dat, VE8313_WDT_TRIG);
+	clrbits_be32(&gpio->dat, VE8313_WDT_TRIG);
+}

Is there a minimal length of the trigger pulse defined for this watchdog? Maybe some udelay() is needed here?

I check this.

Quoted 5 lines
Quoted 5 lines
boards.cfg +0 -0
@@ -135,6 +135,7 @@ TQM8272		powerpc	mpc8260		tqm8272		tqc
 kmeter1		powerpc	mpc83xx		kmeter1		keymile
 MVBLM7		powerpc	mpc83xx		mvblm7		matrix_vision
Quoted 8 lines

boards.cfg TQM8272 powerpc mpc8260 tqm8272 tqc · line 141

Quoted 5 lines
boards.cfg +1 -0
 TQM834x		powerpc	mpc83xx		tqm834x		tqc
+ve8313		powerpc	mpc83xx		ve8313
 PM854		powerpc	mpc85xx		pm854
 PM856		powerpc	mpc85xx		pm856
 stxgp3		powerpc	mpc85xx		stxgp3		stx

This is not sorted correctly. It should go here (because of the empty vendor field).

fixed.

Quoted 10 lines
 SCM            powerpc mpc8260         -               siemens
 TQM8272        powerpc mpc8260         tqm8272         tqc
+ve8313         powerpc mpc83xx         ve8313
 kmeter1        powerpc mpc83xx         kmeter1         keymile
 MVBLM7         powerpc mpc83xx         mvblm7          matrix_vision

The comment in the header of the file describes how to correctly sort this file; if you cannot parse this, just run the command in vi :-)

Quoted 7 lines
Quoted 4 lines

+/*
+ * Manually set up DDR parameters, as this board does not
+ * seem to have the SPD connected to I2C.
+ */

"does not seem to have the SPD connected to I2C"? You know this for sure, so please fix the comment.

fixed

Quoted 9 lines
Quoted 1 line

+#if !defined(CONFIG_SYS_NO_FLASH)

...

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+#else
+#define CONFIG_SYS_FLASH_BASE		0xFE000000
+#define CONFIG_SYS_FLASH_SIZE		32	/* size in MB */
+#endif

I think there are no configurations of this board without NOR flash either; please drop this as well.

fixed.

Quoted 11 lines

boards.cfg

Quoted 8 lines
boards.cfg +8 -0
+/*
+ * TSEC
+ */
+#define CONFIG_TSEC_ENET		/* TSEC ethernet support */
+
+#define CONFIG_NET_MULTI
+
+#define CONFIG_TSEC1

Please drop these empty lines.

Quoted 5 lines

boards.cfg

Quoted 3 lines
boards.cfg +3 -0
+#ifdef CONFIG_TSEC1
+#define CONFIG_HAS_ETH0
+#define CONFIG_TSEC1_NAME      "TSEC0"

Oops? This is TSEC1, so why do you name it "TSEC0" ?

Because this is so in all other board configs I looked.

Kim? Can you comment this?
(I think it is because to be in sync with "eth0" ...)

Quoted 11 lines

boards.cfg

Quoted 8 lines
boards.cfg +8 -0
+/*
+ * Environment
+ */
+#if !defined(CONFIG_SYS_RAMBOOT)
+	#define CONFIG_ENV_IS_IN_FLASH	1
+	#define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE + \
+						CONFIG_SYS_MONITOR_LEN)
+	#define CONFIG_ENV_SECT_SIZE	0x20000	/* 64K(one sector) for env */

Comment and code don't match - the comment is wrong - actual sector size is 128 KiB.

fixed.

Quoted 8 lines

boards.cfg

Quoted 6 lines
boards.cfg +6 -0
+	/* Address and size of Redundant Environment Sector */
+	#define CONFIG_ENV_OFFSET_REDUND	(CONFIG_ENV_OFFSET + \
+						CONFIG_ENV_SECT_SIZE)
+	#define CONFIG_ENV_SIZE_REDUND	(CONFIG_ENV_SIZE)
+#else
+	#define CONFIG_SYS_NO_FLASH	1	/* Flash is not usable now */

Why would flash not be usable when booting from RAM?

Argh, this is a leftover from the first hardware version, where the flash didn;t worked. Good catch remove this.

Quoted 7 lines

boards.cfg

Quoted 3 lines
boards.cfg +3 -0
+	#define CONFIG_ENV_IS_NOWHERE	1	/* Store ENV in mem only */
+	#define CONFIG_ENV_ADDR		(CONFIG_SYS_MONITOR_BASE - 0x1000)
+	#define CONFIG_ENV_SIZE		0x2000

I recommend to use the same CONFIG_ENV_SIZE setting for both configurations (suggestion: use (16 << 10)).

Quoted 6 lines

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+#if defined(CONFIG_SYS_RAMBOOT) && !defined(CONFIG_NAND_U_BOOT)
+    #undef CONFIG_CMD_SAVEENV
+    #undef CONFIG_CMD_LOADS
+#endif

Why do you disable the loads command here? This makes no sense to me.

yep, fixed.

Quoted 6 lines

boards.cfg

Quoted 4 lines
boards.cfg +4 -0
+/*
+ * Environment Configuration
+ */
+#define CONFIG_ENV_OVERWRITE

Please remove.

done.

Quoted 6 lines
Quoted 1 line

+#define CONFIG_SYS_LOAD_ADDR 0x2000000 /* default load address */

...

Quoted 1 line

+#define CONFIG_LOADADDR 800000

Is this 800000 (= 0xc3500) or 0x800000?
and why is it different from the CONFIG_SYS_LOAD_ADDR definition above?

removed this here and fixed CONFIG_SYS_LOAD_ADDR to 0x100000

Quoted 11 lines

boards.cfg

Quoted 9 lines
boards.cfg +9 -0
+#define CONFIG_BOOTDELAY	6	/* -1 disables auto-boot */
+#define CONFIG_BAUDRATE		115200
+
+#define XMK_STR(x)	#x
+#define MK_STR(x)	XMK_STR(x)
+
+#define CONFIG_EXTRA_ENV_SETTINGS \
+	"netdev=" MK_STR(CONFIG_NETDEV) "\0"				\
+	"ethprime=TSEC0\0"						\

Use CONFIG_TSEC1_NAME here?

done

Thanks for the review

bye
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#8 lore raw

PATCH v2 1/1 83xx: add support for ve8313 board

Dear Heiko Schocher,

Quoted 11 lines · In message <[email protected]> you wrote:
Quoted 6 lines

include/configs/ve8313.h line 312

Quoted 4 lines
include/configs/ve8313.h new file +3 -0
+#ifdef CONFIG_TSEC1
+#define CONFIG_HAS_ETH0
+#define CONFIG_TSEC1_NAME      "TSEC0"

Oops? This is TSEC1, so why do you name it "TSEC0" ?

Because this is so in all other board configs I looked.

Kim? Can you comment this?
(I think it is because to be in sync with "eth0" ...)

That is not a good reason to confuse the users. TSEC0 and TSEC1 are different interfaces, and to bend names is definitely a bad idea.

Best regards,

Wolfgang Denk

Signature · 5 lines
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
The trouble with our times is that the future is not what it used  to
be.                                                     - Paul Valery
#9 lore raw

PATCH v3 1/1 83xx: add support for ve8313 board

In reply to Heiko Schocher

This patch add support for the ve8313 board based on Freescale MPC8313 CPU.

- serial console on UART 1
- 128 MB DDR RAM
- 32 MB NOR Flash
- 16 MB NAND Flash
- Ethernet MII Mode over on TSEC0
- micrel ksz804 phy
- Hardware WDT MAX824

changes since v1
- Environment size = sector size
- use red. environment
- add comments from Kim Phillips
- add MAKEALL, MAINTAINERS entry
- Codingstyle issues fixed
- inserted original Copyrights
- PCI subsys vendor ID changed from 0x1057 (Motorola)

    to 0x1957 (Freescale)

changes since v2
- add comments from Wolfgang Denk
- fix Codingstyle and some comments
- reworked WDT reset (just toggling the WD_TRIG pin)
- Environment size now 16KiB
- fixed RAMBOOT version
- fixed CONFIG_SYS_LOAD_ADDR
- renamed CONFIG_TSEC1_NAME to TSEC1

---
 MAINTAINERS              |    1 +
 MAKEALL                  |    1 +
 board/ve8313/Makefile    |   50 +++++
 board/ve8313/config.mk   |    7 +
 board/ve8313/ve8313.c    |  215 +++++++++++++++++++
 boards.cfg               |    1 +
 include/configs/ve8313.h |  511 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 786 insertions(+), 0 deletions(-)
 create mode 100644 board/ve8313/Makefile
 create mode 100644 board/ve8313/config.mk
 create mode 100644 board/ve8313/ve8313.c
 create mode 100644 include/configs/ve8313.h
MAINTAINERS +1 -0
index d7aec98..88a3d79 100644
@@ -427,6 +427,7 @@ Heiko Schocher <[email protected]>
 	sc3		PPC405GP
 	suen3		ARM926EJS (Kirkwood SoC)
 	uc101		MPC5200
+	ve8313		MPC8313
 
 Peter De Schrijver <[email protected]>
 
MAKEALL +1 -0
index 27f8bd1..eedae60 100755
@@ -379,6 +379,7 @@ LIST_83xx="		\
 	sbc8349		\
 	SIMPC8313_LP	\
 	TQM834x		\
+	ve8313		\
 	vme8349		\
 "
 
board/ve8313/Makefile new file +50 -0

50 lines not on this page · raw message

board/ve8313/config.mk new file +7 -0
new file mode 100644
index 0000000..02dd33e
@@ -0,0 +1,7 @@
+ifndef NAND_SPL
+sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
+endif
+
+ifndef TEXT_BASE
+TEXT_BASE = 0xfe000000
+endif
board/ve8313/ve8313.c new file +215 -0

215 lines not on this page · raw message

boards.cfg +1 -0
index 1a5cfb1..cad8ba4 100644
@@ -132,6 +132,7 @@ ZPC1900		powerpc	mpc8260		zpc1900
 mgcoge		powerpc	mpc8260		-		keymile
 SCM		powerpc	mpc8260		-		siemens
 TQM8272		powerpc	mpc8260		tqm8272		tqc
+ve8313		powerpc	mpc83xx		ve8313
 kmeter1		powerpc	mpc83xx		kmeter1		keymile
 MVBLM7		powerpc	mpc83xx		mvblm7		matrix_vision
 TQM834x		powerpc	mpc83xx		tqm834x		tqc
include/configs/ve8313.h new file +511 -0

511 lines not on this page · raw message

Signature · 6 lines
1.6.2.5


-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany