SPEAr: Update platform support for SPEAr3xx/6xx

While the u-boot code is running from the flash, it is essential that no access is made to the bss segment. This is due to the fact that .rel.dyn and .bss areas overlap and former contains information used in relocation. In SPEAr, this was not taken into consideration. As a result, while the relocation wasn't complete, dram_init populated an uninitialized global variable resulting in corruption of .rel.dyn area, which resulted in u-boot crash.

This commit fixes this problem by removing code that accesses bss segment

Signed-off-by Amit Virdi <[email protected]>

Xloader table was used primarily to inform u-boot about the DDR size. However, now the ddr size is calculated at runtime which eliminates any need for the Xloader table. So removing this unnecessary code.

Signed-off-by Amit Virdi <[email protected]>

From: Vipin KUMAR <[email protected]>

ethaddr can be optionally read from i2c memory. So, chip_config command supports reading/writing hw mac id into i2c memory. Placing this code within CONFIG_CMD_NET as this would only be needed when network interface is configured

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

Quoted 7 lines · On Wednesday 07 March 2012 13:03:52 Amit Virdi wrote:

From: Vipin KUMAR <[email protected]>

ethaddr can be optionally read from i2c memory. So, chip_config command supports reading/writing hw mac id into i2c memory. Placing this code within CONFIG_CMD_NET as this would only be needed when network interface is configured

Quoted 2 lines

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

I'm not really sure, why you have this special ethaddr handling in I2C prom here at all. Isn't it enough to have the MAC address (ethaddr) stored in the U-Boot environment (NOR, NAND, etc)? Why do you need this additional I2C stuff? Could you please explain this?

Quoted 7 lines · On Wednesday 07 March 2012 13:03:52 Amit Virdi wrote:

From: Vipin KUMAR<[email protected]>

ethaddr can be optionally read from i2c memory. So, chip_config command supports reading/writing hw mac id into i2c memory. Placing this code within CONFIG_CMD_NET as this would only be needed when network interface is configured

Quoted 2 lines

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

I'm not really sure, why you have this special ethaddr handling in I2C prom here at all. Isn't it enough to have the MAC address (ethaddr) stored in the U-Boot environment (NOR, NAND, etc)? Why do you need this additional I2C stuff? Could you please explain this?

Generally MAC addresses are stored in EEPROMs due to two reasons:
1. Special set of command sequences are required to erase data from an EEPROM
2. EEPROMs are cheaper

From: Vipin KUMAR <[email protected]>

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 arch/arm/include/asm/arch-spear/hardware.h |    1 +
 board/spear/spear300/spear300.c            |   10 ++++++++++
 board/spear/spear310/spear310.c            |   10 ++++++++++
 board/spear/spear320/spear320.c            |   10 ++++++++++
 board/spear/spear600/spear600.c            |   10 ++++++++++
 include/configs/spear-common.h             |   14 ++++++++++++--
 include/configs/spear3xx.h                 |    3 +++
 7 files changed, 56 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-spear/hardware.h index a6517b2..2b9cb0e 100644

arch/arm/include/asm/arch-spear/hardware.h +0 -0

 #define CONFIG_SYS_NAND_CLE			(1 << 16)
 #define CONFIG_SYS_NAND_ALE			(1 << 17)

(quoted diff) +0 -0

b/board/spear/spear300/spear300.c index 32bcb77..3f7ccb8 100644

board/spear/spear300/spear300.c +1 -0

@@ -22,6 +22,7 @@
  */
 
 #include <common.h>
+#include <netdev.h>
 #include <nand.h>
 #include <asm/io.h>
 #include <linux/mtd/fsmc_nand.h>

@@ -57,3 +58,12 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 	return -1;

(quoted diff) +0 -0

b/board/spear/spear310/spear310.c index 8b58218..8c5b5ba 100644

board/spear/spear310/spear310.c +1 -0

@@ -23,6 +23,7 @@
  */
 
 #include <common.h>
+#include <netdev.h>
 #include <nand.h>
 #include <asm/io.h>
 #include <linux/mtd/fsmc_nand.h>

@@ -58,3 +59,12 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 	return -1;

(quoted diff) +0 -0

b/board/spear/spear320/spear320.c index 172ad35..b60acc2 100644

board/spear/spear320/spear320.c +1 -0

@@ -23,6 +23,7 @@
  */
 
 #include <common.h>
+#include <netdev.h>
 #include <nand.h>
 #include <asm/io.h>
 #include <linux/mtd/fsmc_nand.h>

@@ -58,3 +59,12 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 	return -1;

(quoted diff) +0 -0

b/board/spear/spear600/spear600.c index 7cf63d6..5a32b7f 100644

board/spear/spear600/spear600.c +1 -0

@@ -22,6 +22,7 @@
  */
 
 #include <common.h>
+#include <netdev.h>
 #include <nand.h>
 #include <asm/io.h>
 #include <linux/mtd/fsmc_nand.h>

