[U-Boot] [PATCH 0/3] mtd/NAND: Support for FSMC controller

[U-Boot] [PATCH 0/3] mtd/NAND: Support for FSMC controller

[Amit Virdi]

This patchset adds support for ST's FSMC controller. In the current u-boot, a
SPEAr specific driver exists for FSMC controller.  This patchset adds a full
fledged driver that can be used across multiple platforms and removes the
obsolete SPEAr specific driver.

Vipin KUMAR (3):
  mtd/NAND: Add FSMC driver support
  SPEAr: Configure FSMC driver for NAND interface
  mtd/NAND: Remove obsolete SPEAr specific NAND drivers

 arch/arm/include/asm/arch-spear/hardware.h |    8 +-
 arch/arm/include/asm/arch-spear/spr_nand.h |   57 ----
 board/spear/spear300/spear300.c            |    7 +-
 board/spear/spear310/spear310.c            |    7 +-
 board/spear/spear320/spear320.c            |    7 +-
 board/spear/spear600/spear600.c            |    7 +-
 drivers/mtd/nand/Makefile                  |    1 +
 drivers/mtd/nand/fsmc_nand.c               |  441 ++++++++++++++++++++++++++++
 drivers/mtd/nand/spr_nand.c                |  124 --------
 include/configs/spear-common.h             |    2 +-
 include/configs/spear3xx.h                 |    4 +
 include/configs/spear6xx.h                 |    3 +
 include/linux/mtd/fsmc_nand.h              |  110 +++++++
 13 files changed, 580 insertions(+), 198 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-spear/spr_nand.h
 create mode 100644 drivers/mtd/nand/fsmc_nand.c
 delete mode 100644 drivers/mtd/nand/spr_nand.c
 create mode 100644 include/linux/mtd/fsmc_nand.h

-- 
1.7.2.2

[U-Boot] [PATCH 1/3] mtd/NAND: Add FSMC driver support

[Amit Virdi]

From: Vipin KUMAR <vipin.kumar@st.com>

Flexible static memory controller is a peripheral provided by ST,
which controls the access to NAND chips along with many other
memory device chips eg NOR, SRAM.

This patch adds the driver support for FSMC controller interfacing
with NAND memory.

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Amit Virdi <amit.virdi@st.com>
---
 drivers/mtd/nand/Makefile     |    1 +
 drivers/mtd/nand/fsmc_nand.c  |  441 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/fsmc_nand.h |  110 ++++++++++
 3 files changed, 552 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mtd/nand/fsmc_nand.c
 create mode 100644 include/linux/mtd/fsmc_nand.h

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 998fc73..517e5c1 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -49,6 +49,7 @@ COBJS-$(CONFIG_NAND_DAVINCI) += davinci_nand.o
 COBJS-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_nand.o
 COBJS-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_nand.o
 COBJS-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
