diff mbox

[U-Boot,v1,7/8] am33xx_spl_bch: simple SPL nand loader for AM33XX

Message ID 1351637263-17464-8-git-send-email-ilya.yanok@cogentembedded.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Ilya Yanok Oct. 30, 2012, 10:47 p.m. UTC 
AM33XX with BCH8 can't work with nand_spl_simple correctly
because custom read_page implementation is required for proper
syndrome generation.

This simple driver mostly duplicates nand_spl_simple but has
nand_read_page changed to suit our needs.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/am335x_spl_bch.c |  238 +++++++++++++++++++++++++++++++++++++
 2 files changed, 239 insertions(+)
 create mode 100644 drivers/mtd/nand/am335x_spl_bch.c

Comments

Tom Rini Oct. 31, 2012, 12:03 a.m. UTC | #1 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/30/12 15:47, Ilya Yanok wrote:
> AM33XX with BCH8 can't work with nand_spl_simple correctly because
> custom read_page implementation is required for proper syndrome
> generation.
> 
> This simple driver mostly duplicates nand_spl_simple but has 
> nand_read_page changed to suit our needs.
[snip]

> +	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) { 
> +		this->ecc.hwctl(&mtd, NAND_ECC_READ); +		nand_command(block,
> page, data_pos, NAND_CMD_RNDOUT); + +		this->read_buf(&mtd, p,
> eccsize); + +		nand_command(block, page, oob_pos,
> NAND_CMD_RNDOUT); + +		this->read_buf(&mtd, oob, eccbytes); +
> this->ecc.calculate(&mtd, p, &ecc_calc[i]); + +		data_pos +=
> eccsize; +		oob_pos += eccbytes; +		oob += eccbytes; +	}

This is where the function differs.  If we can't merge things
together, I'd like to see about putting just this function into
nand_spl_simple.c under CONFIG_SYS_NAND_HW_BCH8 since if I follow
what's going on, and I need to play with the code to confirm I do,
it's a generic change related to how much more we're reading back out
now.  But good catch finding what was going wrong with nand_spl_simple
here.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQkGrJAAoJENk4IS6UOR1WnqMP/3i/zWcvB7iZhjl5HvrlrYuQ
+6ijXtQzWCROKP/gERTQjKlVOj09jEGvHY5Ac5xpiMqtxvCAxDyz88fu5CWJgtlT
gZJqq7WPaR21FHJzS5GinL7oQBNMY9/m1wNVCVHq971Lq4VxgfYPXdPMytiMPxOy
ExoB034z6KkaFR7FdRFLMZeT9ri292jFlmZuNpyKjjEYigI4+WH4FvlkFxEpL18w
F+bXWzp4Krd8ndgM3OjvzJWmPZxU3M1YjHKOn631W8T2Q7bMe6yOH9PEYKiqQuTG
wniwcmxB6n2vT04enyuUPCfyNA9OPJv7w+pGWdrYXjU22CHUbNfywJZC7sqmYeht
uUhvvdF8jtJFHpK1RScF69Gy2xcmtIa7PPMmLkv3JbtnLgw91NyeM+tuzty2/MIA
icsbE1PwxvLGSMlvJJhag82oKb4kYZ1I5SSmd5aJkyt21CbXiBKf6M+Vsx8TmyCh
03XTuN7iuMpm4jlEed4bv64yLZ7suJxu60XtKAH5yX5S6L8PI+SAEpzknLdqCf7j
A2z8YyLZEaxa27oxOmY4/G8KTpZrPNkei4jL9VOd7wkZ/j9TRUZ6rzrIHVuwWlqS
oVJt/qHq+zznQFZF3iI3kPUtiU1FpiPycPzSevH+V3gZNFdNF1Ze9+VqEyW2QV60
nVRZjXS9VSKeR43dXBgg
=giFN
-----END PGP SIGNATURE-----
Ilya Yanok Nov. 3, 2012, 4:21 p.m. UTC | #2 
Hi Tom,

On Wed, Oct 31, 2012 at 1:03 AM, Tom Rini <trini@ti.com> wrote:
>
> > +     for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> > +             this->ecc.hwctl(&mtd, NAND_ECC_READ); +
> nand_command(block,
> > page, data_pos, NAND_CMD_RNDOUT); + +         this->read_buf(&mtd, p,
> > eccsize); + +         nand_command(block, page, oob_pos,
> > NAND_CMD_RNDOUT); + +         this->read_buf(&mtd, oob, eccbytes); +
> > this->ecc.calculate(&mtd, p, &ecc_calc[i]); + +               data_pos +=
> > eccsize; +            oob_pos += eccbytes; +          oob += eccbytes; +
>      }
>
> This is where the function differs.  If we can't merge things
> together, I'd like to see about putting just this function into
> nand_spl_simple.c under CONFIG_SYS_NAND_HW_BCH8 since if I follow
> what's going on, and I need to play with the code to confirm I do,
> it's a generic change related to how much more we're reading back out
>