@@ -52,3 +53,12 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 	return -1;

(quoted diff) +0 -0

b/include/configs/spear-common.h index 8f4973a..3f52442 100644

include/configs/spear-common.h +5 -2

+
 /* USBD driver configuration */
 #define CONFIG_DW_UDC
 #define CONFIG_USB_DEVICE

@@ -103,11 +111,13 @@
 #define CONFIG_CMD_MEMORY
 #define CONFIG_CMD_RUN
 #define CONFIG_CMD_SAVES
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_MII
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
 
 /* This must be included AFTER the definition of CONFIG_COMMANDS (if any)
 */ #include <config_cmd_default.h>
-#undef CONFIG_CMD_NET
-#undef CONFIG_CMD_NFS
 
 /*
  * Default Environment Varible definitions

include/configs/spear3xx.h +0 -0

index 2a86c21..035b321 100644

Quoted 1 line · On Wednesday 07 March 2012 13:03:53 Amit Virdi wrote:

From: Vipin KUMAR<[email protected]>

Please find some comments below.

arch/arm/include/asm/arch-spear/hardware.h line 34

Quoted 11 lines

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-spear/hardware.h index a6517b2..2b9cb0e 100644

arch/arm/include/asm/arch-spear/hardware.h +1 -0

@@ -31,6 +31,7 @@
  #define CONFIG_SPEAR_SYSCNTLBASE		(0xFCA00000)
  #define CONFIG_SPEAR_TIMERBASE			(0xFC800000)
  #define CONFIG_SPEAR_MISCBASE			(0xFCA80000)
+#define CONFIG_SPEAR_ETHBASE			(0xE0800000)

I would prefer if you removed these unneeded parentheses here:

#define CONFIG_SPEAR_ETHBASE 0xE0800000

Perhaps best done by doing a cosmetic cleanup patch before the newly added defines. I know that checkpatch doesn't complain about this, but these parentheses really distract me. Not sure how other feel about it.

arch/arm/include/asm/arch-spear/hardware.h

Quoted 9 lines

arch/arm/include/asm/arch-spear/hardware.h +9 -0

+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_DESIGNWARE_ETH)
+	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
+#else
+	return -1;
+#endif
+}

This routine is added multiple times in this patch. Perhaps this can be moved to a "common/" file instead?

arch/arm/include/asm/arch-spear/hardware.h

Quoted 9 lines

arch/arm/include/asm/arch-spear/hardware.h +9 -0

+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_DESIGNWARE_ETH)
+	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
+#else
+	return -1;
+#endif
+}

Here again.

arch/arm/include/asm/arch-spear/hardware.h

Quoted 9 lines

arch/arm/include/asm/arch-spear/hardware.h +9 -0

+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_DESIGNWARE_ETH)
+	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
+#else
+	return -1;
+#endif
+}

And again.

Quoted 19 lines

(quoted diff) +0 -0

b/board/spear/spear600/spear600.c index 7cf63d6..5a32b7f 100644

board/spear/spear600/spear600.c +4 -0

@@ -22,6 +22,7 @@
   */
 
  #include<common.h>
+#include<netdev.h>
  #include<nand.h>
  #include<asm/io.h>
  #include<linux/mtd/fsmc_nand.h>

@@ -52,3 +53,12 @@ int board_nand_init(struct nand_chip *nand)
  #endif
  	return -1;
  }
+
+int board_eth_init(bd_t *bis)
+{

board/spear/spear600/spear600.c int board_nand_init(struct nand_chip *nand) · line 64

Quoted 6 lines

board/spear/spear600/spear600.c +6 -0

+#if defined(CONFIG_DESIGNWARE_ETH)
+	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
+#else
+	return -1;
+#endif
+}

and again.

board/spear/spear600/spear600.c

Quoted 8 lines

board/spear/spear600/spear600.c +7 -0

+/* Ethernet driver configuration */
+#define CONFIG_MII
+#define CONFIG_DESIGNWARE_ETH
+#define CONFIG_DW_SEARCH_PHY
+#define CONFIG_DW0_PHY				1
+#define CONFIG_NET_MULTI
+#define CONFIG_PHY_RESET_DELAY			(10000)		/*

in usec */

Again, please remove these parentheses.

Quoted 10 lines

board/spear/spear300/spear300.c int board_nand_init(struct nand_chip *nand) · line 69

Quoted 8 lines

board/spear/spear300/spear300.c +8 -0

+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_DESIGNWARE_ETH)
+	return designware_initialize(0, CONFIG_SPEAR_ETHBASE, CONFIG_DW0_PHY);
+#else
+	return -1;
+#endif
+}

and again.

Yes Stefan, it is planned. There's a lot of cleanup that is needed in the SPEAr platform code. I shall be sending another patchset after this patchset that adds new functionality and does the cleanup.

Can you accept this patch for the time being?

From: Vipin KUMAR <[email protected]>