+COBJS-$(CONFIG_NAND_FSMC) += fsmc_nand.o
 COBJS-$(CONFIG_NAND_JZ4740) += jz4740_nand.o
 COBJS-$(CONFIG_NAND_KB9202) += kb9202_nand.o
 COBJS-$(CONFIG_NAND_KIRKWOOD) += kirkwood_nand.o
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
new file mode 100644
index 0000000..a97dc0d
--- /dev/null
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -0,0 +1,441 @@
+/*
+ * (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
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/mtd/nand_ecc.h>
+#include <linux/mtd/fsmc_nand.h>
+#include <asm/arch/hardware.h>
+
+static u32 fsmc_version;
+static struct fsmc_regs *const fsmc_regs_p = (struct fsmc_regs *)
+	CONFIG_SYS_FSMC_BASE;
+
+/*
+ * ECC4 and ECC1 have 13 bytes and 3 bytes of ecc respectively for 512 bytes of
+ * data. ECC4 can correct up to 8 bits in 512 bytes of data while ECC1 can
+ * correct 1 bit in 512 bytes
+ */
+
+static struct nand_ecclayout fsmc_ecc4_lp_layout = {
+	.eccbytes = 104,
+	.eccpos = {  2,   3,   4,   5,   6,   7,   8,
+		9,  10,  11,  12,  13,  14,
+		18,  19,  20,  21,  22,  23,  24,
+		25,  26,  27,  28,  29,  30,
+		34,  35,  36,  37,  38,  39,  40,
+		41,  42,  43,  44,  45,  46,
+		50,  51,  52,  53,  54,  55,  56,
+		57,  58,  59,  60,  61,  62,
+		66,  67,  68,  69,  70,  71,  72,
+		73,  74,  75,  76,  77,  78,
+		82,  83,  84,  85,  86,  87,  88,
+		89,  90,  91,  92,  93,  94,
+		98,  99, 100, 101, 102, 103, 104,
+		105, 106, 107, 108, 109, 110,
+		114, 115, 116, 117, 118, 119, 120,
+		121, 122, 123, 124, 125, 126
+	},
+	.oobfree = {
+		{.offset = 15, .length = 3},
+		{.offset = 31, .length = 3},
+		{.offset = 47, .length = 3},
+		{.offset = 63, .length = 3},
+		{.offset = 79, .length = 3},
+		{.offset = 95, .length = 3},
+		{.offset = 111, .length = 3},
+		{.offset = 127, .length = 1}
+	}
+};
+
+/*
+ * ECC4 layout for NAND of pagesize 4096 bytes & OOBsize 224 bytes. 13*8 bytes
+ * of OOB size is reserved for ECC, Byte no. 0 & 1 reserved for bad block & 118
+ * bytes are free for use.
+ */
+static struct nand_ecclayout fsmc_ecc4_224_layout = {
+	.eccbytes = 104,
+	.eccpos = {  2,   3,   4,   5,   6,   7,   8,
+		9,  10,  11,  12,  13,  14,
+		18,  19,  20,  21,  22,  23,  24,
+		25,  26,  27,  28,  29,  30,
+		34,  35,  36,  37,  38,  39,  40,
+		41,  42,  43,  44,  45,  46,
+		50,  51,  52,  53,  54,  55,  56,
+		57,  58,  59,  60,  61,  62,
+		66,  67,  68,  69,  70,  71,  72,
+		73,  74,  75,  76,  77,  78,
+		82,  83,  84,  85,  86,  87,  88,
+		89,  90,  91,  92,  93,  94,
+		98,  99, 100, 101, 102, 103, 104,
+		105, 106, 107, 108, 109, 110,
+		114, 115, 116, 117, 118, 119, 120,
+		121, 122, 123, 124, 125, 126
+	},
+	.oobfree = {
+		{.offset = 15, .length = 3},
+		{.offset = 31, .length = 3},
+		{.offset = 47, .length = 3},
+		{.offset = 63, .length = 3},
+		{.offset = 79, .length = 3},
+		{.offset = 95, .length = 3},
+		{.offset = 111, .length = 3},
+		{.offset = 127, .length = 97}
+	}
+};
+
+/*
+ * ECC placement definitions in oobfree type format
+ * There are 13 bytes of ecc for every 512 byte block and it has to be read
+ * consecutively and immediately after the 512 byte data block for hardware to
+ * generate the error bit offsets in 512 byte data
+ * Managing the ecc bytes in the following way makes it easier for software to
+ * read ecc bytes consecutive to data bytes. This way is similar to
+ * oobfree structure maintained already in u-boot nand driver
+ */
+static struct fsmc_eccplace fsmc_eccpl_lp = {
+	.eccplace = {
+		{.offset = 2, .length = 13},
+		{.offset = 18, .length = 13},
+		{.offset = 34, .length = 13},
+		{.offset = 50, .length = 13},
+		{.offset = 66, .length = 13},
+		{.offset = 82, .length = 13},
+		{.offset = 98, .length = 13},
+		{.offset = 114, .length = 13}
+	}
+};
+
+static struct nand_ecclayout fsmc_ecc4_sp_layout = {
+	.eccbytes = 13,
+	.eccpos = { 0,  1,  2,  3,  6,  7, 8,
+		9, 10, 11, 12, 13, 14
+	},
+	.oobfree = {
+		{.offset = 15, .length = 1},
+	}
+};
+
+static struct fsmc_eccplace fsmc_eccpl_sp = {
+	.eccplace = {
+		{.offset = 0, .length = 4},
+		{.offset = 6, .length = 9}
+	}
+};
+
+static struct nand_ecclayout fsmc_ecc1_layout = {
+	.eccbytes = 24,
+	.eccpos = {2, 3, 4, 18, 19, 20, 34, 35, 36, 50, 51, 52,
+		66, 67, 68, 82, 83, 84, 98, 99, 100, 114, 115, 116},
+	.oobfree = {
+		{.offset = 8, .length = 8},
+		{.offset = 24, .length = 8},
+		{.offset = 40, .length = 8},
+		{.offset = 56, .length = 8},
+		{.offset = 72, .length = 8},
+		{.offset = 88, .length = 8},
+		{.offset = 104, .length = 8},
+		{.offset = 120, .length = 8}
+	}
+};
+
+static void fsmc_nand_hwcontrol(struct mtd_info *mtd, int cmd, uint ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+	ulong IO_ADDR_W;
+
+	if (ctrl & NAND_CTRL_CHANGE) {
+		IO_ADDR_W = (ulong)this->IO_ADDR_W;
+
+		IO_ADDR_W &= ~(CONFIG_SYS_NAND_CLE | CONFIG_SYS_NAND_ALE);
+		if (ctrl & NAND_CLE)
+			IO_ADDR_W |= CONFIG_SYS_NAND_CLE;
+		if (ctrl & NAND_ALE)
+			IO_ADDR_W |= CONFIG_SYS_NAND_ALE;
+
+		if (ctrl & NAND_NCE) {
+			writel(readl(&fsmc_regs_p->pc) |
+					FSMC_ENABLE, &fsmc_regs_p->pc);
+		} else {
+			writel(readl(&fsmc_regs_p->pc) &
+					~FSMC_ENABLE, &fsmc_regs_p->pc);
+		}
+		this->IO_ADDR_W = (void *)IO_ADDR_W;
+	}
+
+	if (cmd != NAND_CMD_NONE)
+		writeb(cmd, this->IO_ADDR_W);
+}
+
+static int fsmc_bch8_correct_data(struct mtd_info *mtd, u_char *dat,
+		u_char *read_ecc, u_char *calc_ecc)
+{
+	/* The calculated ecc is actually the correction index in data */
+	u32 err_idx[8];
+	u64 ecc_data[2];
+	u32 num_err, i;
+
+	memcpy(ecc_data, calc_ecc, 13);
+
+	for (i = 0; i < 8; i++) {
+		if (i == 4) {
+			err_idx[4] = ((ecc_data[1] & 0x1) << 12) | ecc_data[0];
+			ecc_data[1] >>= 1;
+			continue;
+		}
+		err_idx[i] = (ecc_data[i/4] & 0x1FFF);
+		ecc_data[i/4] >>= 13;
+	}
+
+	num_err = (readl(&fsmc_regs_p->sts) >> 10) & 0xF;
+
+	if (num_err > 8)
+		return -EBADMSG;
+
+	i = 0;
+	while (num_err--) {
+		change_bit(0, &err_idx[i]);
+		change_bit(1, &err_idx[i]);
+
+		if (err_idx[i] <= 512 * 8) {
+			change_bit(err_idx[i], dat);
+			i++;
+		}
+	}
+	return i;
+}
+
+static int fsmc_read_hwecc(struct mtd_info *mtd,
+			const u_char *data, u_char *ecc)
+{
+	u_int ecc_tmp;
+
+	switch (fsmc_version) {
+	case FSMC_VER8:
+		/* Busy waiting for ecc computation to finish for 512 bytes */
+		while (!(readl(&fsmc_regs_p->sts) & FSMC_CODE_RDY))
+			;
+
+		ecc_tmp = readl(&fsmc_regs_p->ecc1);
+		ecc[0] = (u_char) (ecc_tmp >> 0);
+		ecc[1] = (u_char) (ecc_tmp >> 8);
+		ecc[2] = (u_char) (ecc_tmp >> 16);
+		ecc[3] = (u_char) (ecc_tmp >> 24);
+
+		ecc_tmp = readl(&fsmc_regs_p->ecc2);
+		ecc[4] = (u_char) (ecc_tmp >> 0);
+		ecc[5] = (u_char) (ecc_tmp >> 8);
+		ecc[6] = (u_char) (ecc_tmp >> 16);
+		ecc[7] = (u_char) (ecc_tmp >> 24);
+
+		ecc_tmp = readl(&fsmc_regs_p->ecc3);
+		ecc[8] = (u_char) (ecc_tmp >> 0);
+		ecc[9] = (u_char) (ecc_tmp >> 8);
+		ecc[10] = (u_char) (ecc_tmp >> 16);
+		ecc[11] = (u_char) (ecc_tmp >> 24);
+
+		ecc_tmp = readl(&fsmc_regs_p->sts);
+		ecc[12] = (u_char) (ecc_tmp >> 16);
+		break;
+
+	default:
+		ecc_tmp = readl(&fsmc_regs_p->ecc1);
+		ecc[0] = (u_char) (ecc_tmp >> 0);
+		ecc[1] = (u_char) (ecc_tmp >> 8);
+		ecc[2] = (u_char) (ecc_tmp >> 16);
+		break;
+	}
+
+	return 0;
+}
+
+void fsmc_enable_hwecc(struct mtd_info *mtd, int mode)
+{
+	writel(readl(&fsmc_regs_p->pc) & ~FSMC_ECCPLEN_256,
+			&fsmc_regs_p->pc);
+	writel(readl(&fsmc_regs_p->pc) & ~FSMC_ECCEN,
+			&fsmc_regs_p->pc);
+	writel(readl(&fsmc_regs_p->pc) | FSMC_ECCEN,
+			&fsmc_regs_p->pc);
+}
+
+/*
+ * fsmc_read_page_hwecc
+ * @mtd:	mtd info structure
+ * @chip:	nand chip info structure
+ * @buf:	buffer to store read data
+ * @page:	page number to read
+ *
+ * This routine is needed for fsmc verison 8 as reading from NAND chip has to be
+ * performed in a strict sequence as follows:
+ * data(512 byte) -> ecc(13 byte)
+ * After this read, fsmc hardware generates and reports error data bits(upto a
+ * max of 8 bits)
+ */
+static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
+				 uint8_t *buf, int page)
+{
+	struct fsmc_eccplace *fsmc_eccpl;
+	int i, j, k, s, stat, eccsize = chip->ecc.size;
+	int eccbytes = chip->ecc.bytes;
+	int eccsteps = chip->ecc.steps;
+	uint8_t *p = buf;
+	uint8_t *ecc_calc = chip->buffers->ecccalc;
+	uint8_t *ecc_code = chip->buffers->ecccode;
+	int off, len, group = 0;
+	/*
+	 * ecc_oob is intentionally taken as u16. In 16bit devices, we end up
+	 * reading 14 bytes (7 words) from oob. The local array is to maintain
+	 * word alignment
+	 */
+	uint16_t ecc_oob[7];
+	uint8_t *oob = (uint8_t *)&ecc_oob[0];
+
+	/* Differentiate between small and large page ecc place definitions */
+	if (mtd->writesize == 512)
+		fsmc_eccpl = &fsmc_eccpl_sp;
+	else
+		fsmc_eccpl = &fsmc_eccpl_lp;
+
+	for (i = 0, s = 0; s < eccsteps; s++, i += eccbytes, p += eccsize) {
+
+		chip->cmdfunc(mtd, NAND_CMD_READ0, s * eccsize, page);
+		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		chip->read_buf(mtd, p, eccsize);
+
+		for (j = 0; j < eccbytes;) {
+			off = fsmc_eccpl->eccplace[group].offset;
+			len = fsmc_eccpl->eccplace[group].length;
+			group++;
+
+			/*
+			 * length is intentionally kept a higher multiple of 2
+			 * to read at least 13 bytes even in case of 16 bit NAND
+			 * devices
+			 */
+			len = roundup(len, 2);
+			chip->cmdfunc(mtd, NAND_CMD_READOOB, off, page);
+			chip->read_buf(mtd, oob + j, len);
+			j += len;
+		}
+
+		memcpy(&ecc_code[i], ecc_oob, 13);
+		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+
+		/*
+		 * This is a temporary erase check. A newly erased page read
+		 * would result in an ecc error because the oob data is also
+		 * erased to FF and the calculated ecc for an FF data is not
+		 * FF..FF.
+		 * This is a workaround to skip performing correction in case
+		 * data is FF..FF
+		 */
+		for (k = 0; k < eccsize; k++) {
+			if (*(p + k) != 0xff)
+				break;
+		}
+
+		if (k < eccsize) {
+			stat = chip->ecc.correct(mtd, p, &ecc_code[i],
+					&ecc_calc[i]);
+			if (stat < 0)
+				mtd->ecc_stats.failed++;
+			else
+				mtd->ecc_stats.corrected += stat;
+		}
+	}
+
+	return 0;
+}
+
+int fsmc_nand_init(struct nand_chip *nand)
+{
+	static int chip_nr;
+	struct mtd_info *mtd;
+	u32 peripid2 = readl(&fsmc_regs_p->peripid2);
+
+	fsmc_version = (peripid2 >> FSMC_REVISION_SHFT) &
+		FSMC_REVISION_MSK;
+
+	writel(readl(&fsmc_regs_p->ctrl) | FSMC_WP, &fsmc_regs_p->ctrl);
+
+#if defined(CONFIG_SYS_FSMC_NAND_16BIT)
+	writel(FSMC_DEVWID_16 | FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON,
+			&fsmc_regs_p->pc);
+#elif defined(CONFIG_SYS_FSMC_NAND_8BIT)
+	writel(FSMC_DEVWID_8 | FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON,
+			&fsmc_regs_p->pc);
+#else
+#error Please define CONFIG_SYS_FSMC_NAND_16BIT or CONFIG_SYS_FSMC_NAND_8BIT
+#endif
+	writel(readl(&fsmc_regs_p->pc) | FSMC_TCLR_1 | FSMC_TAR_1,
+			&fsmc_regs_p->pc);
+	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
+			&fsmc_regs_p->comm);
+	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
+			&fsmc_regs_p->attrib);
+
+	nand->options = 0;
+#if defined(CONFIG_SYS_FSMC_NAND_16BIT)
+	nand->options |= NAND_BUSWIDTH_16;
+#endif
+	nand->ecc.mode = NAND_ECC_HW;
+	nand->ecc.size = 512;
+	nand->ecc.calculate = fsmc_read_hwecc;
+	nand->ecc.hwctl = fsmc_enable_hwecc;
+	nand->cmd_ctrl = fsmc_nand_hwcontrol;
+
+	mtd = &nand_info[chip_nr++];
+	mtd->priv = nand;
+
+	/* Detect NAND chips */
+	if (nand_scan_ident(mtd, 1, NULL))
+		return -ENXIO;
+
+	switch (fsmc_version) {
+	case FSMC_VER8:
+		nand->ecc.bytes = 13;
+		nand->ecc.correct = fsmc_bch8_correct_data;
+		nand->ecc.read_page = fsmc_read_page_hwecc;
+		if (mtd->writesize == 512)
+			nand->ecc.layout = &fsmc_ecc4_sp_layout;
+		else {
+			if (mtd->oobsize == 224)
+				nand->ecc.layout = &fsmc_ecc4_224_layout;
+			else
+				nand->ecc.layout = &fsmc_ecc4_lp_layout;
+		}
+
+		break;
+	default:
+		nand->ecc.bytes = 3;
+		nand->ecc.layout = &fsmc_ecc1_layout;
+		nand->ecc.correct = nand_correct_data;
+		break;
+	}
+
+	return 0;
+}
diff --git a/include/linux/mtd/fsmc_nand.h b/include/linux/mtd/fsmc_nand.h
new file mode 100644
index 0000000..2609973
--- /dev/null
+++ b/include/linux/mtd/fsmc_nand.h
@@ -0,0 +1,110 @@
+/*
+ * (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
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef __FSMC_NAND_H__
+#define __FSMC_NAND_H__
+
+#include <linux/mtd/nand.h>
+
+struct fsmc_regs {
+	u32 ctrl;			/* 0x00 */
+	u8 reserved_1[0x40 - 0x04];
+	u32 pc;				/* 0x40 */
+	u32 sts;			/* 0x44 */
+	u32 comm;			/* 0x48 */
+	u32 attrib;			/* 0x4c */
+	u32 ioata;			/* 0x50 */
+	u32 ecc1;			/* 0x54 */
+	u32 ecc2;			/* 0x58 */
+	u32 ecc3;			/* 0x5c */
+	u8 reserved_2[0xfe0 - 0x60];
+	u32 peripid0;			/* 0xfe0 */
+	u32 peripid1;			/* 0xfe4 */
+	u32 peripid2;			/* 0xfe8 */
+	u32 peripid3;			/* 0xfec */
+	u32 pcellid0;			/* 0xff0 */
+	u32 pcellid1;			/* 0xff4 */
+	u32 pcellid2;			/* 0xff8 */
+	u32 pcellid3;			/* 0xffc */
+};
+
+/* ctrl register definitions */
+#define FSMC_WP			(1 << 7)
+
+/* pc register definitions */
+#define FSMC_RESET		(1 << 0)
+#define FSMC_WAITON		(1 << 1)
+#define FSMC_ENABLE		(1 << 2)
+#define FSMC_DEVTYPE_NAND	(1 << 3)
+#define FSMC_DEVWID_8		(0 << 4)
+#define FSMC_DEVWID_16		(1 << 4)
+#define FSMC_ECCEN		(1 << 6)
+#define FSMC_ECCPLEN_512	(0 << 7)
+#define FSMC_ECCPLEN_256	(1 << 7)
+#define FSMC_TCLR_1		(1 << 9)
+#define FSMC_TAR_1		(1 << 13)
+
+/* sts register definitions */
+#define FSMC_CODE_RDY		(1 << 15)
+
+/* comm register definitions */
+#define FSMC_TSET_0		(0 << 0)
+#define FSMC_TWAIT_6		(6 << 8)
+#define FSMC_THOLD_4		(4 << 16)
+#define FSMC_THIZ_1		(1 << 24)
+
+/* peripid2 register definitions */
+#define FSMC_REVISION_MSK	(0xf)
+#define FSMC_REVISION_SHFT	(0x4)
+
+enum {
+	FSMC_VER1 = 1,
+	FSMC_VER2,
+	FSMC_VER3,
+	FSMC_VER4,
+	FSMC_VER5,
+	FSMC_VER6,
+	FSMC_VER7,
+	FSMC_VER8,
+};
+
+/*
+ * There are 13 bytes of ecc for every 512 byte block and it has to be read
+ * consecutively and immediately after the 512 byte data block for hardware to
+ * generate the error bit offsets
+ * Managing the ecc bytes in the following way is easier. This way is similar to
+ * oobfree structure maintained already in u-boot nand driver
+ */
+#define MAX_ECCPLACE_ENTRIES	32
+
+struct fsmc_nand_eccplace {
+	u32 offset;
+	u32 length;
+};
+
+struct fsmc_eccplace {
+	struct fsmc_nand_eccplace eccplace[MAX_ECCPLACE_ENTRIES];
+};
+
+extern int fsmc_nand_init(struct nand_chip *nand);
+#endif
-- 
1.7.2.2

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