Not exactly. This change is rather GPMC-specific: we have to read data
block then it's ecc code to get the syndrome. And even with GPMC in another
configuration we will need another reading order...
I'm not sure if we can do this in some generic way...

Regards, Ilya.
Tom Rini Nov. 5, 2012, 4:57 p.m. UTC | #3 
On Sat, Nov 03, 2012 at 05:21:46PM +0100, Ilya Yanok wrote:
> Hi Tom,
> 
> On Wed, Oct 31, 2012 at 1:03 AM, Tom Rini <trini@ti.com> wrote:
> >
> > > +     for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> > > +             this->ecc.hwctl(&mtd, NAND_ECC_READ); +
> > nand_command(block,
> > > page, data_pos, NAND_CMD_RNDOUT); + +         this->read_buf(&mtd, p,
> > > eccsize); + +         nand_command(block, page, oob_pos,
> > > NAND_CMD_RNDOUT); + +         this->read_buf(&mtd, oob, eccbytes); +
> > > this->ecc.calculate(&mtd, p, &ecc_calc[i]); + +               data_pos +=
> > > eccsize; +            oob_pos += eccbytes; +          oob += eccbytes; +
> >      }
> >
> > This is where the function differs.  If we can't merge things
> > together, I'd like to see about putting just this function into
> > nand_spl_simple.c under CONFIG_SYS_NAND_HW_BCH8 since if I follow
> > what's going on, and I need to play with the code to confirm I do,
> > it's a generic change related to how much more we're reading back out
> 
> Not exactly. This change is rather GPMC-specific: we have to read data
> block then it's ecc code to get the syndrome. And even with GPMC in another
> configuration we will need another reading order...
> I'm not sure if we can do this in some generic way...

OK, lets just leave that part alone for now.  Maybe we need to allow for
some abstraction or __weak in the SPL read case, but we can revisit this
a little later on once we're able to see how say BCH16 behaves too.
diff mbox

Patch

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index beb99ca..5322f3a 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -30,6 +30,7 @@  ifdef CONFIG_SPL_BUILD
 ifdef CONFIG_SPL_NAND_SIMPLE
 COBJS-y += nand_spl_simple.o
 endif
+COBJS-$(CONFIG_SPL_NAND_AM33XX_BCH) += am335x_spl_bch.o
 ifdef CONFIG_SPL_NAND_LOAD
 COBJS-y	+= nand_spl_load.o
 endif