Earlier, architecture specific init code was mixed with board initialization code in board/spear/... This patch updates architecture support for SPEAr in latest u-boot.

It defines the following two flags for SPEAr3xx and SPEAr6xx SoCs
- CONFIG_DISPLAY_CPUINFO: Used to print the SoC information
- CONFIG_ARCH_CPU_INIT: compiles the architecture specific initialization
code with arch/arm/spear/cpu.c

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 arch/arm/cpu/arm926ejs/spear/Makefile      |    3 +-
 arch/arm/cpu/arm926ejs/spear/cpu.c         |   84
++++++++++++++++++++++++++++ arch/arm/include/asm/arch-spear/spr_misc.h | 
  7 ++
 include/configs/spear-common.h             |    2 +
 4 files changed, 95 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/cpu/arm926ejs/spear/cpu.c

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/spear/Makefile index f32ec4c..46923a4 100644

arch/arm/cpu/arm926ejs/spear/Makefile +2 -1

@@ -25,7 +25,8 @@ include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(SOC).o
 
-COBJS	:= reset.o \
+COBJS	:= cpu.o \
+	   reset.o \
 	   timer.o
 SOBJS	:=
 

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/spear/cpu.c new file mode 100644 index 0000000..4dc7d69

arch/arm/cpu/arm926ejs/spear/cpu.c new file +19 -0

@@ -0,0 +1,84 @@
+/*
+ * (C) Copyright 2010
+ * Vipin Kumar, ST Micoelectronics, vipin.kumar at st.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

arch/arm/cpu/arm926ejs/spear/cpu.c new file +29 -0

+int arch_cpu_init(void)
+{
+	struct misc_regs *const misc_p =
+	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
+	u32 periph1_clken;
+
+	periph1_clken = readl(&misc_p->periph1_clken);
+
+#if defined(CONFIG_SPEAR3XX)
+	periph1_clken |= MISC_GPT2ENB;
+#elif defined(CONFIG_SPEAR600)
+	periph1_clken |= MISC_GPT3ENB;
+#endif
+
+#if defined(CONFIG_PL011_SERIAL)
+	periph1_clken |= MISC_UART0ENB;
+#endif
+#if defined(CONFIG_DESIGNWARE_ETH)
+	periph1_clken |= MISC_ETHENB;
+#endif
+#if defined(CONFIG_DW_UDC)
+	periph1_clken |= MISC_USBDENB;
+#endif
+#if defined(CONFIG_DW_I2C)
+	periph1_clken |= MISC_I2CENB;
+#endif
+#if defined(CONFIG_ST_SMI)
+	periph1_clken |= MISC_SMIENB;
+#endif

arch/arm/cpu/arm926ejs/spear/cpu.c new file +16 -0

+int print_cpuinfo(void)
+{
+#ifdef CONFIG_SPEAR300
+	printf("CPU:   SPEAr300\n");
+#elif defined(CONFIG_SPEAR310)
+	printf("CPU:   SPEAr310\n");
+#elif defined(CONFIG_SPEAR320)
+	printf("CPU:   SPEAr320\n");
+#elif defined(CONFIG_SPEAR600)
+	printf("CPU:   SPEAr600\n");
+#else
+#error CPU not supported in spear platform
+#endif
+	return 0;
+}
+#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-spear/spr_misc.h index 8b96d9b..b10c726 100644

arch/arm/include/asm/arch-spear/spr_misc.h +7 -0

@@ -126,5 +126,12 @@ struct misc_regs {
 
 /* PERIPH1_CLKEN, PERIPH1_RST value */
 #define MISC_USBDENB			0x01000000
+#define MISC_ETHENB			0x00800000
+#define MISC_SMIENB			0x00200000
+#define MISC_GPT3ENB			0x00010000
+#define MISC_GPT2ENB			0x00000800
+#define MISC_FSMCENB			0x00000200
+#define MISC_I2CENB			0x00000080
+#define MISC_UART0ENB			0x00000008
 
 #endif

(quoted diff) +0 -0

b/include/configs/spear-common.h index 3f52442..5540630 100644

Review

Quoted 8 lines

+ */

(quoted diff) +7 -0

+
+#include<common.h>
+#include<asm/io.h>
+#include<asm/arch/hardware.h>
+#include<asm/arch/spr_misc.h>
+
+#ifdef CONFIG_ARCH_CPU_INIT

Why not remove this #ifdef here and make compile it in unconditionally? It is needed for each SPEAr board, right?

Quoted 34 lines

arch/arm/cpu/arm926ejs/spear/cpu.c new file +34 -0