From: Vipin KUMAR <vipin.kumar@st.com>

Since FSMC is a standard IP and it supports different memory interfaces, it
is supported independent of spear platform and spear is configured to use that
driver for interfacing with the NAND device

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Amit Virdi <amit.virdi@st.com>
---
 arch/arm/include/asm/arch-spear/hardware.h |    8 ++++----
 board/spear/spear300/spear300.c            |    7 ++++---
 board/spear/spear310/spear310.c            |    7 ++++---
 board/spear/spear320/spear320.c            |    7 ++++---
 board/spear/spear600/spear600.c            |    7 ++++---
 include/configs/spear-common.h             |    2 +-
 include/configs/spear3xx.h                 |    4 ++++
 include/configs/spear6xx.h                 |    3 +++
 8 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/arch/arm/include/asm/arch-spear/hardware.h b/arch/arm/include/asm/arch-spear/hardware.h
index 818f36c..a6517b2 100644
--- a/arch/arm/include/asm/arch-spear/hardware.h
+++ b/arch/arm/include/asm/arch-spear/hardware.h
@@ -37,15 +37,15 @@
 
 #if defined(CONFIG_SPEAR600)
 #define CONFIG_SYS_I2C_BASE			(0xD0200000)
-#define CONFIG_SPEAR_FSMCBASE			(0xD1800000)
+#define CONFIG_SYS_FSMC_BASE			(0xD1800000)
 
 #elif defined(CONFIG_SPEAR300)
 #define CONFIG_SYS_I2C_BASE			(0xD0180000)
-#define CONFIG_SPEAR_FSMCBASE			(0x94000000)
+#define CONFIG_SYS_FSMC_BASE			(0x94000000)
 
 #elif defined(CONFIG_SPEAR310)
 #define CONFIG_SYS_I2C_BASE			(0xD0180000)
-#define CONFIG_SPEAR_FSMCBASE			(0x44000000)
+#define CONFIG_SYS_FSMC_BASE			(0x44000000)
 
 #undef CONFIG_SYS_NAND_CLE
 #undef CONFIG_SYS_NAND_ALE
