lpc32xx: devkit3250 board update

Some NAND controllers define custom functions to read data out, respect this in order to correctly support bad block handling in simple SPL NAND framework.

NAND controller specific read_buf() is used even to read 1 byte in case of connected 8-bit NAND device, it turns out that read_byte() may become outdated.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +5 -2

index 700ca32..e69f662 100644
@@ -115,6 +115,7 @@ static int nand_command(int block, int page, uint32_t offs,
 static int nand_is_bad_block(int block)
 {
 	struct nand_chip *this = mtd.priv;
+	u_char bb_data[2];
 
 	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
 		NAND_CMD_READOOB);

@@ -123,10 +124,12 @@ static int nand_is_bad_block(int block)
 	 * Read one byte (or two if it's a 16 bit chip).
 	 */
 	if (this->options & NAND_BUSWIDTH_16) {
-		if (readw(this->IO_ADDR_R) != 0xffff)
+		this->read_buf(&mtd, bb_data, 2);
+		if (bb_data[0] != 0xff || bb_data[1] != 0xff)
 			return 1;
 	} else {
-		if (readb(this->IO_ADDR_R) != 0xff)
+		this->read_buf(&mtd, bb_data, 1);
+		if (bb_data[0] != 0xff)
 			return 1;
 	}
 

Signature · 1 line

2.1.4

Hello Vladimir,

Quoted 40 lines · Vladimir Zapolskiy

Some NAND controllers define custom functions to read data out, respect this in order to correctly support bad block handling in simple SPL NAND framework.

NAND controller specific read_buf() is used even to read 1 byte in case of connected 8-bit NAND device, it turns out that read_byte() may become outdated.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +5 -2

index 700ca32..e69f662 100644
@@ -115,6 +115,7 @@ static int nand_command(int block, int page, uint32_t offs,
 static int nand_is_bad_block(int block)
 {
 	struct nand_chip *this = mtd.priv;
+	u_char bb_data[2];
 
 	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
 		NAND_CMD_READOOB);

@@ -123,10 +124,12 @@ static int nand_is_bad_block(int block)
 	 * Read one byte (or two if it's a 16 bit chip).
 	 */
 	if (this->options & NAND_BUSWIDTH_16) {
-		if (readw(this->IO_ADDR_R) != 0xffff)
+		this->read_buf(&mtd, bb_data, 2);
+		if (bb_data[0] != 0xff || bb_data[1] != 0xff)
 			return 1;
 	} else {
-		if (readb(this->IO_ADDR_R) != 0xff)
+		this->read_buf(&mtd, bb_data, 1);
+		if (bb_data[0] != 0xff)
 			return 1;
 	}
 

Quoted 3 lines

Signature · 1 line

2.1.4

The way you describe this patch, it looks like a bug that should have bitten way more boards than lpc32xx. Did you have a look at some other boards to see why this did not affect them?

Conversively, what is the actual failure scenario that led you to writing this patch?

Hello Albert,

Quoted 44 lines · Albert ARIBAUD

Hello Vladimir,

Quoted 40 lines · Vladimir Zapolskiy

Some NAND controllers define custom functions to read data out, respect this in order to correctly support bad block handling in simple SPL NAND framework.

NAND controller specific read_buf() is used even to read 1 byte in case of connected 8-bit NAND device, it turns out that read_byte() may become outdated.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +5 -2

index 700ca32..e69f662 100644
@@ -115,6 +115,7 @@ static int nand_command(int block, int page, uint32_t offs,
 static int nand_is_bad_block(int block)
 {
 	struct nand_chip *this = mtd.priv;
+	u_char bb_data[2];
 
 	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
 		NAND_CMD_READOOB);

@@ -123,10 +124,12 @@ static int nand_is_bad_block(int block)
 	 * Read one byte (or two if it's a 16 bit chip).
 	 */
 	if (this->options & NAND_BUSWIDTH_16) {
-		if (readw(this->IO_ADDR_R) != 0xffff)
+		this->read_buf(&mtd, bb_data, 2);
+		if (bb_data[0] != 0xff || bb_data[1] != 0xff)
 			return 1;
 	} else {
-		if (readb(this->IO_ADDR_R) != 0xff)
+		this->read_buf(&mtd, bb_data, 1);
+		if (bb_data[0] != 0xff)
 			return 1;
 	}
 

Quoted 7 lines

Quoted 3 lines

Signature · 1 line

2.1.4

The way you describe this patch, it looks like a bug that should have bitten way more boards than lpc32xx. Did you have a look at some other boards to see why this did not affect them?

Yes, it is a bug IMHO.

Grepping for CONFIG_SPL_NAND_SIMPLE I see that TI and Tegra boards may be impacted (positively or negatively):

* CONFIG_NAND_OMAP_GPMC --- own .read_buf(), default .read_byte()
* CONFIG_NAND_DAVINCI --- own .read_buf(), default .read_byte()
* CONFIG_TEGRA_NAND --- own .read_byte(), own .read_buf()

Quoted 2 lines

Conversively, what is the actual failure scenario that led you to writing this patch?

The failure scenario is quite simple, the ARM core gets stuck on first attempt to access SLC NAND data register -- traced with JTAG.

The same happens, if I remove custom .read_byte()/.read_buf() from SLC NAND driver. The only difference between custom .read_byte() and shared read_byte() is in readb()/readl() access to the data register, it is essential to have only 32-bit wide access to SLC NAND registers.

--
With best wishes,
Vladimir

Hello Vladimir,

Quoted 55 lines · Vladimir Zapolskiy

Hello Albert,

Quoted 44 lines · Albert ARIBAUD

Hello Vladimir,

Quoted 40 lines · Vladimir Zapolskiy

Some NAND controllers define custom functions to read data out, respect this in order to correctly support bad block handling in simple SPL NAND framework.

NAND controller specific read_buf() is used even to read 1 byte in case of connected 8-bit NAND device, it turns out that read_byte() may become outdated.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 drivers/mtd/nand/nand_spl_simple.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/mtd/nand/nand_spl_simple.c +5 -2

index 700ca32..e69f662 100644
@@ -115,6 +115,7 @@ static int nand_command(int block, int page, uint32_t offs,
 static int nand_is_bad_block(int block)
 {
 	struct nand_chip *this = mtd.priv;
+	u_char bb_data[2];
 
 	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
 		NAND_CMD_READOOB);

@@ -123,10 +124,12 @@ static int nand_is_bad_block(int block)
 	 * Read one byte (or two if it's a 16 bit chip).
 	 */
 	if (this->options & NAND_BUSWIDTH_16) {
-		if (readw(this->IO_ADDR_R) != 0xffff)
+		this->read_buf(&mtd, bb_data, 2);
+		if (bb_data[0] != 0xff || bb_data[1] != 0xff)
 			return 1;
 	} else {
-		if (readb(this->IO_ADDR_R) != 0xff)
+		this->read_buf(&mtd, bb_data, 1);
+		if (bb_data[0] != 0xff)
 			return 1;
 	}
 

Quoted 7 lines

Quoted 3 lines

Signature · 1 line

2.1.4

The way you describe this patch, it looks like a bug that should have bitten way more boards than lpc32xx. Did you have a look at some other boards to see why this did not affect them?

Quoted 1 line

Yes, it is a bug IMHO.

If it is, then it has hit no board which defines CONFIG_SPL_NAND_SIMPLE and we should understand why.

Quoted 3 lines

Grepping for CONFIG_SPL_NAND_SIMPLE I see that TI and Tegra boards may be impacted (positively or negatively):

Quoted 3 lines

* CONFIG_NAND_OMAP_GPMC --- own .read_buf(), default .read_byte()
* CONFIG_NAND_DAVINCI --- own .read_buf(), default .read_byte()
* CONFIG_TEGRA_NAND --- own .read_byte(), own .read_buf()

They may be impacted by your change, but they are working now -- they are not obscure models. Let's dig a bit...

Quoted 10 lines

Quoted 2 lines

Conversively, what is the actual failure scenario that led you to writing this patch?

The failure scenario is quite simple, the ARM core gets stuck on first attempt to access SLC NAND data register -- traced with JTAG.

The same happens, if I remove custom .read_byte()/.read_buf() from SLC NAND driver. The only difference between custom .read_byte() and shared read_byte() is in readb()/readl() access to the data register, it is essential to have only 32-bit wide access to SLC NAND registers.

README describes CONFIG_SPL_NAND_SIMPLE as "Support for NAND boot using simple NAND drivers that expose the cmd_ctrl() interface". The cmd_ctrl interface actually contains the cmd_ctrl() function *and* the IO_ADDR_[RW] fields. IOW, a simple NAND driver provides byte or word access to the NAND's I/O lines on which command, address and data are passed. If the NAND is 8 bits, then there are 8 lines; if it is 16 bit, then there are 16 lines.

I reckon that the OMAP/DAVINCI and TEGRA simple drivers just do that:
set IO_ADDR_[RW] to the IP register through which direct access to the NAND's I/O lines can be performed, byte or word depending on the chip width.

As such, there is no bug in the way nand_simple.c uses IO_ADDR_[RW].

OTOH, the SLC IP does not provide direct access to the NAND I/O lines through a general register; what it provides is a set of three specialized registers one for commands, one for addresses and one for data. Plus, even though only 8 bit NANDs are supported, the data register does not physically support 8-bit wide accesses (NXP: I am still struggling to understand what you were trying to achieve there).

All this basically makes the SLC controller a 'not simple NAND IP', and its driver a 'not simple NAND driver' incompatible with nand_simple.c.

Your solution to this incompatibility is to change nand_simple.c to support other types of drivers; but by doing that, you're changing the API between nand_simple.c and all simple drivers, possibly changing the established behavior.

I personally don't think this is the right way; nand_simple.c should be left unchanged and the board should simply not use it since it does not have a simple NAND controller, and instead it should provide its own nand_spl_load_image().

But hey, I'm not then NAND custodian. Scott: your call. :)

Quoted 4 lines

I personally don't think this is the right way; nand_simple.c should be left unchanged and the board should simply not use it since it does not have a simple NAND controller, and instead it should provide its own nand_spl_load_image().

For me an alternative change to the proposed one is to duplicate nand_spl_simple.c functionality in LPC32xx SLC NAND driver. From maintenance point of view this is not the best thing to do IMHO.

Quoted 1 line

But hey, I'm not then NAND custodian. Scott: your call. :)

Anyway thank you for review and comments.

--
With best wishes,
Vladimir

Hello Vladimir,

On Thu, 16 Jul 2015 16:48:26 +0300, Vladimir Zapolskiy <[email protected]> wrote:

(cutting short to the essential point remaining)

Quoted 4 lines

Quoted 4 lines

I personally don't think this is the right way; nand_simple.c should be left unchanged and the board should simply not use it since it does not have a simple NAND controller, and instead it should provide its own nand_spl_load_image().

Quoted 3 lines

For me an alternative change to the proposed one is to duplicate nand_spl_simple.c functionality in LPC32xx SLC NAND driver. From maintenance point of view this is not the best thing to do IMHO.

You're right that some of the functionality present in nand_simple.c is duplicated elsewhere; however, that functionality is not the one specific to nand_simple; actually, it is nand_spl_load_image(), the overall load functionality which should be in no driver at all but of which I see ten incarnations in ten drivers.

There should be only one nand_spl_load_image(), in its own file, with placeholders for driver-specific actions such as page read, bad page check, etc. One day, maybe, there will be a patch for that.

Hello Vladimir,

drivers/mtd/nand/lpc32xx_nand_slc.c line 142

Quoted 8 lines · Vladimir Zapolskiy

drivers/mtd/nand/lpc32xx_nand_slc.c new file +4 -0

new file mode 100644
 
+int board_nand_init(struct nand_chip *lpc32xx_chip)
+{
+   lpc32xx_chip->IO_ADDR_R = &lpc32xx_nand_slc_registers->data;
+   lpc32xx_chip->IO_ADDR_W = &lpc32xx_nand_slc_registers->data;

Consistent with my comment re nand_simpl.c, I think that the two assignments above are incorrect since the data register may not provide general access to the NAND's I/O lines, and is not 8-bit accessible even though the NAND is 8-bit wide.

If Scott give his go, though, disregard my comment.

Quoted 19 lines · Albert ARIBAUD

Hello Vladimir,

drivers/mtd/nand/lpc32xx_nand_slc.c line 142

Quoted 8 lines · Vladimir Zapolskiy

drivers/mtd/nand/lpc32xx_nand_slc.c new file +4 -0

new file mode 100644
 
+int board_nand_init(struct nand_chip *lpc32xx_chip)
+{
+   lpc32xx_chip->IO_ADDR_R = &lpc32xx_nand_slc_registers->data;
+   lpc32xx_chip->IO_ADDR_W = &lpc32xx_nand_slc_registers->data;

Consistent with my comment re nand_simpl.c, I think that the two assignments above are incorrect since the data register may not provide general access to the NAND's I/O lines, and is not 8-bit accessible even though the NAND is 8-bit wide.

Quoted 1 line

If Scott give his go, though, disregard my comment.

I agree with Albert -- if you need custom accessors, there's no benefit to using IO_ADDR_R/W, and it could make it harder to debug if some user of IO_ADDR_R/W is missed.

drivers/mtd/nand/lpc32xx_nand_slc.c new file +12 -0

+static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+     while (len-- > 0)
+             *buf++ = (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
+}
+
+static uint8_t lpc32xx_read_byte(struct mtd_info *mtd)
+{
+     return (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
+}
+
+static void lpc32xx_write_buf(struct mtd_info *mtd, const uint8_t *buf, 

int len)

Quoted 16 lines · Vladimir Zapolskiy

drivers/mtd/nand/lpc32xx_nand_slc.c new file +12 -0

+static void lpc32xx_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+     while (len-- > 0)
+             *buf++ = (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
+}
+
+static uint8_t lpc32xx_read_byte(struct mtd_info *mtd)
+{
+     return (uint8_t)readl(&lpc32xx_nand_slc_registers->data);
+}
+
+static void lpc32xx_write_buf(struct mtd_info *mtd, const uint8_t *buf, 

int len)

drivers/mtd/nand/lpc32xx_nand_slc.c new file +3 -0

+{
+     while (len-- > 0)
+             writel((uint32_t)*buf++, &lpc32xx_nand_slc_registers->data);

drivers/mtd/nand/lpc32xx_nand_slc.c line 132

Quoted 6 lines

drivers/mtd/nand/lpc32xx_nand_slc.c new file +6 -0

+}
+
+static void lpc32xx_write_byte(struct mtd_info *mtd, uint8_t byte)
+{
+     writel((uint32_t)byte, &lpc32xx_nand_slc_registers->data);
+}

Unnecessary casts.

drivers/mtd/nand/lpc32xx_nand_slc.c line 151

Quoted 5 lines

drivers/mtd/nand/lpc32xx_nand_slc.c new file +5 -0

+     /*
+      * Hardware ECC calculation is not supported by the driver, because it
+      * requires DMA support, see Note after SLC_ECC register description
+      */
+     lpc32xx_chip->ecc.mode  = NAND_ECC_SOFT;

Where can I find this note? I don't see any "SLC_ECC register description".

drivers/mtd/nand/lpc32xx_nand_slc.c line 173

Quoted 3 lines

drivers/mtd/nand/lpc32xx_nand_slc.c new file +3 -0

+#if defined(CONFIG_SPL_BUILD)
+     lpc32xx_chip->options           |= NAND_SKIP_BBTSCAN;
+#endif

Does this make any difference? nand_spl_simple will not do a bbt scan in any case.

This change adds more peripherals to Timll DevKit3250 board, namely MAC and SMSC phy, SLC NAND, GPIO, SPI and I2C.

Also the default serial console is changed to UART5, added an option to pass device tree blob by means of bootm, predefined environment variables are slightly extended and reserved space on NAND to store user defined U-boot environment.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 board/timll/devkit3250/devkit3250.c | 31 ++++++++++++-
 configs/devkit3250_defconfig        |  3 ++
 include/configs/devkit3250.h        | 92 +++++++++++++++++++++++++++++++++++--
 3 files changed, 120 insertions(+), 6 deletions(-)

board/timll/devkit3250/devkit3250.c +30 -1

index 6acc416..4b3c94e 100644
@@ -1,23 +1,52 @@
 /*
  * Embest/Timll DevKit3250 board support
  *
- * Copyright (C) 2011 Vladimir Zapolskiy <[email protected]>
+ * Copyright (C) 2011-2015 Vladimir Zapolskiy <[email protected]>
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>
 #include <asm/arch/sys_proto.h>
+#include <asm/arch/clk.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/emc.h>
+#include <asm/arch/wdt.h>
+#include <asm/io.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 static struct emc_regs *emc = (struct emc_regs *)EMC_BASE;
+static struct clk_pm_regs *clk = (struct clk_pm_regs *)CLK_PM_BASE;
+static struct wdt_regs *wdt = (struct wdt_regs *)WDT_BASE;
+
+void reset_periph(void)
+{
+	/* This function resets peripherals by triggering RESOUT_N */
+	setbits_le32(&clk->timclk_ctrl, CLK_TIMCLK_WATCHDOG);
+	writel(WDTIM_MCTRL_RESFRC1, &wdt->mctrl);
+	udelay(300);
+
+	writel(0, &wdt->mctrl);
+	clrbits_le32(&clk->timclk_ctrl, CLK_TIMCLK_WATCHDOG);
+
+	/* Such a long delay is needed to initialize SMSC phy */
+	udelay(10000);
+}
 
 int board_early_init_f(void)
 {
 	lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+	lpc32xx_i2c_init(1);
+	lpc32xx_i2c_init(2);
+	lpc32xx_ssp_init();
+	lpc32xx_mac_init();
+
+	/*
+	 * nWP may be controlled by GPO19, but unpopulated by default R23
+	 * makes no sense to configure this GPIO level, nWP is always high
+	 */
+	lpc32xx_slc_nand_init();
 
 	return 0;
 }

configs/devkit3250_defconfig +3 -0

index f0c4ee1..56d719f 100644
@@ -1,3 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_DEVKIT3250=y
+# CONFIG_CMD_FPGA is not set
 # CONFIG_CMD_SETEXPR is not set
+CONFIG_DM=y
+CONFIG_DM_GPIO=y

include/configs/devkit3250.h +1 -1

index 4f35234..b8218b5 100644
@@ -1,7 +1,7 @@
 /*
  * Embest/Timll DevKit3250 board configuration file
  *
- * Copyright (C) 2011 Vladimir Zapolskiy <[email protected]>
+ * Copyright (C) 2011-2015 Vladimir Zapolskiy <[email protected]>
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */

include/configs/devkit3250.h +58 -1

 #define CONFIG_BAUDRATE			115200
 
 /*
+ * I2C
+ */
+#define CONFIG_SYS_I2C
+#define CONFIG_SYS_I2C_LPC32XX
+#define CONFIG_SYS_I2C_SPEED		100000
+#define CONFIG_CMD_I2C
+
+/*
+ * GPIO
+ */
+#define CONFIG_LPC32XX_GPIO
+#define CONFIG_CMD_GPIO
+
+/*
+ * SSP/SPI
+ */
+#define CONFIG_LPC32XX_SSP
+#define CONFIG_LPC32XX_SSP_TIMEOUT	100000
+#define CONFIG_CMD_SPI
+
+/*
+ * Ethernet
+ */
+#define CONFIG_RMII
+#define CONFIG_PHY_SMSC
+#define CONFIG_LPC32XX_ETH
+#define CONFIG_PHYLIB
+#define CONFIG_PHY_ADDR			0x1F
+#define CONFIG_SYS_FAULT_ECHO_LINK_DOWN
+#define CONFIG_CMD_MII
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+
+/*
  * NOR Flash
  */
 #define CONFIG_SYS_MAX_FLASH_BANKS	1

@@ -56,6 +90,29 @@
 #define CONFIG_SYS_FLASH_CFI
 
 /*
+ * NAND controller
+ */
+#define CONFIG_NAND_LPC32XX_SLC
+#define CONFIG_SYS_NAND_BASE		SLC_NAND_BASE
+#define CONFIG_SYS_MAX_NAND_DEVICE	1
+#define CONFIG_SYS_NAND_BASE_LIST	{ CONFIG_SYS_NAND_BASE }
+
+/*
+ * NAND chip timings
+ */
+#define CONFIG_LPC32XX_NAND_SLC_WDR_CLKS	14
+#define CONFIG_LPC32XX_NAND_SLC_WWIDTH		66666666
+#define CONFIG_LPC32XX_NAND_SLC_WHOLD		200000000
+#define CONFIG_LPC32XX_NAND_SLC_WSETUP		50000000
+#define CONFIG_LPC32XX_NAND_SLC_RDR_CLKS	14
+#define CONFIG_LPC32XX_NAND_SLC_RWIDTH		66666666
+#define CONFIG_LPC32XX_NAND_SLC_RHOLD		200000000
+#define CONFIG_LPC32XX_NAND_SLC_RSETUP		50000000
+
+#define CONFIG_SYS_NAND_USE_FLASH_BBT
+#define CONFIG_CMD_NAND
+
+/*
  * U-Boot General Configurations
  */
 #define CONFIG_SYS_LONGHELP

@@ -71,8 +128,33 @@
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DOS_PARTITION
 
-#define CONFIG_ENV_IS_NOWHERE
+/*

include/configs/devkit3250.h +17 -2

+#define CONFIG_BOOTCOMMAND			\
+	"dhcp; "				\
+	"tftp ${loadaddr} ${serverip}:${tftpdir}/${bootfile}; "		\
+	"tftp ${dtbaddr} ${serverip}:${tftpdir}/devkit3250.dtb; "	\
+	"setenv nfsargs ip=dhcp root=/dev/nfs nfsroot=${serverip}:${nfsroot},tcp; "	\
+	"setenv bootargs ${bootargs} ${nfsargs} ${userargs}; "			\
+	"bootm ${loadaddr} - ${dtbaddr}"
+
+#define CONFIG_EXTRA_ENV_SETTINGS		\
+	"autoload=no\0"				\
+	"ethaddr=00:01:90:00:C0:81\0"		\
+	"dtbaddr=0x81000000\0"			\
+	"nfsroot=/opt/projects/images/vladimir/oe/devkit3250/rootfs\0"	\
+	"tftpdir=vladimir/oe/devkit3250\0"	\
+	"userargs=oops=panic\0"
 
 /*
  * U-Boot Commands

@@ -85,10 +167,10 @@
 #define CONFIG_CMDLINE_TAG
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_ZERO_BOOTDELAY_CHECK
-#define CONFIG_BOOTDELAY		3
+#define CONFIG_BOOTDELAY		1
 
 #define CONFIG_BOOTFILE			"uImage"
-#define CONFIG_BOOTARGS			"console=ttyS2,115200n8"
+#define CONFIG_BOOTARGS			"console=ttyS0,115200n8"
 #define CONFIG_LOADADDR			0x80008000
 
 /*

Signature · 1 line

2.1.4

Hello Vladimir,

Quoted 100 lines · Vladimir Zapolskiy

This change adds more peripherals to Timll DevKit3250 board, namely MAC and SMSC phy, SLC NAND, GPIO, SPI and I2C.

Also the default serial console is changed to UART5, added an option to pass device tree blob by means of bootm, predefined environment variables are slightly extended and reserved space on NAND to store user defined U-boot environment.

Signed-off-by Vladimir Zapolskiy <[email protected]>

---
 board/timll/devkit3250/devkit3250.c | 31 ++++++++++++-
 configs/devkit3250_defconfig        |  3 ++
 include/configs/devkit3250.h        | 92 +++++++++++++++++++++++++++++++++++--
 3 files changed, 120 insertions(+), 6 deletions(-)

board/timll/devkit3250/devkit3250.c +30 -1

index 6acc416..4b3c94e 100644
@@ -1,23 +1,52 @@
 /*
  * Embest/Timll DevKit3250 board support
  *
- * Copyright (C) 2011 Vladimir Zapolskiy <[email protected]>
+ * Copyright (C) 2011-2015 Vladimir Zapolskiy <[email protected]>
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>
 #include <asm/arch/sys_proto.h>
+#include <asm/arch/clk.h>
 #include <asm/arch/cpu.h>
 #include <asm/arch/emc.h>
+#include <asm/arch/wdt.h>
+#include <asm/io.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 static struct emc_regs *emc = (struct emc_regs *)EMC_BASE;
+static struct clk_pm_regs *clk = (struct clk_pm_regs *)CLK_PM_BASE;
+static struct wdt_regs *wdt = (struct wdt_regs *)WDT_BASE;
+
+void reset_periph(void)
+{
+	/* This function resets peripherals by triggering RESOUT_N */
+	setbits_le32(&clk->timclk_ctrl, CLK_TIMCLK_WATCHDOG);
+	writel(WDTIM_MCTRL_RESFRC1, &wdt->mctrl);
+	udelay(300);
+
+	writel(0, &wdt->mctrl);
+	clrbits_le32(&clk->timclk_ctrl, CLK_TIMCLK_WATCHDOG);
+
+	/* Such a long delay is needed to initialize SMSC phy */
+	udelay(10000);
+}
 
 int board_early_init_f(void)
 {
 	lpc32xx_uart_init(CONFIG_SYS_LPC32XX_UART);
+	lpc32xx_i2c_init(1);
+	lpc32xx_i2c_init(2);
+	lpc32xx_ssp_init();
+	lpc32xx_mac_init();
+
+	/*
+	 * nWP may be controlled by GPO19, but unpopulated by default R23
+	 * makes no sense to configure this GPIO level, nWP is always high
+	 */
+	lpc32xx_slc_nand_init();
 
 	return 0;
 }

configs/devkit3250_defconfig +3 -0

index f0c4ee1..56d719f 100644
@@ -1,3 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_DEVKIT3250=y
+# CONFIG_CMD_FPGA is not set
 # CONFIG_CMD_SETEXPR is not set
+CONFIG_DM=y
+CONFIG_DM_GPIO=y

include/configs/devkit3250.h +1 -1

index 4f35234..b8218b5 100644
@@ -1,7 +1,7 @@
 /*
  * Embest/Timll DevKit3250 board configuration file
  *
- * Copyright (C) 2011 Vladimir Zapolskiy <[email protected]>
+ * Copyright (C) 2011-2015 Vladimir Zapolskiy <[email protected]>
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */

@@ -43,10 +43,44 @@
 /*
  * Serial Driver

include/configs/devkit3250.h line 46

Quoted 3 lines

include/configs/devkit3250.h +1 -1

  */
-#define CONFIG_SYS_LPC32XX_UART		2   /* UART2 */
+#define CONFIG_SYS_LPC32XX_UART		5   /* UART5 */

This will affect existing users of this board, who expect it to use UART2 as console. Was this expectation wrong, or can both UART2 and UART5 be used as console? IOW, is this change required or is it just easier for you secifically?

Quoted 83 lines

include/configs/devkit3250.h +65 -1

 #define CONFIG_BAUDRATE			115200
 
 /*
+ * I2C
+ */
+#define CONFIG_SYS_I2C
+#define CONFIG_SYS_I2C_LPC32XX
+#define CONFIG_SYS_I2C_SPEED		100000
+#define CONFIG_CMD_I2C
+
+/*
+ * GPIO
+ */
+#define CONFIG_LPC32XX_GPIO
+#define CONFIG_CMD_GPIO
+
+/*
+ * SSP/SPI
+ */
+#define CONFIG_LPC32XX_SSP
+#define CONFIG_LPC32XX_SSP_TIMEOUT	100000
+#define CONFIG_CMD_SPI
+
+/*
+ * Ethernet
+ */
+#define CONFIG_RMII
+#define CONFIG_PHY_SMSC
+#define CONFIG_LPC32XX_ETH
+#define CONFIG_PHYLIB
+#define CONFIG_PHY_ADDR			0x1F
+#define CONFIG_SYS_FAULT_ECHO_LINK_DOWN
+#define CONFIG_CMD_MII
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+
+/*
  * NOR Flash
  */
 #define CONFIG_SYS_MAX_FLASH_BANKS	1

@@ -56,6 +90,29 @@
 #define CONFIG_SYS_FLASH_CFI
 
 /*
+ * NAND controller
+ */
+#define CONFIG_NAND_LPC32XX_SLC
+#define CONFIG_SYS_NAND_BASE		SLC_NAND_BASE
+#define CONFIG_SYS_MAX_NAND_DEVICE	1
+#define CONFIG_SYS_NAND_BASE_LIST	{ CONFIG_SYS_NAND_BASE }
+
+/*
+ * NAND chip timings
+ */
+#define CONFIG_LPC32XX_NAND_SLC_WDR_CLKS	14
+#define CONFIG_LPC32XX_NAND_SLC_WWIDTH		66666666
+#define CONFIG_LPC32XX_NAND_SLC_WHOLD		200000000
+#define CONFIG_LPC32XX_NAND_SLC_WSETUP		50000000
+#define CONFIG_LPC32XX_NAND_SLC_RDR_CLKS	14
+#define CONFIG_LPC32XX_NAND_SLC_RWIDTH		66666666
+#define CONFIG_LPC32XX_NAND_SLC_RHOLD		200000000
+#define CONFIG_LPC32XX_NAND_SLC_RSETUP		50000000
+
+#define CONFIG_SYS_NAND_USE_FLASH_BBT
+#define CONFIG_CMD_NAND
+
+/*
  * U-Boot General Configurations
  */
 #define CONFIG_SYS_LONGHELP

@@ -71,8 +128,33 @@
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DOS_PARTITION
 
-#define CONFIG_ENV_IS_NOWHERE
+/*
+ * Pass open firmware flat tree
+ */
+#define CONFIG_OF_LIBFDT
+
+/*
+ * Environment
+ */

include/configs/devkit3250.h line 141

Quoted 3 lines

include/configs/devkit3250.h +2 -0

+#define CONFIG_ENV_IS_IN_NAND		1
 #define CONFIG_ENV_SIZE			SZ_128K
+#define CONFIG_ENV_OFFSET		0x000A0000

This in itself is an unanticipated change too, but I think it does not affect current users of the boards much, except for a warning at boot that the (NAND) env is invalid -- but the env does not contain much anyway.

But how does this affect potential mapping of the NAND? Do Linux images hard-code partitions on the NAND and is the address and size chosen compatible with that, or do the images just not care about NAND?

Quoted 30 lines

include/configs/devkit3250.h +17 -2

+#define CONFIG_BOOTCOMMAND			\
+	"dhcp; "				\
+	"tftp ${loadaddr} ${serverip}:${tftpdir}/${bootfile}; "		\
+	"tftp ${dtbaddr} ${serverip}:${tftpdir}/devkit3250.dtb; "	\
+	"setenv nfsargs ip=dhcp root=/dev/nfs nfsroot=${serverip}:${nfsroot},tcp; "	\
+	"setenv bootargs ${bootargs} ${nfsargs} ${userargs}; "			\
+	"bootm ${loadaddr} - ${dtbaddr}"
+
+#define CONFIG_EXTRA_ENV_SETTINGS		\
+	"autoload=no\0"				\
+	"ethaddr=00:01:90:00:C0:81\0"		\
+	"dtbaddr=0x81000000\0"			\
+	"nfsroot=/opt/projects/images/vladimir/oe/devkit3250/rootfs\0"	\
+	"tftpdir=vladimir/oe/devkit3250\0"	\
+	"userargs=oops=panic\0"
 
 /*
  * U-Boot Commands

@@ -85,10 +167,10 @@
 #define CONFIG_CMDLINE_TAG
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_ZERO_BOOTDELAY_CHECK
-#define CONFIG_BOOTDELAY		3
+#define CONFIG_BOOTDELAY		1
 
 #define CONFIG_BOOTFILE			"uImage"
-#define CONFIG_BOOTARGS			"console=ttyS2,115200n8"
+#define CONFIG_BOOTARGS			"console=ttyS0,115200n8"
 #define CONFIG_LOADADDR			0x80008000
 

Quoted 4 lines

include/configs/devkit3250.h +0 -0

 /*

Signature · 1 line

2.1.4