+int arch_cpu_init(void)
+{
+	struct misc_regs *const misc_p =
+	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
+	u32 periph1_clken;
+
+	periph1_clken = readl(&misc_p->periph1_clken);
+
+#if defined(CONFIG_SPEAR3XX)
+	periph1_clken |= MISC_GPT2ENB;
+#elif defined(CONFIG_SPEAR600)
+	periph1_clken |= MISC_GPT3ENB;
+#endif
+
+#if defined(CONFIG_PL011_SERIAL)
+	periph1_clken |= MISC_UART0ENB;
+#endif
+#if defined(CONFIG_DESIGNWARE_ETH)
+	periph1_clken |= MISC_ETHENB;
+#endif
+#if defined(CONFIG_DW_UDC)
+	periph1_clken |= MISC_USBDENB;
+#endif
+#if defined(CONFIG_DW_I2C)
+	periph1_clken |= MISC_I2CENB;
+#endif
+#if defined(CONFIG_ST_SMI)
+	periph1_clken |= MISC_SMIENB;
+#endif
+#if defined(CONFIG_NAND_FSMC)
+	periph1_clken |= MISC_FSMCENB;
+#endif
+
+	writel(periph1_clken,&misc_p->periph1_clken);

arch/arm/cpu/arm926ejs/spear/cpu.c line 68

Quoted 5 lines

arch/arm/cpu/arm926ejs/spear/cpu.c new file +5 -0

+	return 0;
+}
+#endif
+
+#ifdef CONFIG_DISPLAY_CPUINFO

Again, I would remove this #ifdef here. Make it unconditionally for all SPEAr boards. We have enough #ifdef's already. :)

Quoted 40 lines

arch/arm/cpu/arm926ejs/spear/cpu.c new file +16 -0

+int print_cpuinfo(void)
+{
+#ifdef CONFIG_SPEAR300
+	printf("CPU:   SPEAr300\n");
+#elif defined(CONFIG_SPEAR310)
+	printf("CPU:   SPEAr310\n");
+#elif defined(CONFIG_SPEAR320)
+	printf("CPU:   SPEAr320\n");
+#elif defined(CONFIG_SPEAR600)
+	printf("CPU:   SPEAr600\n");
+#else
+#error CPU not supported in spear platform
+#endif
+	return 0;
+}
+#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-spear/spr_misc.h index 8b96d9b..b10c726 100644

arch/arm/include/asm/arch-spear/spr_misc.h +7 -0