@@ -57,7 +57,7 @@
 
 #elif defined(CONFIG_SPEAR320)
 #define CONFIG_SYS_I2C_BASE			(0xD0180000)
-#define CONFIG_SPEAR_FSMCBASE			(0x4C000000)
+#define CONFIG_SYS_FSMC_BASE			(0x4C000000)
 
 #define CONFIG_SPEAR_EMIBASE			(0x40000000)
 #define CONFIG_SPEAR_RASBASE			(0xB3000000)
diff --git a/board/spear/spear300/spear300.c b/board/spear/spear300/spear300.c
index 60ee544..32bcb77 100644
--- a/board/spear/spear300/spear300.c
+++ b/board/spear/spear300/spear300.c
@@ -24,10 +24,10 @@
 #include <common.h>
 #include <nand.h>
 #include <asm/io.h>
+#include <linux/mtd/fsmc_nand.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/spr_defs.h>
 #include <asm/arch/spr_misc.h>
-#include <asm/arch/spr_nand.h>
 
 int board_init(void)
 {
@@ -46,13 +46,14 @@ int board_nand_init(struct nand_chip *nand)
 	struct misc_regs *const misc_regs_p =
 	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
 
+#if defined(CONFIG_NAND_FSMC)
 	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG30) ||
 	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG31)) {
 
-		return spear_nand_init(nand);
+		return fsmc_nand_init(nand);
 	}
-
+#endif
 	return -1;
 }
diff --git a/board/spear/spear310/spear310.c b/board/spear/spear310/spear310.c
index 03dfe16..8b58218 100644
--- a/board/spear/spear310/spear310.c
+++ b/board/spear/spear310/spear310.c
@@ -25,10 +25,10 @@
 #include <common.h>
 #include <nand.h>
 #include <asm/io.h>
+#include <linux/mtd/fsmc_nand.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/spr_defs.h>
 #include <asm/arch/spr_misc.h>
-#include <asm/arch/spr_nand.h>
 
 int board_init(void)
 {
@@ -47,13 +47,14 @@ int board_nand_init(struct nand_chip *nand)
 	struct misc_regs *const misc_regs_p =
 	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
 
+#if defined(CONFIG_NAND_FSMC)
 	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG30) ||
 	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG31)) {
 
-		return spear_nand_init(nand);
+		return fsmc_nand_init(nand);
 	}
-
+#endif
 	return -1;
 }
diff --git a/board/spear/spear320/spear320.c b/board/spear/spear320/spear320.c
index 2ba2dbb..172ad35 100644
--- a/board/spear/spear320/spear320.c
+++ b/board/spear/spear320/spear320.c
@@ -25,10 +25,10 @@
 #include <common.h>
 #include <nand.h>
 #include <asm/io.h>
+#include <linux/mtd/fsmc_nand.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/spr_defs.h>
 #include <asm/arch/spr_misc.h>
-#include <asm/arch/spr_nand.h>
 
 int board_init(void)
 {
@@ -47,13 +47,14 @@ int board_nand_init(struct nand_chip *nand)
 	struct misc_regs *const misc_regs_p =
 	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
 
+#if defined(CONFIG_NAND_FSMC)
 	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG30) ||
 	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
 	     MISC_SOCCFG31)) {
 
-		return spear_nand_init(nand);
+		return fsmc_nand_init(nand);
 	}
-
+#endif
 	return -1;
 }
diff --git a/board/spear/spear600/spear600.c b/board/spear/spear600/spear600.c
index eef9a37..7cf63d6 100644
--- a/board/spear/spear600/spear600.c
+++ b/board/spear/spear600/spear600.c
@@ -24,10 +24,10 @@
 #include <common.h>
 #include <nand.h>
 #include <asm/io.h>
+#include <linux/mtd/fsmc_nand.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/spr_defs.h>
 #include <asm/arch/spr_misc.h>
-#include <asm/arch/spr_nand.h>
 
 int board_init(void)
 {
@@ -46,8 +46,9 @@ int board_nand_init(struct nand_chip *nand)
 	struct misc_regs *const misc_regs_p =
 	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
 
+#if defined(CONFIG_NAND_FSMC)
 	if (!(readl(&misc_regs_p->auto_cfg_reg) & MISC_NANDDIS))
-		return spear_nand_init(nand);
-
+		return fsmc_nand_init(nand);
+#endif
 	return -1;
 }
diff --git a/include/configs/spear-common.h b/include/configs/spear-common.h
index 516b78e..c37305f 100644
--- a/include/configs/spear-common.h
+++ b/include/configs/spear-common.h
@@ -90,7 +90,7 @@
 /* NAND FLASH Configuration */
 #define CONFIG_MTD_DEVICE
 #define CONFIG_MTD_PARTITIONS
-#define CONFIG_NAND_SPEAR			1
+#define CONFIG_NAND_FSMC
 #define CONFIG_SYS_MAX_NAND_DEVICE		1
 #define CONFIG_MTD_NAND_VERIFY_WRITE
 
diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h
index bd5d111..5bdd874 100644
--- a/include/configs/spear3xx.h
+++ b/include/configs/spear3xx.h
@@ -117,6 +117,10 @@
 
 #endif
 
+/* NAND flash configuration */
+#define CONFIG_SYS_FSMC_NAND_SP
+#define CONFIG_SYS_FSMC_NAND_8BIT
+
 #if defined(CONFIG_SPEAR300)
 #define CONFIG_SYS_NAND_BASE			0x80000000
 
diff --git a/include/configs/spear6xx.h b/include/configs/spear6xx.h
index 8de7ebd..1e06c72 100644
--- a/include/configs/spear6xx.h
+++ b/include/configs/spear6xx.h
@@ -38,6 +38,9 @@
 #define CONFIG_PL01x_PORTS			{ (void *)CONFIG_SYS_SERIAL0, \
 						(void *)CONFIG_SYS_SERIAL1 }
 
+/* NAND flash configuration */
+#define CONFIG_SYS_FSMC_NAND_SP
+#define CONFIG_SYS_FSMC_NAND_8BIT
 #define CONFIG_SYS_NAND_BASE			(0xD2000000)
 
 #endif  /* __CONFIG_H */
-- 
1.7.2.2

[U-Boot] [PATCH 3/3] mtd/NAND: Remove obsolete SPEAr specific NAND drivers

[Amit Virdi]

From: Vipin KUMAR <vipin.kumar@st.com>

Since, SPEAr platform uses generic FSMC driver now, so spear specific files
drivers/mtd/nand/spr_nand.c, arch/arm/include/asm/arch-spear/spr_nand.h are
removed

Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
Signed-off-by: Amit Virdi <amit.virdi@st.com>
---
 arch/arm/include/asm/arch-spear/spr_nand.h |   57 -------------
 drivers/mtd/nand/spr_nand.c                |  124 ----------------------------
 2 files changed, 0 insertions(+), 181 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-spear/spr_nand.h
 delete mode 100644 drivers/mtd/nand/spr_nand.c