diff --git a/drivers/mtd/nand/am335x_spl_bch.c b/drivers/mtd/nand/am335x_spl_bch.c
new file mode 100644
index 0000000..b84528b
--- /dev/null
+++ b/drivers/mtd/nand/am335x_spl_bch.c
@@ -0,0 +1,238 @@ 
+/*
+ * (C) Copyright 2012
+ * Konstantin Kozhevnikov, Cogent Embedded
+ *
+ * based on nand_spl_simple code
+ *
+ * (C) Copyright 2006-2008
+ * Stefan Roese, DENX Software Engineering, sr@denx.de.
+ *
+ * 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.
+ */
+
+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <linux/mtd/nand_ecc.h>
+
+static int nand_ecc_pos[] = CONFIG_SYS_NAND_ECCPOS;
+static nand_info_t mtd;
+static struct nand_chip nand_chip;
+
+#define ECCSTEPS	(CONFIG_SYS_NAND_PAGE_SIZE / \
+					CONFIG_SYS_NAND_ECCSIZE)
+#define ECCTOTAL	(ECCSTEPS * CONFIG_SYS_NAND_ECCBYTES)
+
+
+/*
+ * NAND command for large page NAND devices (2k)
+ */
+static int nand_command(int block, int page, uint32_t offs,
+	u8 cmd)
+{
+	struct nand_chip *this = mtd.priv;
+	int page_addr = page + block * CONFIG_SYS_NAND_PAGE_COUNT;
+	void (*hwctrl)(struct mtd_info *mtd, int cmd,
+			unsigned int ctrl) = this->cmd_ctrl;
+
+	while (!this->dev_ready(&mtd))
+		;
+
+	/* Emulate NAND_CMD_READOOB */
+	if (cmd == NAND_CMD_READOOB) {
+		offs += CONFIG_SYS_NAND_PAGE_SIZE;
+		cmd = NAND_CMD_READ0;
+	}
+
+	/* Begin command latch cycle */
+	hwctrl(&mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
+
+	if (cmd == NAND_CMD_RESET) {
+		hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+		while (!this->dev_ready(&mtd))
+			;
+		return 0;
+	}
+
+	/* Shift the offset from byte addressing to word addressing. */
+	if (this->options & NAND_BUSWIDTH_16)
+		offs >>= 1;
+
+	/* Set ALE and clear CLE to start address cycle */
+	/* Column address */
+	hwctrl(&mtd, offs & 0xff,
+		       NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
+	hwctrl(&mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
+	/* Row address */
+	hwctrl(&mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
+	hwctrl(&mtd, ((page_addr >> 8) & 0xff),
+		       NAND_CTRL_ALE); /* A[27:20] */
+#ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
+	/* One more address cycle for devices > 128MiB */
+	hwctrl(&mtd, (page_addr >> 16) & 0x0f,
+		       NAND_CTRL_ALE); /* A[31:28] */
+#endif
+	hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+
+	if (cmd == NAND_CMD_READ0) {
+		/* Latch in address */
+		hwctrl(&mtd, NAND_CMD_READSTART,
+			   NAND_CTRL_CLE | NAND_CTRL_CHANGE);
+		hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+
+		/*
+		 * Wait a while for the data to be ready
+		 */
+		while (!this->dev_ready(&mtd))
+			;
+	} else if (cmd == NAND_CMD_RNDOUT) {
+		hwctrl(&mtd, NAND_CMD_RNDOUTSTART, NAND_CTRL_CLE |
+					NAND_CTRL_CHANGE);
+		hwctrl(&mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+	}
+
+	return 0;
+}
+
+static int nand_is_bad_block(int block)
+{
+	struct nand_chip *this = mtd.priv;
+
+	nand_command(block, 0, CONFIG_SYS_NAND_BAD_BLOCK_POS,
+		NAND_CMD_READOOB);
+
+	/*
+	 * 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)
+			return 1;
+	} else {
+		if (readb(this->IO_ADDR_R) != 0xff)
+			return 1;
+	}
+
+	return 0;
+}
+
+static int nand_read_page(int block, int page, void *dst)
+{
+	struct nand_chip *this = mtd.priv;
+	u_char ecc_calc[ECCTOTAL];
+	u_char ecc_code[ECCTOTAL];
+	u_char oob_data[CONFIG_SYS_NAND_OOBSIZE];
+	int i;
+	int eccsize = CONFIG_SYS_NAND_ECCSIZE;
+	int eccbytes = CONFIG_SYS_NAND_ECCBYTES;
+	int eccsteps = ECCSTEPS;
+	uint8_t *p = dst;
+	uint32_t data_pos = 0;
+	uint8_t *oob = &oob_data[0] + nand_ecc_pos[0];
+	uint32_t oob_pos = eccsize * eccsteps + nand_ecc_pos[0];
+
+	nand_command(block, page, 0, NAND_CMD_READ0);
+
+	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		this->ecc.hwctl(&mtd, NAND_ECC_READ);
+		nand_command(block, page, data_pos, NAND_CMD_RNDOUT);
+
+		this->read_buf(&mtd, p, eccsize);
+
+		nand_command(block, page, oob_pos, NAND_CMD_RNDOUT);
+
+		this->read_buf(&mtd, oob, eccbytes);
+		this->ecc.calculate(&mtd, p, &ecc_calc[i]);
+
+		data_pos += eccsize;
+		oob_pos += eccbytes;
+		oob += eccbytes;
+	}
+
+	/* Pick the ECC bytes out of the oob data */
+	for (i = 0; i < ECCTOTAL; i++)
+		ecc_code[i] = oob_data[nand_ecc_pos[i]];
+
+	eccsteps = ECCSTEPS;
+	p = dst;
+
+	for (i = 0 ; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+		/* No chance to do something with the possible error message
+		 * from correct_data(). We just hope that all possible errors
+		 * are corrected by this routine.
+		 */
+		this->ecc.correct(&mtd, p, &ecc_code[i], &ecc_calc[i]);
+	}
+
+	return 0;
+}
+
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+{
+	unsigned int block, lastblock;
+	unsigned int page;
+
+	/*
+	 * offs has to be aligned to a page address!
+	 */
+	block = offs / CONFIG_SYS_NAND_BLOCK_SIZE;
+	lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE;
+	page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) / CONFIG_SYS_NAND_PAGE_SIZE;
+
+	while (block <= lastblock) {
+		if (!nand_is_bad_block(block)) {
+			/*
+			 * Skip bad blocks
+			 */
+			while (page < CONFIG_SYS_NAND_PAGE_COUNT) {
+				nand_read_page(block, page, dst);
+				dst += CONFIG_SYS_NAND_PAGE_SIZE;
+				page++;
+			}
+
+			page = 0;
+		} else {
+			lastblock++;
+		}
+
+		block++;
+	}
+
+	return 0;
+}
+
+/* nand_init() - initialize data to make nand usable by SPL */
+void nand_init(void)
+{
+	/*
+	 * Init board specific nand support
+	 */
+	mtd.priv = &nand_chip;
+	nand_chip.IO_ADDR_R = nand_chip.IO_ADDR_W =
+		(void  __iomem *)CONFIG_SYS_NAND_BASE;
+	board_nand_init(&nand_chip);
+
+	if (nand_chip.select_chip)
+		nand_chip.select_chip(&mtd, 0);
+
+	/* NAND chip may require reset after power-on */
+	nand_command(0, 0, 0, NAND_CMD_RESET);
+}
+
+/* Unselect after operation */
+void nand_deselect(void)
+{
+	if (nand_chip.select_chip)
+		nand_chip.select_chip(&mtd, -1);
+}