@@ -126,5 +126,12 @@ struct misc_regs {
 
  /* PERIPH1_CLKEN, PERIPH1_RST value */
  #define MISC_USBDENB			0x01000000
+#define MISC_ETHENB			0x00800000
+#define MISC_SMIENB			0x00200000
+#define MISC_GPT3ENB			0x00010000
+#define MISC_GPT2ENB			0x00000800
+#define MISC_FSMCENB			0x00000200
+#define MISC_I2CENB			0x00000080
+#define MISC_UART0ENB			0x00000008
 
  #endif

(quoted diff) +0 -0

b/include/configs/spear-common.h index 3f52442..5540630 100644

include/configs/spear-common.h +0 -0

@@ -182,6 +182,8 @@
  #define CONFIG_ENV_SIZE				0x02000
 

include/configs/spear-common.h line 186

Quoted 3 lines

include/configs/spear-common.h +2 -0

  /* Miscellaneous configurable options */
+#define CONFIG_ARCH_CPU_INIT
+#define CONFIG_DISPLAY_CPUINFO

Other than my comment above:

Acked-by Stefan Roese<[email protected]>

From: Vipin KUMAR <[email protected]>

spear3xx and 6xx boards can be compiled in following configurations
1. Environment placed in NAND
2. console on usb device
3. console on usb device with environment placed in NAND

Also, renaming the include/configs/spearxxx.h files to spear3xx_evb.h, spear6xx_evb.h etc to depict evaluation board configuration.

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 board/spear/spear300/config.mk                 |   13 +---------
 board/spear/spear310/config.mk                 |   11 --------
 board/spear/spear320/config.mk                 |   11 --------
 board/spear/spear600/config.mk                 |   13 +---------
 boards.cfg                                     |   20 ++++++++++++---
 doc/README.spear                               |   31
+++++++++++++++++------- include/configs/{spear3xx.h => spear3xx_evb.h} | 
 10 +++++++
 include/configs/{spear6xx.h => spear6xx_evb.h} |   10 +++++++
 8 files changed, 60 insertions(+), 59 deletions(-)
 rename include/configs/{spear3xx.h => spear3xx_evb.h} (96%)
 rename include/configs/{spear6xx.h => spear6xx_evb.h} (89%)

(quoted diff) +0 -0

b/board/spear/spear300/config.mk index 5848ef8..0706430 100644

board/spear/spear300/config.mk +0 -3

@@ -25,15 +25,4 @@
 
 CONFIG_SYS_TEXT_BASE = 0x00700000
 
-ALL-y += $(obj)u-boot.img
-
-# Environment variables in NAND

+++ b/include/configs/spear3xx_evb.h

board/spear/spear300/config.mk +0 -0

@@ -39,6 +39,16 @@
 #define CONFIG_SPEAR320				1
 #endif

board/spear/spear300/config.mk +0 -0

 #include <configs/spear-common.h>
 
 /* Ethernet driver configuration */

include/configs/spear6xx_evb.h renamed from include/configs/spear6xx.h +0 -0

similarity index 89%
rename from include/configs/spear6xx.h
rename to include/configs/spear6xx_evb.h
index c5bcc30..a61d9be 100644
@@ -30,6 +30,16 @@
  */
 #define CONFIG_SPEAR600				1
 

Quoted 8 lines

(quoted diff) +0 -0

b/board/spear/spear300/config.mk index 5848ef8..0706430 100644

board/spear/spear300/config.mk +0 -0

@@ -25,15 +25,4 @@
 
  CONFIG_SYS_TEXT_BASE = 0x00700000
 

board/spear/spear300/config.mk line 28

Quoted 13 lines

board/spear/spear300/config.mk +1 -12

-ALL-y += $(obj)u-boot.img
-
-# Environment variables in NAND
-ifeq ($(ENV),NAND)
-PLATFORM_RELFLAGS += -DCONFIG_ENV_IS_IN_NAND
-else
-PLATFORM_RELFLAGS += -DCONFIG_ENV_IS_IN_FLASH
-endif
-
-ifeq ($(CONSOLE),USB)
-PLATFORM_RELFLAGS += -DCONFIG_SPEAR_USBTTY
-endif
+ALL += $(obj)u-boot.img

Can't you just remove this config.mk file completely? CONFIG_SYS_TEXT_BASE can be moved to the config header instead. What else is this file needed for?

The same comment for all other config.mk files.

Quoted 14 lines

Quoted 14 lines

+++ b/board/spear/spear300/config.mk

board/spear/spear300/config.mk +0 -9

@@ -25,15 +25,4 @@
 
  CONFIG_SYS_TEXT_BASE = 0x00700000
 
-ALL-y += $(obj)u-boot.img
-
-# Environment variables in NAND
-ifeq ($(ENV),NAND)
-PLATFORM_RELFLAGS += -DCONFIG_ENV_IS_IN_NAND
-else
-PLATFORM_RELFLAGS += -DCONFIG_ENV_IS_IN_FLASH
-endif
-

Quoted 8 lines

board/spear/spear300/config.mk line 28

Quoted 4 lines

board/spear/spear300/config.mk +1 -3

-ifeq ($(CONSOLE),USB)
-PLATFORM_RELFLAGS += -DCONFIG_SPEAR_USBTTY
-endif
+ALL += $(obj)u-boot.img

Can't you just remove this config.mk file completely? CONFIG_SYS_TEXT_BASE can be moved to the config header instead. What else is this file needed for?

Could you please explain which config header you are referring to here? Do you mean include/configs/spear..... ?

For SPEAr310 and SPEAr320, config.mk is needed to make image (by specifying FLASH=PNOR during compile time) that can access CFI compliant PNOR flash.

Quoted 7 lines

Quoted 4 lines

Can't you just remove this config.mk file completely? CONFIG_SYS_TEXT_BASE can be moved to the config header instead. What else is this file needed for?

Could you please explain which config header you are referring to here? Do you mean include/configs/spear..... ?

Yes.

Quoted 3 lines

For SPEAr310 and SPEAr320, config.mk is needed to make image (by specifying FLASH=PNOR during compile time) that can access CFI compliant PNOR flash.

In the example above (spear300), only CONFIG_SYS_TEXT_BASE is left in config.mk. This can be moved to the config header (include/configs/spear-xxx- h).

You should be able to move the other remaining options (PNOR...) to boards.cfg. Or is this not possible?

From: Vipin KUMAR <[email protected]>

SPEAr doesn't need CONFIG_SYS_HZ_CLOCK. This commit removes it.

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

From: Vipin KUMAR <[email protected]>

This patch modifies the default environment variables as:
1. Default bootargs:
- console=ttyAMA0,115200
- For environment present in NOR flash
root=/dev/mtdblock3
- For environment present in NAND flash
root=/dev/mtdblock7
- Removes "mem=" option
2. Introduces CONFIG_EXTRA_ENV_USBTTY as default usbtty env var even when usbtty is not selected
3. Add default definitions for nfsboot and ramboot
4. Add a new default environment variable(CONFIG_EXTRA_ENV_UNLOCK)

Signifacance of CONFIG_EXTRA_ENV_USBTTY:
This environment variable is important for flashing utility to work. So if somebody accidently erases the env sector then also this variable must be preserved so that flashing utility functions properly.

Signifacance of CONFIG_EXTRA_ENV_UNLOCK:
This env variable is read by the cfi driver to unlock all flash sectors. This is necessary because the Parallel NOR flash connected on the spear boards, M28W64, has all its sectors in locked state at reset and these have to be unlocked explicitly before being erased or written.

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Shiraz Hashim <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

Quoted 5 lines

Signifacance of CONFIG_EXTRA_ENV_UNLOCK:
This env variable is read by the cfi driver to unlock all flash sectors. This is necessary because the Parallel NOR flash connected on the spear boards, M28W64, has all its sectors in locked state at reset and these have to be unlocked explicitly before being erased or written.

But this is not needed for boards not using the common CFI flash driver. As the SPEAr600 based ones. So please only define it for board using the CFI driver.

Quoted 3 lines

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Shiraz Hashim<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

Other than my comment above:

Acked-by Stefan Roese<[email protected]>

flash reading is required earlier than flash_init is called since the env_init is called before flash_init. This makes the smi_init necessary before env_init being called.

Signed-off-by Amit Virdi <[email protected]>

From: Vipin KUMAR <[email protected]>

This patch enables the support for usb high speed device for spear platform SOCs

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear-common.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index 66cf19d..f4980b0 100644

Quoted 11 lines · On Wednesday 07 March 2012 13:04:01 Amit Virdi wrote:

From: Vipin KUMAR<[email protected]>

This patch enables the support for usb high speed device for spear platform SOCs

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

---
  include/configs/spear-common.h |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

include/configs/spear-common.h line 41

Quoted 9 lines

(quoted diff) +0 -0

b/include/configs/spear-common.h index 66cf19d..f4980b0 100644

include/configs/spear-common.h +1 -0

@@ -38,6 +38,7 @@
  /* USBD driver configuration */
  #define CONFIG_DW_UDC
  #define CONFIG_USB_DEVICE
+#define CONFIG_USBD_HS

This define doesn't seem to be mentioned in the toplevel README file. Please add a short description there.

Hello Stefan,

Quoted 13 lines · Stefan Roese

Quoted 12 lines · On Wednesday 07 March 2012 13:04:01 Amit Virdi wrote:

From: Vipin KUMAR<[email protected]>

This patch enables the support for usb high speed device for spear platform
SOCs

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

---
include/configs/spear-common.h | 1 +

1 files changed, 1 insertions(+), 0 deletions(-)

Quoted 14 lines

include/configs/spear-common.h line 41

Quoted 9 lines

(quoted diff) +0 -0

b/include/configs/spear-common.h index 66cf19d..f4980b0 100644

include/configs/spear-common.h +1 -0

@@ -38,6 +38,7 @@
 /* USBD driver configuration */
 #define CONFIG_DW_UDC
 #define CONFIG_USB_DEVICE
+#define CONFIG_USBD_HS

This define doesn't seem to be mentioned in the toplevel README file. Please
add a short description there.

Sure.

From: Vipin KUMAR <[email protected]>

Ethernet probing is removed from u-boot compiled as a firmware for spear flashing utility. This enables faster access to usbtty interface as unused ethernet i/f is not initialized.

Quoted 5 lines · On Wednesday 07 March 2012 13:04:02 Amit Virdi wrote:

From: Vipin KUMAR<[email protected]>

Ethernet probing is removed from u-boot compiled as a firmware for spear flashing utility. This enables faster access to usbtty interface as unused ethernet i/f is not initialized.

Ethernet probing should only be done, when the device is really accessed (e.g. by using tftp commands). I know that the current implementation of the designware ethernet driver does this probing (PHY link negotiation etc) unconditionally at startup.

I have a patch in my queue that changes the designware driver to only start probing/auto-negotiating once its really used. I'll rebase my patch on top of your designware rework patches, once they are approved.

With this patch, this patch here should be unnecessary. At least when reading the patch description above.

So is this patch still needed, after my designware net driver rework?

From: Vipin KUMAR <[email protected]>

This patch enables the UDC and usb-console support only for usbtty configurations

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

From: Shiraz Hashim <[email protected]>

The linux-2.6.37 base port has few problems with nfs boot. The boot fails while getting timeout on attempting to mount root file system. This helps the ethernet to bring up faster in linux boot thus avoiding the time out.
Besides, it was reported that few phys on SPEAr board are failing in certain network conditions which is avoided by enabling autonegotiation. See issue #115943 in global bug tracker.

Reported-by Deepak Sikri <[email protected]>

Reported-by Armando Visconti <[email protected]>

Signed-off-by Shiraz Hashim <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear-common.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index 5b6ef72..26642f1 100644

include/configs/spear-common.h +0 -0

 #endif
 
 /* USBD driver configuration */

Quoted 9 lines · On Wednesday 07 March 2012 13:04:04 Amit Virdi wrote:

From: Shiraz Hashim<[email protected]>

The linux-2.6.37 base port has few problems with nfs boot. The boot fails while getting timeout on attempting to mount root file system. This helps the ethernet to bring up faster in linux boot thus avoiding the time out.
Besides, it was reported that few phys on SPEAr board are failing in certain network conditions which is avoided by enabling autonegotiation. See issue #115943 in global bug tracker.

Hmmm. This sounds like a problem in the Linux network/PHY code then. Which needs to get fixed there instead of changing something here in U-Boot. Please

think about situations where the ethernet driver is not called at all in U- Boot (booting from NOR/NAND). With my changes to defer the ethernet probing to the first usage.

One more comment below.

Quoted 13 lines

Reported-by Deepak Sikri<[email protected]>

Reported-by Armando Visconti<[email protected]>

Signed-off-by Shiraz Hashim<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

---
  include/configs/spear-common.h |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index 5b6ef72..26642f1 100644

include/configs/spear-common.h line 37

Quoted 3 lines

include/configs/spear-common.h +0 -0

@@ -35,6 +35,7 @@
  #define CONFIG_DW0_PHY				1
  #define CONFIG_NET_MULTI
  #define CONFIG_PHY_RESET_DELAY			(10000)		/*

in usec */

include/configs/spear-common.h line 38

Quoted 1 line

include/configs/spear-common.h +1 -0

+#define CONFIG_DW_AUTONEG			1

Please don't add the "1" here.

Quoted 11 lines

Quoted 7 lines

The linux-2.6.37 base port has few problems with nfs boot. The boot fails while getting timeout on attempting to mount root file system. This helps the ethernet to bring up faster in linux boot thus avoiding the time out.
Besides, it was reported that few phys on SPEAr board are failing in certain network conditions which is avoided by enabling autonegotiation. See issue #115943 in global bug tracker.

Hmmm. This sounds like a problem in the Linux network/PHY code then. Which needs to get fixed there instead of changing something here in U-Boot. Please

Probably, this is the problem with phys used in some boards.

Quoted 3 lines

think about situations where the ethernet driver is not called at all in U- Boot (booting from NOR/NAND). With my changes to defer the ethernet probing to the first usage.

Keeping your changes, we can still move ahead with autoneg ON. I can think of two solutions for this -

Solution 1:
- Use fix configuration for all SPEAr boards except the one on which
the problem has been reported. In that case we can ON autoneg.

Solution 2:
- Enable autoneg for all SPEAr and specify a larger delay parameter if
rootfs has to be mounted from nfs. This would ensure that timeout won't happen.

I would prefer solution 2. What's your opinion?

Quoted 5 lines

Solution 2:
- Enable autoneg for all SPEAr and specify a larger delay parameter if
rootfs has to be mounted from nfs. This would ensure that timeout won't happen.

Quoted 1 line

I would prefer solution 2. What's your opinion?

I have no problem with enabling autoneg on all boards. My only concern was the error description in the commit log. This seemed to indicate some error in the Linux ethernet driver. As it relies on the ethernet setup done by the bootloader.

But again, I have generally no objection to enable autoneg for the SPEAr boards.

From: Vipin Kumar <[email protected]>

This patch enables flash protection(lock/unlock) for CFI devices. This is necessary because the Parallel NOR flash connected on the spear boards, M28W64, can be locked/unlocked on a sector basis. Moreover, all its sectors are in locked state at reset and these have to be unlocked explicitly before being erased or written.

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear3xx_evb.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear3xx_evb.h index 84fad0c..e827a33 100644

include/configs/spear3xx_evb.h +1 -0

@@ -113,6 +113,7 @@
 #define CONFIG_FLASH_CFI_DRIVER
 
 #if defined(CONFIG_SPEAR310)
+#define CONFIG_SYS_FLASH_PROTECTION		1
 #define CONFIG_SYS_FLASH_BASE			0x50000000
 #define CONFIG_SYS_CS1_FLASH_BASE		0x60000000
 #define CONFIG_SYS_CS2_FLASH_BASE		0x70000000

Quoted 29 lines · On Wednesday 07 March 2012 13:04:06 Amit Virdi wrote:

From: Vipin Kumar<[email protected]>

This patch enables flash protection(lock/unlock) for CFI devices. This is necessary because the Parallel NOR flash connected on the spear boards, M28W64, can be locked/unlocked on a sector basis. Moreover, all its sectors are in locked state at reset and these have to be unlocked explicitly before being erased or written.

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

---
  include/configs/spear3xx_evb.h |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear3xx_evb.h index 84fad0c..e827a33 100644

include/configs/spear3xx_evb.h +1 -0

@@ -113,6 +113,7 @@
  #define CONFIG_FLASH_CFI_DRIVER
 
  #if defined(CONFIG_SPEAR310)
+#define CONFIG_SYS_FLASH_PROTECTION		1
  #define CONFIG_SYS_FLASH_BASE			0x50000000
  #define CONFIG_SYS_CS1_FLASH_BASE		0x60000000
  #define CONFIG_SYS_CS2_FLASH_BASE		0x70000000

@@ -128,6 +129,7 @@
  #define CONFIG_SYS_MAX_FLASH_BANKS		6
 

include/configs/spear3xx_evb.h line 132

Quoted 2 lines

include/configs/spear3xx_evb.h +1 -0

  #elif defined(CONFIG_SPEAR320)
+#define CONFIG_SYS_FLASH_PROTECTION		1

Remove the "1"'s above.

Other that that:

Acked-by Stefan Roese<[email protected]>

From: Vipin Kumar <[email protected]>

The below text is copy pasted from README
- CONFIG_SYS_MONITOR_BASE:
Physical start address of boot monitor code (set by make config files to be same as the text base address (TEXT_BASE) used when linking) - same as CONFIG_SYS_FLASH_BASE when booting from flash.

This patch corrects the definition of CONFIG_SYS_MONITOR_BASE and sets it to TEXT_BASE

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear-common.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index 26642f1..0998157 100644

include/configs/spear-common.h line 209

Quoted 5 lines

  #define CONFIG_ENV_SIZE				0x02000

include/configs/spear-common.h +2 -0

+#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
+#define CONFIG_MONITOR_IS_IN_RAM		1

Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please without the 1.

Quoted 7 lines

include/configs/spear-common.h line 209

Quoted 4 lines

  #define CONFIG_ENV_SIZE				0x02000

include/configs/spear-common.h +2 -0

+#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
+#define CONFIG_MONITOR_IS_IN_RAM		1

Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please without the 1.

CONFIG_MONITOR_IS_IN_RAM is defined with the understanding that since, u-boot is loaded by the BootROM (embedded in eROM) to RAM, so the monitor is present in RAM. Please confirm if this understanding is correct.

Amit,

Quoted 8 lines · On Tuesday 27 March 2012 08:38:18 Amit Virdi wrote:

Quoted 7 lines

include/configs/spear-common.h line 209

Quoted 4 lines

   #define CONFIG_ENV_SIZE				0x02000

include/configs/spear-common.h +2 -0

+#define CONFIG_SYS_MONITOR_BASE			CONFIG_SYS_TEXT_BASE
+#define CONFIG_MONITOR_IS_IN_RAM		1

Why is CONFIG_MONITOR_IS_IN_RAM defined? And if really needed, please without the 1.

Quoted 3 lines

CONFIG_MONITOR_IS_IN_RAM is defined with the understanding that since, u-boot is loaded by the BootROM (embedded in eROM) to RAM, so the monitor is present in RAM. Please confirm if this understanding is correct.

Hmmm, seems to be used on some Coldfire and Blackfin platforms. Its not needed for your platform. Please remove it.

From: Vipin Kumar <[email protected]>

Enable CONFIG_SYS_FLASH_EMPTY_INFO macro to enable reporting of empty sector information through flinfo command.

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear-common.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index 0998157..bde99d2 100644

CONFIG_SYS_CS1_FLASH_BASE}