diff --git a/arch/arm/include/asm/arch-spear/spr_nand.h b/arch/arm/include/asm/arch-spear/spr_nand.h
deleted file mode 100644
index 2b63dc7..0000000
--- a/arch/arm/include/asm/arch-spear/spr_nand.h
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * (C) Copyright 2009
- * 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
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#ifndef __SPR_NAND_H__
-#define __SPR_NAND_H__
-
-struct fsmc_regs {
-	u32 reserved_1[0x10];
-	u32 genmemctrl_pc;
-	u32 reserved_2;
-	u32 genmemctrl_comm;
-	u32 genmemctrl_attrib;
-	u32 reserved_3;
-	u32 genmemctrl_ecc;
-};
-
-/* genmemctrl_pc register definitions */
-#define FSMC_RESET		(1 << 0)
-#define FSMC_WAITON		(1 << 1)
-#define FSMC_ENABLE		(1 << 2)
-#define FSMC_DEVTYPE_NAND	(1 << 3)
-#define FSMC_DEVWID_8		(0 << 4)
-#define FSMC_DEVWID_16		(1 << 4)
-#define FSMC_ECCEN		(1 << 6)
-#define FSMC_ECCPLEN_512	(0 << 7)
-#define FSMC_ECCPLEN_256	(1 << 7)
-#define FSMC_TCLR_1		(1 << 9)
-#define FSMC_TAR_1		(1 << 13)
-
-/* genmemctrl_comm register definitions */
-#define FSMC_TSET_0		(0 << 0)
-#define FSMC_TWAIT_6		(6 << 8)
-#define FSMC_THOLD_4		(4 << 16)
-#define FSMC_THIZ_1		(1 << 24)
-
-extern int spear_nand_init(struct nand_chip *nand);
-#endif
diff --git a/drivers/mtd/nand/spr_nand.c b/drivers/mtd/nand/spr_nand.c
deleted file mode 100644
index 097d0c6..0000000
--- a/drivers/mtd/nand/spr_nand.c
+++ /dev/null
@@ -1,124 +0,0 @@
-/*
- * (C) Copyright 2009
- * Vipin Kumar, ST Micoelectronics, vipin.kumar@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
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <nand.h>
-#include <linux/mtd/nand_ecc.h>
-#include <asm/io.h>
-#include <asm/arch/hardware.h>
-#include <asm/arch/spr_nand.h>
-
-static struct fsmc_regs *const fsmc_regs_p =
-    (struct fsmc_regs *)CONFIG_SPEAR_FSMCBASE;
-
-static struct nand_ecclayout spear_nand_ecclayout = {
-	.eccbytes = 24,
-	.eccpos = {2, 3, 4, 18, 19, 20, 34, 35, 36, 50, 51, 52,
-		   66, 67, 68, 82, 83, 84, 98, 99, 100, 114, 115, 116},
-	.oobfree = {
-		    {.offset = 8, .length = 8},
-		    {.offset = 24, .length = 8},
-		    {.offset = 40, .length = 8},
-		    {.offset = 56, .length = 8},
-		    {.offset = 72, .length = 8},
-		    {.offset = 88, .length = 8},
-		    {.offset = 104, .length = 8},
-		    {.offset = 120, .length = 8}
-		    }
-};
-
-static void spear_nand_hwcontrol(struct mtd_info *mtd, int cmd, uint ctrl)
-{
-	struct nand_chip *this = mtd->priv;
-	ulong IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		IO_ADDR_W = (ulong)this->IO_ADDR_W;
-
-		IO_ADDR_W &= ~(CONFIG_SYS_NAND_CLE | CONFIG_SYS_NAND_ALE);
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W |= CONFIG_SYS_NAND_CLE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W |= CONFIG_SYS_NAND_ALE;
-
-		if (ctrl & NAND_NCE) {
-			writel(readl(&fsmc_regs_p->genmemctrl_pc) |
-			       FSMC_ENABLE, &fsmc_regs_p->genmemctrl_pc);
-		} else {
-			writel(readl(&fsmc_regs_p->genmemctrl_pc) &
-			       ~FSMC_ENABLE, &fsmc_regs_p->genmemctrl_pc);
-		}
-		this->IO_ADDR_W = (void *)IO_ADDR_W;
-	}
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-static int spear_read_hwecc(struct mtd_info *mtd,
-			    const u_char *data, u_char ecc[3])
-{
-	u_int ecc_tmp;
-
-	/* read the h/w ECC */
-	ecc_tmp = readl(&fsmc_regs_p->genmemctrl_ecc);
-
-	ecc[0] = (u_char) (ecc_tmp & 0xFF);
-	ecc[1] = (u_char) ((ecc_tmp & 0xFF00) >> 8);
-	ecc[2] = (u_char) ((ecc_tmp & 0xFF0000) >> 16);
-
-	return 0;
-}
-
-void spear_enable_hwecc(struct mtd_info *mtd, int mode)
-{
-	writel(readl(&fsmc_regs_p->genmemctrl_pc) & ~0x80,
-	       &fsmc_regs_p->genmemctrl_pc);
-	writel(readl(&fsmc_regs_p->genmemctrl_pc) & ~FSMC_ECCEN,
-	       &fsmc_regs_p->genmemctrl_pc);
-	writel(readl(&fsmc_regs_p->genmemctrl_pc) | FSMC_ECCEN,
-	       &fsmc_regs_p->genmemctrl_pc);
-}
-
-int spear_nand_init(struct nand_chip *nand)
-{
-	writel(FSMC_DEVWID_8 | FSMC_DEVTYPE_NAND | FSMC_ENABLE | FSMC_WAITON,
-	       &fsmc_regs_p->genmemctrl_pc);
-	writel(readl(&fsmc_regs_p->genmemctrl_pc) | FSMC_TCLR_1 | FSMC_TAR_1,
-	       &fsmc_regs_p->genmemctrl_pc);
-	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
-	       &fsmc_regs_p->genmemctrl_comm);
-	writel(FSMC_THIZ_1 | FSMC_THOLD_4 | FSMC_TWAIT_6 | FSMC_TSET_0,
-	       &fsmc_regs_p->genmemctrl_attrib);
-
-	nand->options = 0;
-	nand->ecc.mode = NAND_ECC_HW;
-	nand->ecc.layout = &spear_nand_ecclayout;
-	nand->ecc.size = 512;
-	nand->ecc.bytes = 3;
-	nand->ecc.calculate = spear_read_hwecc;
-	nand->ecc.hwctl = spear_enable_hwecc;
-	nand->ecc.correct = nand_correct_data;
-	nand->cmd_ctrl = spear_nand_hwcontrol;
-	return 0;
-}
-- 
1.7.2.2

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Stefan Roese]

Hi Amit,

please find a few comments below.

On Monday 27 February 2012 10:38:23 Amit Virdi wrote:
> From: Vipin KUMAR <vipin.kumar@st.com>
> 
> Since FSMC is a standard IP and it supports different memory interfaces, it
> is supported independent of spear platform and spear is configured to use
> that driver for interfacing with the NAND device
> 
> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
> Signed-off-by: Amit Virdi <amit.virdi@st.com>
> ---
>  arch/arm/include/asm/arch-spear/hardware.h |    8 ++++----
>  board/spear/spear300/spear300.c            |    7 ++++---
>  board/spear/spear310/spear310.c            |    7 ++++---
>  board/spear/spear320/spear320.c            |    7 ++++---
>  board/spear/spear600/spear600.c            |    7 ++++---
>  include/configs/spear-common.h             |    2 +-
>  include/configs/spear3xx.h                 |    4 ++++
>  include/configs/spear6xx.h                 |    3 +++
>  8 files changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-spear/hardware.h
> b/arch/arm/include/asm/arch-spear/hardware.h index 818f36c..a6517b2 100644
> --- a/arch/arm/include/asm/arch-spear/hardware.h
> +++ b/arch/arm/include/asm/arch-spear/hardware.h
> @@ -37,15 +37,15 @@
> 
>  #if defined(CONFIG_SPEAR600)
>  #define CONFIG_SYS_I2C_BASE			(0xD0200000)
> -#define CONFIG_SPEAR_FSMCBASE			(0xD1800000)
> +#define CONFIG_SYS_FSMC_BASE			(0xD1800000)

Please remove the parentheses here as they are not needed. Should be done to 
all those defines, perhaps in a cosmetic cleanup patch at some time as well:

+#define CONFIG_SYS_FSMC_BASE			0xD1800000
 
>  #elif defined(CONFIG_SPEAR300)
>  #define CONFIG_SYS_I2C_BASE			(0xD0180000)
> -#define CONFIG_SPEAR_FSMCBASE			(0x94000000)
> +#define CONFIG_SYS_FSMC_BASE			(0x94000000)
> 
>  #elif defined(CONFIG_SPEAR310)
>  #define CONFIG_SYS_I2C_BASE			(0xD0180000)
> -#define CONFIG_SPEAR_FSMCBASE			(0x44000000)
> +#define CONFIG_SYS_FSMC_BASE			(0x44000000)
> 
>  #undef CONFIG_SYS_NAND_CLE
>  #undef CONFIG_SYS_NAND_ALE
> @@ -57,7 +57,7 @@
> 
>  #elif defined(CONFIG_SPEAR320)
>  #define CONFIG_SYS_I2C_BASE			(0xD0180000)
> -#define CONFIG_SPEAR_FSMCBASE			(0x4C000000)
> +#define CONFIG_SYS_FSMC_BASE			(0x4C000000)
> 
>  #define CONFIG_SPEAR_EMIBASE			(0x40000000)
>  #define CONFIG_SPEAR_RASBASE			(0xB3000000)
> diff --git a/board/spear/spear300/spear300.c
> b/board/spear/spear300/spear300.c index 60ee544..32bcb77 100644
> --- a/board/spear/spear300/spear300.c
> +++ b/board/spear/spear300/spear300.c
> @@ -24,10 +24,10 @@
>  #include <common.h>
>  #include <nand.h>
>  #include <asm/io.h>
> +#include <linux/mtd/fsmc_nand.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/spr_defs.h>
>  #include <asm/arch/spr_misc.h>
> -#include <asm/arch/spr_nand.h>
> 
>  int board_init(void)
>  {
> @@ -46,13 +46,14 @@ int board_nand_init(struct nand_chip *nand)
>  	struct misc_regs *const misc_regs_p =
>  	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
> 
> +#if defined(CONFIG_NAND_FSMC)
>  	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG30) ||
>  	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG31)) {
> 
> -		return spear_nand_init(nand);
> +		return fsmc_nand_init(nand);
>  	}
> -
> +#endif
>  	return -1;
>  }
> diff --git a/board/spear/spear310/spear310.c
> b/board/spear/spear310/spear310.c index 03dfe16..8b58218 100644
> --- a/board/spear/spear310/spear310.c
> +++ b/board/spear/spear310/spear310.c
> @@ -25,10 +25,10 @@
>  #include <common.h>
>  #include <nand.h>
>  #include <asm/io.h>
> +#include <linux/mtd/fsmc_nand.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/spr_defs.h>
>  #include <asm/arch/spr_misc.h>
> -#include <asm/arch/spr_nand.h>
> 
>  int board_init(void)
>  {
> @@ -47,13 +47,14 @@ int board_nand_init(struct nand_chip *nand)
>  	struct misc_regs *const misc_regs_p =
>  	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
> 
> +#if defined(CONFIG_NAND_FSMC)
>  	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG30) ||
>  	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG31)) {
> 
> -		return spear_nand_init(nand);
> +		return fsmc_nand_init(nand);
>  	}
> -
> +#endif
>  	return -1;
>  }
> diff --git a/board/spear/spear320/spear320.c
> b/board/spear/spear320/spear320.c index 2ba2dbb..172ad35 100644
> --- a/board/spear/spear320/spear320.c
> +++ b/board/spear/spear320/spear320.c
> @@ -25,10 +25,10 @@
>  #include <common.h>
>  #include <nand.h>
>  #include <asm/io.h>
> +#include <linux/mtd/fsmc_nand.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/spr_defs.h>
>  #include <asm/arch/spr_misc.h>
> -#include <asm/arch/spr_nand.h>
> 
>  int board_init(void)
>  {
> @@ -47,13 +47,14 @@ int board_nand_init(struct nand_chip *nand)
>  	struct misc_regs *const misc_regs_p =
>  	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
> 
> +#if defined(CONFIG_NAND_FSMC)
>  	if (((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG30) ||
>  	    ((readl(&misc_regs_p->auto_cfg_reg) & MISC_SOCCFGMSK) ==
>  	     MISC_SOCCFG31)) {
> 
> -		return spear_nand_init(nand);
> +		return fsmc_nand_init(nand);
>  	}
> -
> +#endif
>  	return -1;
>  }
> diff --git a/board/spear/spear600/spear600.c
> b/board/spear/spear600/spear600.c index eef9a37..7cf63d6 100644
> --- a/board/spear/spear600/spear600.c
> +++ b/board/spear/spear600/spear600.c
> @@ -24,10 +24,10 @@
>  #include <common.h>
>  #include <nand.h>
>  #include <asm/io.h>
> +#include <linux/mtd/fsmc_nand.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/spr_defs.h>
>  #include <asm/arch/spr_misc.h>
> -#include <asm/arch/spr_nand.h>
> 
>  int board_init(void)
>  {
> @@ -46,8 +46,9 @@ int board_nand_init(struct nand_chip *nand)
>  	struct misc_regs *const misc_regs_p =
>  	    (struct misc_regs *)CONFIG_SPEAR_MISCBASE;
> 
> +#if defined(CONFIG_NAND_FSMC)
>  	if (!(readl(&misc_regs_p->auto_cfg_reg) & MISC_NANDDIS))
> -		return spear_nand_init(nand);
> -
> +		return fsmc_nand_init(nand);
> +#endif
>  	return -1;
>  }
> diff --git a/include/configs/spear-common.h
> b/include/configs/spear-common.h index 516b78e..c37305f 100644
> --- a/include/configs/spear-common.h
> +++ b/include/configs/spear-common.h
> @@ -90,7 +90,7 @@
>  /* NAND FLASH Configuration */
>  #define CONFIG_MTD_DEVICE
>  #define CONFIG_MTD_PARTITIONS
> -#define CONFIG_NAND_SPEAR			1
> +#define CONFIG_NAND_FSMC
>  #define CONFIG_SYS_MAX_NAND_DEVICE		1
>  #define CONFIG_MTD_NAND_VERIFY_WRITE

I suggest that you remove this last define. Most likely it was added for 
debugging purpose only. It slows down the speed and it brakes UBI support.
 
> diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h
> index bd5d111..5bdd874 100644
> --- a/include/configs/spear3xx.h
> +++ b/include/configs/spear3xx.h
> @@ -117,6 +117,10 @@
> 
>  #endif
> 
> +/* NAND flash configuration */
> +#define CONFIG_SYS_FSMC_NAND_SP
> +#define CONFIG_SYS_FSMC_NAND_8BIT
> +
>  #if defined(CONFIG_SPEAR300)
>  #define CONFIG_SYS_NAND_BASE			0x80000000
> 
> diff --git a/include/configs/spear6xx.h b/include/configs/spear6xx.h
> index 8de7ebd..1e06c72 100644
> --- a/include/configs/spear6xx.h
> +++ b/include/configs/spear6xx.h
> @@ -38,6 +38,9 @@
>  #define CONFIG_PL01x_PORTS			{ (void 
*)CONFIG_SYS_SERIAL0, \
>  						(void 
*)CONFIG_SYS_SERIAL1 }
> 
> +/* NAND flash configuration */
> +#define CONFIG_SYS_FSMC_NAND_SP
> +#define CONFIG_SYS_FSMC_NAND_8BIT

You also need the following define for this to work with the latest NAND 
subsystem:

#define CONFIG_MTD_ECC_SOFT

Not sure about SPEAr3xx. Most likely this needs it as well.

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

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

Hello Stefan,

On 2/27/2012 3:32 PM, Stefan Roese wrote:
> Hi Amit,
>
> please find a few comments below.
>
> On Monday 27 February 2012 10:38:23 Amit Virdi wrote:
>> From: Vipin KUMAR<vipin.kumar@st.com>
>>
>> Since FSMC is a standard IP and it supports different memory interfaces, it
>> is supported independent of spear platform and spear is configured to use
>> that driver for interfacing with the NAND device
>>
>> Signed-off-by: Vipin Kumar<vipin.kumar@st.com>
>> Signed-off-by: Amit Virdi<amit.virdi@st.com>
>> ---
>>   arch/arm/include/asm/arch-spear/hardware.h |    8 ++++----
>>   board/spear/spear300/spear300.c            |    7 ++++---
>>   board/spear/spear310/spear310.c            |    7 ++++---
>>   board/spear/spear320/spear320.c            |    7 ++++---
>>   board/spear/spear600/spear600.c            |    7 ++++---
>>   include/configs/spear-common.h             |    2 +-
>>   include/configs/spear3xx.h                 |    4 ++++
>>   include/configs/spear6xx.h                 |    3 +++
>>   8 files changed, 28 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-spear/hardware.h
>> b/arch/arm/include/asm/arch-spear/hardware.h index 818f36c..a6517b2 100644
>> --- a/arch/arm/include/asm/arch-spear/hardware.h
>> +++ b/arch/arm/include/asm/arch-spear/hardware.h
>> @@ -37,15 +37,15 @@
>>
>>   #if defined(CONFIG_SPEAR600)
>>   #define CONFIG_SYS_I2C_BASE			(0xD0200000)
>> -#define CONFIG_SPEAR_FSMCBASE			(0xD1800000)
>> +#define CONFIG_SYS_FSMC_BASE			(0xD1800000)
>
> Please remove the parentheses here as they are not needed. Should be done to
> all those defines, perhaps in a cosmetic cleanup patch at some time as well:
>

Yes. I'll be sending a separate cleanup commit.

> +#define CONFIG_SYS_FSMC_BASE			0xD1800000
>
>>   #elif defined(CONFIG_SPEAR300)

<snip>

>> diff --git a/include/configs/spear-common.h
>> b/include/configs/spear-common.h index 516b78e..c37305f 100644
>> --- a/include/configs/spear-common.h
>> +++ b/include/configs/spear-common.h
>> @@ -90,7 +90,7 @@
>>   /* NAND FLASH Configuration */
>>   #define CONFIG_MTD_DEVICE
>>   #define CONFIG_MTD_PARTITIONS
>> -#define CONFIG_NAND_SPEAR			1
>> +#define CONFIG_NAND_FSMC
>>   #define CONFIG_SYS_MAX_NAND_DEVICE		1
>>   #define CONFIG_MTD_NAND_VERIFY_WRITE
>
> I suggest that you remove this last define. Most likely it was added for
> debugging purpose only. It slows down the speed and it brakes UBI support.
>