Quoted 10 lines

  #define CONFIG_SYS_MAX_FLASH_SECT		128

include/configs/spear-common.h +0 -1

-#define CONFIG_SYS_FLASH_EMPTY_INFO		1
  #define CONFIG_SYS_FLASH_ERASE_TOUT		(3 * CONFIG_SYS_HZ)
  #define CONFIG_SYS_FLASH_WRITE_TOUT		(3 * CONFIG_SYS_HZ)

@@ -238,6 +237,7 @@
  #define CONFIG_SYS_CONSOLE_INFO_QUIET		1
  #define CONFIG_SYS_64BIT_VSPRINTF		1
 

include/configs/spear-common.h line 240

Quoted 1 line

include/configs/spear-common.h +1 -0

+#define CONFIG_SYS_FLASH_EMPTY_INFO		1

No 1 please.

Other than that:

Acked-by Stefan Roese<[email protected]>

From: Vipin Kumar <[email protected]>

Signed-off-by Vipin Kumar <[email protected]>

Signed-off-by Amit Virdi <[email protected]>

---
 include/configs/spear-common.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

(quoted diff) +0 -0

b/include/configs/spear-common.h index bde99d2..4fb56ba 100644

Quoted 8 lines · On Wednesday 07 March 2012 13:04:09 Amit Virdi wrote:

From: Vipin Kumar<[email protected]>

Signed-off-by Vipin Kumar<[email protected]>

Signed-off-by Amit Virdi<[email protected]>

---
  include/configs/spear-common.h |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

include/configs/spear-common.h line 108

Quoted 10 lines

(quoted diff) +0 -0

b/include/configs/spear-common.h index bde99d2..4fb56ba 100644

include/configs/spear-common.h +2 -0

@@ -104,6 +104,8 @@
  #define CONFIG_NAND_FSMC
  #define CONFIG_SYS_MAX_NAND_DEVICE		1
  #define CONFIG_MTD_NAND_VERIFY_WRITE		1
+#define CONFIG_SYS_NAND_ONFI_DETECTION		1
+#define CONFIG_SYS_NAND_QUIET_TEST		1

Remove the "1"'s please. And please also remove CONFIG_MTD_NAND_VERIFY_WRITE. Or is it really needed? I didn't need it on my SPEAr600 boards. And it slows NAND access unnecessarily down.