Ok.

>> diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h
>> index bd5d111..5bdd874 100644
>> --- a/include/configs/spear3xx.h
>> +++ b/include/configs/spear3xx.h
>> @@ -117,6 +117,10 @@
>>
>>   #endif
>>
>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
>> +
>>   #if defined(CONFIG_SPEAR300)
>>   #define CONFIG_SYS_NAND_BASE			0x80000000
>>
>> diff --git a/include/configs/spear6xx.h b/include/configs/spear6xx.h
>> index 8de7ebd..1e06c72 100644
>> --- a/include/configs/spear6xx.h
>> +++ b/include/configs/spear6xx.h
>> @@ -38,6 +38,9 @@
>>   #define CONFIG_PL01x_PORTS			{ (void
> *)CONFIG_SYS_SERIAL0, \
>>   						(void
> *)CONFIG_SYS_SERIAL1 }
>>
>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
>
> You also need the following define for this to work with the latest NAND
> subsystem:
>
> #define CONFIG_MTD_ECC_SOFT
>
> Not sure about SPEAr3xx. Most likely this needs it as well.
>

Ok. I'll verify the image after the making the changes you suggested and 
then get back.

Thanks
Amit Virdi

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Scott Wood]

On 02/27/2012 04:02 AM, Stefan Roese wrote:
>> diff --git a/include/configs/spear-common.h
>> b/include/configs/spear-common.h index 516b78e..c37305f 100644
>> --- a/include/configs/spear-common.h
>> +++ b/include/configs/spear-common.h
>> @@ -90,7 +90,7 @@
>>  /* NAND FLASH Configuration */
>>  #define CONFIG_MTD_DEVICE
>>  #define CONFIG_MTD_PARTITIONS
>> -#define CONFIG_NAND_SPEAR			1
>> +#define CONFIG_NAND_FSMC
>>  #define CONFIG_SYS_MAX_NAND_DEVICE		1
>>  #define CONFIG_MTD_NAND_VERIFY_WRITE
> 
> I suggest that you remove this last define. Most likely it was added for 
> debugging purpose only. It slows down the speed and it brakes UBI support.

What is the problem with UBI?

>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
> 
> You also need the following define for this to work with the latest NAND 
> subsystem:
> 
> #define CONFIG_MTD_ECC_SOFT
> 
> Not sure about SPEAr3xx. Most likely this needs it as well.

This is going to be reverted for now -- I meant to leave it out of the
last patchset because of the need to update all boards (which the patch
did not do).

-Scott

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

Hello Stefan,

[...]
>> +/* NAND flash configuration */
>> +#define CONFIG_SYS_FSMC_NAND_SP
>> +#define CONFIG_SYS_FSMC_NAND_8BIT
>
> You also need the following define for this to work with the latest NAND
> subsystem:
>
> #define CONFIG_MTD_ECC_SOFT
>
> Not sure about SPEAr3xx. Most likely this needs it as well.
>

Could you please explain the rationale for defining this flag?

Thanks
Amit Virdi

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

Hello Scott,

On 2/28/2012 4:02 AM, Scott Wood wrote:
> On 02/27/2012 04:02 AM, Stefan Roese wrote:
[...]
>>
>> You also need the following define for this to work with the latest NAND
>> subsystem:
>>
>> #define CONFIG_MTD_ECC_SOFT
>>
>> Not sure about SPEAr3xx. Most likely this needs it as well.
>
> This is going to be reverted for now -- I meant to leave it out of the
> last patchset because of the need to update all boards (which the patch
> did not do).
>

Sorry, I didn't get you. Could you please elaborate?

Thanks
Amit Virdi

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Scott Wood]

On 02/29/2012 04:11 AM, Amit Virdi wrote:
> Hello Scott,
> 
> On 2/28/2012 4:02 AM, Scott Wood wrote:
>> On 02/27/2012 04:02 AM, Stefan Roese wrote:
> [...]
>>>
>>> You also need the following define for this to work with the latest NAND
>>> subsystem:
>>>
>>> #define CONFIG_MTD_ECC_SOFT
>>>
>>> Not sure about SPEAr3xx. Most likely this needs it as well.
>>
>> This is going to be reverted for now -- I meant to leave it out of the
>> last patchset because of the need to update all boards (which the patch
>> did not do).
>>
> 
> Sorry, I didn't get you. Could you please elaborate?

It was intended to reduce U-Boot code size in cases where soft ECC is
not needed.  However, the patch that introduced it did not update the
boards to select it when necessary, so it is being reverted for now.

It can be resubmitted with proper documentation and board config file
updates.

-Scott

[U-Boot] [PATCH 1/3] mtd/NAND: Add FSMC driver support

[Scott Wood]

On 02/27/2012 03:38 AM, Amit Virdi wrote:
> +		/*
> +		 * This is a temporary erase check. A newly erased page read
> +		 * would result in an ecc error because the oob data is also
> +		 * erased to FF and the calculated ecc for an FF data is not
> +		 * FF..FF.
> +		 * This is a workaround to skip performing correction in case
> +		 * data is FF..FF
> +		 */
> +		for (k = 0; k < eccsize; k++) {
> +			if (*(p + k) != 0xff)
> +				break;
> +		}

Shouldn't this apply over the whole page (including the ECC bytes
themselves), not just the ECC chunk?  The data could legitimately be all
0xff except for one bit, and that bit could have flipped...

Will a freshly erased page show up as having correctable errors, or
uncorrectable?  If the latter, just hold off on declaring the page as an
ECC fail until you've read the whole thing, and then if you're about to
mark it failed, check wheter it's freshly erased.

> +	nand->options = 0;
> +#if defined(CONFIG_SYS_FSMC_NAND_16BIT)
> +	nand->options |= NAND_BUSWIDTH_16;
> +#endif

No on-flash BBT?

> +	/* Detect NAND chips */
> +	if (nand_scan_ident(mtd, 1, NULL))
> +		return -ENXIO;

You should #define CONFIG_SYS_NAND_SELF_INIT if you want to call this
yourself (see the documentation for what else you need to do).

> +/*
> + * There are 13 bytes of ecc for every 512 byte block and it has to be read
> + * consecutively and immediately after the 512 byte data block for hardware to
> + * generate the error bit offsets
> + * Managing the ecc bytes in the following way is easier. This way is similar to
> + * oobfree structure maintained already in u-boot nand driver
> + */
> +#define MAX_ECCPLACE_ENTRIES	32

No FSMC namespace... is/will this file included by anything but the FSMC
 driver?

-Scott

[U-Boot] [PATCH 1/3] mtd/NAND: Add FSMC driver support

[Amit Virdi]

Hello Scott,

On 3/2/2012 2:57 AM, Scott Wood wrote:
> On 02/27/2012 03:38 AM, Amit Virdi wrote:
>> +		/*
>> +		 * This is a temporary erase check. A newly erased page read
>> +		 * would result in an ecc error because the oob data is also
>> +		 * erased to FF and the calculated ecc for an FF data is not
>> +		 * FF..FF.
>> +		 * This is a workaround to skip performing correction in case
>> +		 * data is FF..FF
>> +		 */
>> +		for (k = 0; k<  eccsize; k++) {
>> +			if (*(p + k) != 0xff)
>> +				break;
>> +		}
>
> Shouldn't this apply over the whole page (including the ECC bytes
> themselves), not just the ECC chunk?  The data could legitimately be all
> 0xff except for one bit, and that bit could have flipped...

So you're saying that such a page is wrongly interpreted as "erased 
page" and ecc check is skipped! Yes, you are very right. This check 
should be applied to the whole page and not only the chunk.

>
> Will a freshly erased page show up as having correctable errors, or

A newly erased page contains 0xff in data as well as spare area. So most 
likely, it shows up as having uncorrectable errors.

> uncorrectable?  If the latter, just hold off on declaring the page as an
> ECC fail until you've read the whole thing, and then if you're about to
> mark it failed, check wheter it's freshly erased.

So do you mean to say:

			stat = chip->ecc.correct(mtd, p, &ecc_code[i],
					&ecc_calc[i]);
			if (stat < 0) {
			  if (data as well as the oob is 0xff)
				do nothing;
                           else
				mtd->ecc_stats.failed++;
			} else
				mtd->ecc_stats.corrected += stat;


Can we have a scenario like this:
A page has been erased - so it contains 0xff in data area as well as 
spare area. Somehow, a bit in either or say both areas are flipped, then 
the SW will not be able to distinguish if it's and erased page or a page 
with data and uncorrectable errors. How do we take care of such scenarios?

>
>> +	nand->options = 0;
>> +#if defined(CONFIG_SYS_FSMC_NAND_16BIT)
>> +	nand->options |= NAND_BUSWIDTH_16;
>> +#endif
>
> No on-flash BBT?
>

No, not implemented yet.

>> +	/* Detect NAND chips */
>> +	if (nand_scan_ident(mtd, 1, NULL))
>> +		return -ENXIO;
>
> You should #define CONFIG_SYS_NAND_SELF_INIT if you want to call this
> yourself (see the documentation for what else you need to do).
>

Ok. I see this is the new philosophy that is encouraged even for the 
existing drivers.

>> +/*
>> + * There are 13 bytes of ecc for every 512 byte block and it has to be read
>> + * consecutively and immediately after the 512 byte data block for hardware to
>> + * generate the error bit offsets
>> + * Managing the ecc bytes in the following way is easier. This way is similar to
>> + * oobfree structure maintained already in u-boot nand driver
>> + */
>> +#define MAX_ECCPLACE_ENTRIES	32
>
> No FSMC namespace... is/will this file included by anything but the FSMC
>   driver?
>

Sorry, it didn't get you here. This file (fsmc_nand.h) shall be included 
by board files.

Thanks
Amit Virdi

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Amit Virdi]

Hi Scott,

>>>>
>>>> You also need the following define for this to work with the latest NAND
>>>> subsystem:
>>>>
>>>> #define CONFIG_MTD_ECC_SOFT
>>>>
>>>> Not sure about SPEAr3xx. Most likely this needs it as well.
>>>
>>> This is going to be reverted for now -- I meant to leave it out of the
>>> last patchset because of the need to update all boards (which the patch
>>> did not do).
>>>
>>
>> Sorry, I didn't get you. Could you please elaborate?
>
> It was intended to reduce U-Boot code size in cases where soft ECC is
> not needed.  However, the patch that introduced it did not update the
> boards to select it when necessary, so it is being reverted for now.
>
> It can be resubmitted with proper documentation and board config file
> updates.
>

Ok. So I need not to define "CONFIG_MTD_ECC_SOFT" for now.

Thanks
Amit Virdi

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Stefan Roese]

Hi Scott,

On Wednesday 29 February 2012 19:03:20 Scott Wood wrote:
> >>> You also need the following define for this to work with the latest
> >>> NAND subsystem:
> >>> 
> >>> #define CONFIG_MTD_ECC_SOFT
> >>> 
> >>> Not sure about SPEAr3xx. Most likely this needs it as well.
> >> 
> >> This is going to be reverted for now -- I meant to leave it out of the
> >> last patchset because of the need to update all boards (which the patch
> >> did not do).
> > 
> > Sorry, I didn't get you. Could you please elaborate?
> 
> It was intended to reduce U-Boot code size in cases where soft ECC is
> not needed.  However, the patch that introduced it did not update the
> boards to select it when necessary, so it is being reverted for now.

Okay, but it does not "hurt" to add this define to platforms using soft ECC 
right now, when such NAND driver related config options are updated? Or would 
you advise to remove it for now?

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

[U-Boot] [PATCH 2/3] SPEAr: Configure FSMC driver for NAND interface

[Scott Wood]

On 03/02/2012 07:44 AM, Stefan Roese wrote:
> Hi Scott,
> 
> On Wednesday 29 February 2012 19:03:20 Scott Wood wrote:
>>>>> You also need the following define for this to work with the latest
>>>>> NAND subsystem:
>>>>>
>>>>> #define CONFIG_MTD_ECC_SOFT
>>>>>
>>>>> Not sure about SPEAr3xx. Most likely this needs it as well.
>>>>
>>>> This is going to be reverted for now -- I meant to leave it out of the
>>>> last patchset because of the need to update all boards (which the patch
>>>> did not do).
>>>
>>> Sorry, I didn't get you. Could you please elaborate?
>>
>> It was intended to reduce U-Boot code size in cases where soft ECC is
>> not needed.  However, the patch that introduced it did not update the
>> boards to select it when necessary, so it is being reverted for now.
> 
> Okay, but it does not "hurt" to add this define to platforms using soft ECC 
> right now, when such NAND driver related config options are updated? Or would 
> you advise to remove it for now?

It doesn't hurt, other than that you're defining an undocumented option,
assuming someone actually does put the effort into identifying the
affected boards soon (or implementing some sort of transition
mechanism), so that it doesn't just sit around forever without being used.

-Scott

[U-Boot] [PATCH 1/3] mtd/NAND: Add FSMC driver support

[Scott Wood]

On 03/02/2012 05:16 AM, Amit Virdi wrote:
> Hello Scott,
> 
> On 3/2/2012 2:57 AM, Scott Wood wrote:
>> On 02/27/2012 03:38 AM, Amit Virdi wrote:
>>> +        /*
>>> +         * This is a temporary erase check. A newly erased page read
>>> +         * would result in an ecc error because the oob data is also
>>> +         * erased to FF and the calculated ecc for an FF data is not
>>> +         * FF..FF.
>>> +         * This is a workaround to skip performing correction in case
>>> +         * data is FF..FF
>>> +         */
>>> +        for (k = 0; k<  eccsize; k++) {
>>> +            if (*(p + k) != 0xff)
>>> +                break;
>>> +        }
>>
>> Shouldn't this apply over the whole page (including the ECC bytes
>> themselves), not just the ECC chunk?  The data could legitimately be all
>> 0xff except for one bit, and that bit could have flipped...
> 
> So you're saying that such a page is wrongly interpreted as "erased
> page" and ecc check is skipped! Yes, you are very right. This check
> should be applied to the whole page and not only the chunk.
> 
>>
>> Will a freshly erased page show up as having correctable errors, or
> 
> A newly erased page contains 0xff in data as well as spare area. So most
> likely, it shows up as having uncorrectable errors.

This is what I saw as well on FSL IFC with 4-bit ECC (see is_blank() and
related code in drivers/mtd/nand/fsl_ifc_nand.c for another driver doing
this sort of check).

>> uncorrectable?  If the latter, just hold off on declaring the page as an
>> ECC fail until you've read the whole thing, and then if you're about to
>> mark it failed, check wheter it's freshly erased.
> 
> So do you mean to say:
> 
>             stat = chip->ecc.correct(mtd, p, &ecc_code[i],
>                     &ecc_calc[i]);
>             if (stat < 0) {
>               if (data as well as the oob is 0xff)
>                 do nothing;
>                           else
>                 mtd->ecc_stats.failed++;
>             } else
>                 mtd->ecc_stats.corrected += stat;

Yes, provided you check the entire page and not just the ecc subpage
after seeing an error.

> Can we have a scenario like this:
> A page has been erased - so it contains 0xff in data area as well as
> spare area. Somehow, a bit in either or say both areas are flipped, then
> the SW will not be able to distinguish if it's and erased page or a page
> with data and uncorrectable errors. How do we take care of such scenarios?

It will show up as an uncorrectable error (which is no big loss, since
the page has no data, but may result in a block being marked bad before
its time).

If you really want, you could keep a count and allow a certain number of
bits to be flipped (up to the ECC threshold) and still consider it blank
(and report the number of corrections as usual).

>>> +    /* Detect NAND chips */
>>> +    if (nand_scan_ident(mtd, 1, NULL))
>>> +        return -ENXIO;
>>
>> You should #define CONFIG_SYS_NAND_SELF_INIT if you want to call this
>> yourself (see the documentation for what else you need to do).
>>
> 
> Ok. I see this is the new philosophy that is encouraged even for the
> existing drivers.

Yes, it would be nice if the old way could go away at some point.

>>> +/*
>>> + * There are 13 bytes of ecc for every 512 byte block and it has to
>>> be read
>>> + * consecutively and immediately after the 512 byte data block for
>>> hardware to
>>> + * generate the error bit offsets
>>> + * Managing the ecc bytes in the following way is easier. This way
>>> is similar to
>>> + * oobfree structure maintained already in u-boot nand driver
>>> + */
>>> +#define MAX_ECCPLACE_ENTRIES    32
>>
>> No FSMC namespace... is/will this file included by anything but the FSMC
>>   driver?
>>
> 
> Sorry, it didn't get you here. This file (fsmc_nand.h) shall be included
> by board files.

I'd either namespace it or (preferably) keep this in the driver file
itself, but as long as nothing outside FSMC-specific code includes this
(directly or indirectly -- e.g. the board config file must not include
it) I won't complain too loudly.

-Scott