[PATCH 1/3] mtd/nand : use elbc_fcm_ctrl->oob to set FPAR_MS bit of FPAR

[PATCH 1/3] mtd/nand : use elbc_fcm_ctrl->oob to set FPAR_MS bit of FPAR

[shuo.liu]

From: Liu Shuo <b35362@freescale.com>

On both of large-page chip and small-page chip, we always should use
'elbc_fcm_ctrl->oob' to set the FPAR_LP_MS/FPAR_SP_MS bit of FPAR, don't
use a overflowed 'column' to set it.

Signed-off-by: Liu Shuo <b35362@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/mtd/nand/fsl_elbc_nand.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index cc08a11..6fce7da 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -414,9 +414,17 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		         page_addr, column);
 
 		elbc_fcm_ctrl->column = column;
-		elbc_fcm_ctrl->oob = 0;
 		elbc_fcm_ctrl->use_mdr = 1;
 
+		if (column >= mtd->writesize) {
+			/* OOB area */
+			column -= mtd->writesize;
+			elbc_fcm_ctrl->oob = 1;
+		} else {
+			WARN_ON(column != 0);
+			elbc_fcm_ctrl->oob = 0;
+		}
+
 		fcr = (NAND_CMD_STATUS   << FCR_CMD1_SHIFT) |
 		      (NAND_CMD_SEQIN    << FCR_CMD2_SHIFT) |
 		      (NAND_CMD_PAGEPROG << FCR_CMD3_SHIFT);
@@ -441,16 +449,12 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 			         (FIR_OP_CW1 << FIR_OP6_SHIFT) |
 			         (FIR_OP_RS  << FIR_OP7_SHIFT));
 
-			if (column >= mtd->writesize) {
+			if (elbc_fcm_ctrl->oob)
 				/* OOB area --> READOOB */
-				column -= mtd->writesize;
 				fcr |= NAND_CMD_READOOB << FCR_CMD0_SHIFT;
-				elbc_fcm_ctrl->oob = 1;
-			} else {
-				WARN_ON(column != 0);
+			else
 				/* First 256 bytes --> READ0 */
 				fcr |= NAND_CMD_READ0 << FCR_CMD0_SHIFT;
-			}
 		}
 
 		out_be32(&lbc->fcr, fcr);
-- 
1.7.1

[PATCH 2/3] mtd/nand : set correct length to FBCR for a non-full-page write

[shuo.liu]

From: Liu Shuo <b35362@freescale.com>

When we do a non-full-page write, the length be set to FBCR should
not be 'elbc_fcm_ctrl->index', it should be 'elbc_fcm_ctrl->index -
elbc_fcm_ctrl->column'.

Signed-off-by: Liu Shuo <b35362@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/mtd/nand/fsl_elbc_nand.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 6fce7da..d634c5f 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -474,7 +474,8 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		 */
 		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
 		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
-			out_be32(&lbc->fbcr, elbc_fcm_ctrl->index);
+			out_be32(&lbc->fbcr,
+				elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
 		else
 			out_be32(&lbc->fbcr, 0);
 
-- 
1.7.1

[PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[shuo.liu]

From: Liu Shuo <shuo.liu@freescale.com>

Freescale FCM controller has a 2K size limitation of buffer RAM. In order
to support the Nand flash chip whose page size is larger than 2K bytes,
we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
them to a large buffer.

Signed-off-by: Liu Shuo <shuo.liu@freescale.com>
---
v3:
    -remove page_size of struct fsl_elbc_mtd.
    -do a oob write by NAND_CMD_RNDIN. 

 drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
 1 files changed, 218 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index d634c5f..a92411a 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -55,7 +55,6 @@ struct fsl_elbc_mtd {
 	struct device *dev;
 	int bank;               /* Chip select bank number           */
 	u8 __iomem *vbase;      /* Chip select base virtual address  */
-	int page_size;          /* NAND page size (0=512, 1=2048)    */
 	unsigned int fmr;       /* FCM Flash Mode Register value     */
 };
 
@@ -75,6 +74,8 @@ struct fsl_elbc_fcm_ctrl {
 	unsigned int use_mdr;    /* Non zero if the MDR is to be set      */
 	unsigned int oob;        /* Non zero if operating on OOB data     */
 	unsigned int counter;	 /* counter for the initializations	  */
+
+	char *buffer;            /* just be used when pagesize > 2048     */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -150,6 +151,42 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 };
 
 /*=================================*/
+static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
+	void *src, *dst;
+	int len = (oob ? 64 : 2048);
+
+	if (oob)
+		dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
+	else
+		dst = elbc_fcm_ctrl->buffer + subpage * 2048;
+
+	src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
+	memcpy_fromio(dst, src, len);
+}
+
+static void buffer_to_io(struct mtd_info *mtd, int subpage, int oob)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
+	void *src, *dst;
+	int len = (oob ? 64 : 2048);
+
+	if (oob)
+		src = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
+	else
+		src = elbc_fcm_ctrl->buffer + subpage * 2048;
+
+	dst = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
+	memcpy_toio(dst, src, len);
+
+	/* See the in_8() in fsl_elbc_write_buf() */
+	in_8(elbc_fcm_ctrl->addr + (oob ? 2111 : 2047));
+}
 
 /*
  * Set up the FCM hardware block and page address fields, and the fcm
@@ -166,7 +203,7 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 
 	elbc_fcm_ctrl->page = page_addr;
 
-	if (priv->page_size) {
+	if (mtd->writesize >= 2048) {
 		/*
 		 * large page size chip : FPAR[PI] save the lowest 6 bits,
 		 *                        FBAR[BLK] save the other bits.
@@ -193,7 +230,7 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 
 	/* for OOB data point to the second half of the buffer */
 	if (oob)
-		elbc_fcm_ctrl->index += priv->page_size ? 2048 : 512;
+		elbc_fcm_ctrl->index += mtd->writesize;
 
 	dev_vdbg(priv->dev, "set_addr: bank=%d, "
 			    "elbc_fcm_ctrl->addr=0x%p (0x%p), "
@@ -272,13 +309,14 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 	return 0;
 }
 
-static void fsl_elbc_do_read(struct nand_chip *chip, int oob)
+static void fsl_elbc_do_read(struct mtd_info *mtd, int oob)
 {
+	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
 	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 
-	if (priv->page_size) {
+	if (mtd->writesize >= 2048) {
 		out_be32(&lbc->fir,
 		         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
 		         (FIR_OP_CA  << FIR_OP1_SHIFT) |
@@ -311,6 +349,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+	int i, n;
 
 	elbc_fcm_ctrl->use_mdr = 0;
 
@@ -337,8 +376,29 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		elbc_fcm_ctrl->read_bytes = mtd->writesize + mtd->oobsize;
 		elbc_fcm_ctrl->index += column;
 
-		fsl_elbc_do_read(chip, 0);
+		fsl_elbc_do_read(mtd, 0);
 		fsl_elbc_run_command(mtd);
+
+		if (mtd->writesize <= 2048)
+			return;
+
+		/* Continue to read the rest bytes if writesize > 2048 */
+		io_to_buffer(mtd, 0, 0);
+		io_to_buffer(mtd, 0, 1);
+
+		out_be32(&lbc->fir, FIR_OP_RB << FIR_OP1_SHIFT);
+
+		n = mtd->writesize / 2048;
+		for (i = 1; i < n; i++) {
+			/*
+			 * Maybe there are some reasons of FCM hardware timing,
+			 * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB.
+			 */
+			fsl_elbc_run_command(mtd);
+			io_to_buffer(mtd, i, 0);
+			io_to_buffer(mtd, i, 1);
+		}
+
 		return;
 
 	/* READOOB reads only the OOB because no ECC is performed. */
@@ -347,13 +407,37 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		         "fsl_elbc_cmdfunc: NAND_CMD_READOOB, page_addr:"
 			 " 0x%x, column: 0x%x.\n", page_addr, column);
 
-		out_be32(&lbc->fbcr, mtd->oobsize - column);
-		set_addr(mtd, column, page_addr, 1);
+		if (mtd->writesize <= 2048) {
+			out_be32(&lbc->fbcr, mtd->oobsize - column);
+			set_addr(mtd, column, page_addr, 1);
+		} else {
+			out_be32(&lbc->fbcr, 64);
+			set_addr(mtd, 0, page_addr, 1);
+			elbc_fcm_ctrl->index += column;
+		}
 
 		elbc_fcm_ctrl->read_bytes = mtd->writesize + mtd->oobsize;
 
-		fsl_elbc_do_read(chip, 1);
+		fsl_elbc_do_read(mtd, 1);
 		fsl_elbc_run_command(mtd);
+
+		if (mtd->writesize <= 2048)
+			return;
+
+		if (column < 64)
+			io_to_buffer(mtd, 0, 1);
+
+		out_be32(&lbc->fbcr, 2112);
+		out_be32(&lbc->fir, FIR_OP_RB << FIR_OP1_SHIFT);
+		out_be32(&lbc->fpar, in_be32(&lbc->fpar) & ~FPAR_LP_MS);
+
+		n = mtd->writesize / 2048;
+		for (i = 1; i < n; i++) {
+			fsl_elbc_run_command(mtd);
+			if (column < (64 * (i + 1)))
+				io_to_buffer(mtd, i, 1);
+		}
+
 		return;
 
 	/* READID must read all 5 possible bytes while CEB is active */
@@ -429,7 +513,17 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		      (NAND_CMD_SEQIN    << FCR_CMD2_SHIFT) |
 		      (NAND_CMD_PAGEPROG << FCR_CMD3_SHIFT);
 
-		if (priv->page_size) {
+		if (mtd->writesize > 2048) {
+			/* writesize > 2048 */
+			out_be32(&lbc->fir,
+				 (FIR_OP_CM2 << FIR_OP0_SHIFT) |
+				 (FIR_OP_CA  << FIR_OP1_SHIFT) |
+				 (FIR_OP_PA  << FIR_OP2_SHIFT) |
+				 (FIR_OP_WB  << FIR_OP3_SHIFT));
+
+			if (elbc_fcm_ctrl->oob)
+				fcr |= NAND_CMD_RNDIN << FCR_CMD0_SHIFT;
+		} else if (mtd->writesize == 2048) {
 			out_be32(&lbc->fir,
 			         (FIR_OP_CM2 << FIR_OP0_SHIFT) |
 			         (FIR_OP_CA  << FIR_OP1_SHIFT) |
@@ -464,6 +558,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 	/* PAGEPROG reuses all of the setup from SEQIN and adds the length */
 	case NAND_CMD_PAGEPROG: {
+		int pos;
 		dev_vdbg(priv->dev,
 		         "fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG "
 			 "writing %d bytes.\n", elbc_fcm_ctrl->index);
@@ -473,13 +568,72 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		 * write so the HW generates the ECC.
 		 */
 		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
-		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
-			out_be32(&lbc->fbcr,
-				elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
-		else
+		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
+			if (elbc_fcm_ctrl->oob && mtd->writesize > 2048) {
+				out_be32(&lbc->fbcr, 64);
+			} else {
+				out_be32(&lbc->fbcr, elbc_fcm_ctrl->index
+						- elbc_fcm_ctrl->column);
+			}
+		} else {
 			out_be32(&lbc->fbcr, 0);
+		}
+
+		if (mtd->writesize > 2048) {
+			if (!elbc_fcm_ctrl->oob)
+				buffer_to_io(mtd, 0, 0);
+			buffer_to_io(mtd, 0, 1);
+		}
 
 		fsl_elbc_run_command(mtd);
+
+		if (mtd->writesize <= 2048)
+			return;
+
+		n = mtd->writesize / 2048;
+
+		if (elbc_fcm_ctrl->oob) {
+			pos = 2048;
+			out_be32(&lbc->fir,
+				(FIR_OP_CM0 << FIR_OP0_SHIFT) |
+				(FIR_OP_UA  << FIR_OP1_SHIFT) |
+				(FIR_OP_UA  << FIR_OP2_SHIFT) |
+				(FIR_OP_WB  << FIR_OP3_SHIFT));
+
+			for (i = 1; i < n; i++) {
+				pos += 2112;
+				elbc_fcm_ctrl->mdr = pos;
+				elbc_fcm_ctrl->use_mdr = 1;
+				if (i == n - 1) {
+					out_be32(&lbc->fir,
+						(FIR_OP_CM0 << FIR_OP1_SHIFT) |
+						(FIR_OP_UA  << FIR_OP2_SHIFT) |
+						(FIR_OP_UA  << FIR_OP3_SHIFT) |
+						(FIR_OP_WB  << FIR_OP4_SHIFT) |
+						(FIR_OP_CM3 << FIR_OP5_SHIFT) |
+						(FIR_OP_CW1 << FIR_OP6_SHIFT) |
+						(FIR_OP_RS  << FIR_OP7_SHIFT));
+				}
+				buffer_to_io(mtd, i, 1);
+				fsl_elbc_run_command(mtd);
+			}
+		} else {
+			out_be32(&lbc->fir, FIR_OP_WB << FIR_OP1_SHIFT);
+			for (i = 1; i < n; i++) {
+				if (i == n - 1) {
+					elbc_fcm_ctrl->use_mdr = 1;
+					out_be32(&lbc->fir,
+						(FIR_OP_WB  << FIR_OP1_SHIFT) |
+						(FIR_OP_CM3 << FIR_OP2_SHIFT) |
+						(FIR_OP_CW1 << FIR_OP3_SHIFT) |
+						(FIR_OP_RS  << FIR_OP4_SHIFT));
+				}
+				buffer_to_io(mtd, i, 0);
+				buffer_to_io(mtd, i, 1);
+				fsl_elbc_run_command(mtd);
+			}
+		}
+
 		return;
 	}
 
@@ -500,6 +654,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		 * write-protected, even when it is not.
 		 */
 		setbits8(elbc_fcm_ctrl->addr, NAND_STATUS_WP);
+		elbc_fcm_ctrl->buffer[0] = in_8(elbc_fcm_ctrl->addr);
 		return;
 
 	/* RESET without waiting for the ready line */
@@ -548,7 +703,14 @@ static void fsl_elbc_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 		len = bufsize - elbc_fcm_ctrl->index;
 	}
 
-	memcpy_toio(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], buf, len);
+	if (mtd->writesize > 2048) {
+		memcpy(&elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index],
+				buf, len);
+	} else {
+		memcpy_toio(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index],
+				buf, len);
+	}
+
 	/*
 	 * This is workaround for the weird elbc hangs during nand write,
 	 * Scott Wood says: "...perhaps difference in how long it takes a
@@ -572,8 +734,13 @@ static u8 fsl_elbc_read_byte(struct mtd_info *mtd)
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
 
 	/* If there are still bytes in the FCM, then use the next byte. */
-	if (elbc_fcm_ctrl->index < elbc_fcm_ctrl->read_bytes)
-		return in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index++]);
+	if (elbc_fcm_ctrl->index < elbc_fcm_ctrl->read_bytes) {
+		int index = elbc_fcm_ctrl->index++;
+		if (mtd->writesize > 2048)
+			return elbc_fcm_ctrl->buffer[index];
+		else
+			return in_8(&elbc_fcm_ctrl->addr[index]);
+	}
 
 	dev_err(priv->dev, "read_byte beyond end of buffer\n");
 	return ERR_BYTE;
@@ -594,7 +761,13 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 
 	avail = min((unsigned int)len,
 			elbc_fcm_ctrl->read_bytes - elbc_fcm_ctrl->index);
-	memcpy_fromio(buf, &elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], avail);
+	if (mtd->writesize > 2048) {
+		memcpy(buf, &elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index],
+				avail);
+	} else {
+		memcpy_fromio(buf, &elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index],
+				avail);
+	}
 	elbc_fcm_ctrl->index += avail;
 
 	if (len > avail)
@@ -630,10 +803,17 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < len; i++)
-		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
-				!= buf[i])
-			break;
+	if (mtd->writesize > 2048) {
+		for (i = 0; i < len; i++)
+			if (elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index + i]
+					!= buf[i])
+				break;
+	} else {
+		for (i = 0; i < len; i++)
+			if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
+					!= buf[i])
+				break;
+	}
 
 	elbc_fcm_ctrl->index += len;
 	return i == len && elbc_fcm_ctrl->status == LTESR_CC ? 0 : -EIO;
@@ -714,10 +894,9 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 
 	/* adjust Option Register and ECC to match Flash page size */
 	if (mtd->writesize == 512) {
-		priv->page_size = 0;
 		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
-	} else if (mtd->writesize == 2048) {
-		priv->page_size = 1;
+	} else if (mtd->writesize >= 2048 && mtd->writesize <= 16 * 1024) {
+
 		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
 		/* adjust ecc setup if needed */
 		if ((in_be32(&lbc->bank[priv->bank].br) & BR_DECC) ==
@@ -891,6 +1070,19 @@ static int __devinit fsl_elbc_nand_probe(struct platform_device *pdev)
 			goto err;
 		}
 		elbc_fcm_ctrl->counter++;
+		/*
+		 * Freescale FCM controller has a 2K size limitation of buffer
+		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
+		 * of chip is greater than 2048.
+		 * We malloc a large enough buffer (maximum page size is 16K).
+		 */
+		elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024, GFP_KERNEL);
+		if (!elbc_fcm_ctrl->buffer) {
+			dev_err(dev, "failed to allocate memory\n");
+			mutex_unlock(&fsl_elbc_nand_mutex);
+			ret = -ENOMEM;
+			goto err;
+		}
 
 		spin_lock_init(&elbc_fcm_ctrl->controller.lock);
 		init_waitqueue_head(&elbc_fcm_ctrl->controller.wq);
@@ -960,6 +1152,7 @@ static int fsl_elbc_nand_remove(struct platform_device *pdev)
 	elbc_fcm_ctrl->counter--;
 	if (!elbc_fcm_ctrl->counter) {
 		fsl_lbc_ctrl_dev->nand = NULL;
+		kfree(elbc_fcm_ctrl->buffer);
 		kfree(elbc_fcm_ctrl);
 	}
 	mutex_unlock(&fsl_elbc_nand_mutex);
-- 
1.7.1

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

On Sun, 2011-12-04 at 12:31 +0800, shuo.liu@freescale.com wrote:
> +		/*
> +		 * Freescale FCM controller has a 2K size limitation of buffer
> +		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> +		 * of chip is greater than 2048.
> +		 * We malloc a large enough buffer (maximum page size is 16K).
> +		 */
> +		elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024, GFP_KERNEL);
> +		if (!elbc_fcm_ctrl->buffer) {
> +			dev_err(dev, "failed to allocate memory\n");
> +			mutex_unlock(&fsl_elbc_nand_mutex);
> +			ret = -ENOMEM;
> +			goto err;
> +		}

Sorry for returning to this again and agian - I do not have time to dig
suggest you the right solutions on the one hand, you do not provide me a
good answer on the other hand (or I forgot?).

16KiB pages do not even exist I believe. And you kmalloc 33KiB or RAM
although in most cases you need only 5KiB. I think this is wrong - what
is the very strong reason of wasting RAM you have?

Why you cannot allocate exactly the required amount of RAM after
'nand_scan_ident()' finishes and you know the page size?

Artem.

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/05/2011 12:47 AM, Artem Bityutskiy wrote:
> On Sun, 2011-12-04 at 12:31 +0800, shuo.liu@freescale.com wrote:
>> +		/*
>> +		 * Freescale FCM controller has a 2K size limitation of buffer
>> +		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
>> +		 * of chip is greater than 2048.
>> +		 * We malloc a large enough buffer (maximum page size is 16K).
>> +		 */
>> +		elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024, GFP_KERNEL);
>> +		if (!elbc_fcm_ctrl->buffer) {
>> +			dev_err(dev, "failed to allocate memory\n");
>> +			mutex_unlock(&fsl_elbc_nand_mutex);
>> +			ret = -ENOMEM;
>> +			goto err;
>> +		}
> 
> Sorry for returning to this again and agian - I do not have time to dig
> suggest you the right solutions on the one hand, you do not provide me a
> good answer on the other hand (or I forgot?).
> 
> 16KiB pages do not even exist I believe.

Googling turns up some hints of it, but nothing concrete such as a
datasheet.  We can assume 8K max for now and adjust it later, as the
need becomes clear.

> And you kmalloc 33KiB or RAM

17KiB, or 9KiB if we forget about 16K-page NAND.

> although in most cases you need only 5KiB. I think this is wrong -
> what is the very strong reason of wasting RAM you have?
> 
> Why you cannot allocate exactly the required amount of RAM after
> 'nand_scan_ident()' finishes and you know the page size?

Because this is a controller resource, shared by multiple NAND chips
that may be different page sizes (even if not, it's adding another point
of synchronization required between initialization of different chips).
 I don't think it's worth the gymnastics to save a few KiB.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

On Mon, 2011-12-05 at 13:46 -0600, Scott Wood wrote:
> Because this is a controller resource, shared by multiple NAND chips
> that may be different page sizes (even if not, it's adding another point
> of synchronization required between initialization of different chips).
>  I don't think it's worth the gymnastics to save a few KiB.

OK, I see.

Artem.

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

On Sun, 2011-12-04 at 12:31 +0800, shuo.liu@freescale.com wrote:
> From: Liu Shuo <shuo.liu@freescale.com>
> 
> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> to support the Nand flash chip whose page size is larger than 2K bytes,
> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> them to a large buffer.
> 
> Signed-off-by: Liu Shuo <shuo.liu@freescale.com>

This patch does not apply on top of my l2-mtd-2.6.git tree. Could you
please send a patch that applies cleanly?

Thanks!

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
> From: Liu Shuo <shuo.liu@freescale.com>
> 
> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> to support the Nand flash chip whose page size is larger than 2K bytes,
> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> them to a large buffer.
> 
> Signed-off-by: Liu Shuo <shuo.liu@freescale.com>
> ---
> v3:
>     -remove page_size of struct fsl_elbc_mtd.
>     -do a oob write by NAND_CMD_RNDIN. 
> 
>  drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
>  1 files changed, 218 insertions(+), 25 deletions(-)

What is the plan for bad block marker migration?

> @@ -473,13 +568,72 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
>  		 * write so the HW generates the ECC.
>  		 */
>  		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
> -		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
> -			out_be32(&lbc->fbcr,
> -				elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
> -		else
> +		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
> +			if (elbc_fcm_ctrl->oob && mtd->writesize > 2048) {
> +				out_be32(&lbc->fbcr, 64);
> +			} else {
> +				out_be32(&lbc->fbcr, elbc_fcm_ctrl->index
> +						- elbc_fcm_ctrl->column);
> +			}

We need to limit ourselves to the regions that have actually been
written to in the buffer.  fbcr needs to be set separately for first and
last subpages, with intermediate subpages having 0, 64, or 2112 as
appropriate.  Subpages that are entirely before column or entirely after
column + index should be skipped.

> +		} else {
> +			out_be32(&lbc->fir, FIR_OP_WB << FIR_OP1_SHIFT);
> +			for (i = 1; i < n; i++) {
> +				if (i == n - 1) {
> +					elbc_fcm_ctrl->use_mdr = 1;
> +					out_be32(&lbc->fir,
> +						(FIR_OP_WB  << FIR_OP1_SHIFT) |
> +						(FIR_OP_CM3 << FIR_OP2_SHIFT) |
> +						(FIR_OP_CW1 << FIR_OP3_SHIFT) |
> +						(FIR_OP_RS  << FIR_OP4_SHIFT));

Please explicitly show the (FIR_OP_NOP << FIR_OP0_SHIFT) compenent.

> +	} else if (mtd->writesize >= 2048 && mtd->writesize <= 16 * 1024) {
> +
>  		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);

Don't insert a blank line here.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月07日 08:09, Scott Wood 写道:
> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>> From: Liu Shuo<shuo.liu@freescale.com>
>>
>> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
>> to support the Nand flash chip whose page size is larger than 2K bytes,
>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>> them to a large buffer.
>>
>> Signed-off-by: Liu Shuo<shuo.liu@freescale.com>
>> ---
>> v3:
>>      -remove page_size of struct fsl_elbc_mtd.
>>      -do a oob write by NAND_CMD_RNDIN.
>>
>>   drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
>>   1 files changed, 218 insertions(+), 25 deletions(-)
> What is the plan for bad block marker migration?
This patch has been ported to uboot now, I think we can make a special 
uboot image for bad
block marker migration when first use the chip.

>> @@ -473,13 +568,72 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
>>   		 * write so the HW generates the ECC.
>>   		 */
>>   		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
>> -		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
>> -			out_be32(&lbc->fbcr,
>> -				elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
>> -		else
>> +		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
>> +			if (elbc_fcm_ctrl->oob&&  mtd->writesize>  2048) {
>> +				out_be32(&lbc->fbcr, 64);
>> +			} else {
>> +				out_be32(&lbc->fbcr, elbc_fcm_ctrl->index
>> +						- elbc_fcm_ctrl->column);
>> +			}
> We need to limit ourselves to the regions that have actually been
> written to in the buffer.  fbcr needs to be set separately for first and
> last subpages, with intermediate subpages having 0, 64, or 2112 as
> appropriate.  Subpages that are entirely before column or entirely after
> column + index should be skipped.

I have considered this case, but I don't think it is useful.
     1.There isn't a 'length' parameter in driver interface, although we 
can get it from 'index - column'.
     2.To see nand_do_write_oob() in nand_base.c, it fill '0xff' to 
entire oob area first and write the user data by nand_fill_oob(), then 
call ecc.write_oob (default is nand_write_oob_std()). 'column' is 
mtd->writesize and 'length' of write_buf() is mtd->oobsize. So I don't 
think we need to deal with it there.

>> +		} else {
>> +			out_be32(&lbc->fir, FIR_OP_WB<<  FIR_OP1_SHIFT);
>> +			for (i = 1; i<  n; i++) {
>> +				if (i == n - 1) {
>> +					elbc_fcm_ctrl->use_mdr = 1;
>> +					out_be32(&lbc->fir,
>> +						(FIR_OP_WB<<  FIR_OP1_SHIFT) |
>> +						(FIR_OP_CM3<<  FIR_OP2_SHIFT) |
>> +						(FIR_OP_CW1<<  FIR_OP3_SHIFT) |
>> +						(FIR_OP_RS<<  FIR_OP4_SHIFT));
> Please explicitly show the (FIR_OP_NOP<<  FIR_OP0_SHIFT) compenent.
>
>> +	} else if (mtd->writesize>= 2048&&  mtd->writesize<= 16 * 1024) {
>> +
>>   		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
> Don't insert a blank line here.
>
Ok.

-LiuShuo
> -Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/06/2011 09:55 PM, LiuShuo wrote:
> 于 2011年12月07日 08:09, Scott Wood 写道:
>> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>>> From: Liu Shuo<shuo.liu@freescale.com>
>>>
>>> Freescale FCM controller has a 2K size limitation of buffer RAM. In
>>> order
>>> to support the Nand flash chip whose page size is larger than 2K bytes,
>>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>>> them to a large buffer.
>>>
>>> Signed-off-by: Liu Shuo<shuo.liu@freescale.com>
>>> ---
>>> v3:
>>>      -remove page_size of struct fsl_elbc_mtd.
>>>      -do a oob write by NAND_CMD_RNDIN.
>>>
>>>   drivers/mtd/nand/fsl_elbc_nand.c |  243
>>> ++++++++++++++++++++++++++++++++++----
>>>   1 files changed, 218 insertions(+), 25 deletions(-)
>> What is the plan for bad block marker migration?
> This patch has been ported to uboot now, I think we can make a special
> uboot image for bad
> block marker migration when first use the chip.

It should not be a special image, and there should be some way to mark
that the migration has happened.  Even if we do the migration in U-Boot,
Linux could check for the marker and if absent, disallow access and tell
the user to run the migration tool.

>>> @@ -473,13 +568,72 @@ static void fsl_elbc_cmdfunc(struct mtd_info
>>> *mtd, unsigned int command,
>>>            * write so the HW generates the ECC.
>>>            */
>>>           if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
>>> -            elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
>>> -            out_be32(&lbc->fbcr,
>>> -                elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
>>> -        else
>>> +            elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
>>> +            if (elbc_fcm_ctrl->oob&&  mtd->writesize>  2048) {
>>> +                out_be32(&lbc->fbcr, 64);
>>> +            } else {
>>> +                out_be32(&lbc->fbcr, elbc_fcm_ctrl->index
>>> +                        - elbc_fcm_ctrl->column);
>>> +            }
>> We need to limit ourselves to the regions that have actually been
>> written to in the buffer.  fbcr needs to be set separately for first and
>> last subpages, with intermediate subpages having 0, 64, or 2112 as
>> appropriate.  Subpages that are entirely before column or entirely after
>> column + index should be skipped.
> 
> I have considered this case, but I don't think it is useful.
>     1.There isn't a 'length' parameter in driver interface, although we
> can get it from 'index - column'.

Right.  column is start, and index is end + 1.  We have the bounds of
what has been written.

>     2.To see nand_do_write_oob() in nand_base.c, it fill '0xff' to
> entire oob area first and write the user data by nand_fill_oob(), then
> call ecc.write_oob (default is nand_write_oob_std()).

Do we really want to assume that that's what it will always do?

And if we do want to make such assumptions, we could rip out all usage
of index/column here, and just handle "oob" and "full page" cases.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月08日 03:11, Scott Wood 写道:
> On 12/06/2011 09:55 PM, LiuShuo wrote:
>> 于 2011年12月07日 08:09, Scott Wood 写道:
>>> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>>>> From: Liu Shuo<shuo.liu@freescale.com>
>>>>
>>>> Freescale FCM controller has a 2K size limitation of buffer RAM. In
>>>> order
>>>> to support the Nand flash chip whose page size is larger than 2K bytes,
>>>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>>>> them to a large buffer.
>>>>
>>>> Signed-off-by: Liu Shuo<shuo.liu@freescale.com>
>>>> ---
>>>> v3:
>>>>       -remove page_size of struct fsl_elbc_mtd.
>>>>       -do a oob write by NAND_CMD_RNDIN.
>>>>
>>>>    drivers/mtd/nand/fsl_elbc_nand.c |  243
>>>> ++++++++++++++++++++++++++++++++++----
>>>>    1 files changed, 218 insertions(+), 25 deletions(-)
>>> What is the plan for bad block marker migration?
>> This patch has been ported to uboot now, I think we can make a special
>> uboot image for bad
>> block marker migration when first use the chip.
> It should not be a special image, and there should be some way to mark
> that the migration has happened.  Even if we do the migration in U-Boot,
> Linux could check for the marker and if absent, disallow access and tell
> the user to run the migration tool.
>
>>>> @@ -473,13 +568,72 @@ static void fsl_elbc_cmdfunc(struct mtd_info
>>>> *mtd, unsigned int command,
>>>>             * write so the HW generates the ECC.
>>>>             */
>>>>            if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
>>>> -            elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
>>>> -            out_be32(&lbc->fbcr,
>>>> -                elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
>>>> -        else
>>>> +            elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
>>>> +            if (elbc_fcm_ctrl->oob&&   mtd->writesize>   2048) {
>>>> +                out_be32(&lbc->fbcr, 64);
>>>> +            } else {
>>>> +                out_be32(&lbc->fbcr, elbc_fcm_ctrl->index
>>>> +                        - elbc_fcm_ctrl->column);
>>>> +            }
>>> We need to limit ourselves to the regions that have actually been
>>> written to in the buffer.  fbcr needs to be set separately for first and
>>> last subpages, with intermediate subpages having 0, 64, or 2112 as
>>> appropriate.  Subpages that are entirely before column or entirely after
>>> column + index should be skipped.
>> I have considered this case, but I don't think it is useful.
>>      1.There isn't a 'length' parameter in driver interface, although we
>> can get it from 'index - column'.
> Right.  column is start, and index is end + 1.  We have the bounds of
> what has been written.
>
>>      2.To see nand_do_write_oob() in nand_base.c, it fill '0xff' to
>> entire oob area first and write the user data by nand_fill_oob(), then
>> call ecc.write_oob (default is nand_write_oob_std()).
> Do we really want to assume that that's what it will always do?
>
> And if we do want to make such assumptions, we could rip out all usage
> of index/column here, and just handle "oob" and "full page" cases.
The function nand_do_write_ops() in nandbase.c is a Nand internal interface.
It always is called when application write to nand flash. (e.g. dd)
In this function, partial page write is dealt with by filling '0xff' to 
buffer before data copy.
(nand_do_write_oob() is similar)
So I don't think we need to do it in our controller driver again, it 
should be a job of upper layer.


I found that 'column' for NAND_CMD_SEQIN is always 0 or writesize except 
for oob write with
  NAND_ECC_HW_SYNDROME, but it's not useful case for our controller.

-LiuShuo
> -Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/08/2011 04:44 AM, LiuShuo wrote:
> 于 2011年12月08日 03:11, Scott Wood 写道:
>> And if we do want to make such assumptions, we could rip out all usage
>> of index/column here, and just handle "oob" and "full page" cases.
> The function nand_do_write_ops() in nandbase.c is a Nand internal
> interface.
> It always is called when application write to nand flash. (e.g. dd)
> In this function, partial page write is dealt with by filling '0xff' to
> buffer before data copy.
> (nand_do_write_oob() is similar)
> So I don't think we need to do it in our controller driver again, it
> should be a job of upper layer.

If this is to be considered part of the interface contract, then perhaps
do a WARN_ON() if we see an unexpected index/column?  And after that,
only consider full-page or full-oob possibilities.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

On Tue, 2011-12-06 at 18:09 -0600, Scott Wood wrote:
> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
> > From: Liu Shuo <shuo.liu@freescale.com>
> > 
> > Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> > to support the Nand flash chip whose page size is larger than 2K bytes,
> > we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> > them to a large buffer.
> > 
> > Signed-off-by: Liu Shuo <shuo.liu@freescale.com>
> > ---
> > v3:
> >     -remove page_size of struct fsl_elbc_mtd.
> >     -do a oob write by NAND_CMD_RNDIN. 
> > 
> >  drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
> >  1 files changed, 218 insertions(+), 25 deletions(-)
> 
> What is the plan for bad block marker migration?

Why it should be migrated? I thought that you support 2KiB pages, and
this adds 4 and 8 KiB pages support, which you never supported before.
What is the migration you guys are talking about?

Artem.

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/12/2011 03:09 PM, Artem Bityutskiy wrote:
> On Tue, 2011-12-06 at 18:09 -0600, Scott Wood wrote:
>> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>>> From: Liu Shuo <shuo.liu@freescale.com>
>>>
>>> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
>>> to support the Nand flash chip whose page size is larger than 2K bytes,
>>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>>> them to a large buffer.
>>>
>>> Signed-off-by: Liu Shuo <shuo.liu@freescale.com>
>>> ---
>>> v3:
>>>     -remove page_size of struct fsl_elbc_mtd.
>>>     -do a oob write by NAND_CMD_RNDIN. 
>>>
>>>  drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
>>>  1 files changed, 218 insertions(+), 25 deletions(-)
>>
>> What is the plan for bad block marker migration?
> 
> Why it should be migrated? I thought that you support 2KiB pages, and
> this adds 4 and 8 KiB pages support, which you never supported before.
> What is the migration you guys are talking about?

NAND chips come from the factory with bad blocks marked at a certain
offset into each page.  This offset is normally in the OOB area, but
since we change the layout from "4k data, 128 byte oob" to "2k data, 64
byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
first use we need to migrate the markers so that they are still in the oob.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
> NAND chips come from the factory with bad blocks marked at a certain
> offset into each page.  This offset is normally in the OOB area, but
> since we change the layout from "4k data, 128 byte oob" to "2k data, 64
> byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
> first use we need to migrate the markers so that they are still in the oob.

Ah, I see, thanks. Are you planning to implement in-kernel migration or
use a user-space tool?

Artem.

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>> NAND chips come from the factory with bad blocks marked at a certain
>> offset into each page.  This offset is normally in the OOB area, but
>> since we change the layout from "4k data, 128 byte oob" to "2k data, 64
>> byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
>> first use we need to migrate the markers so that they are still in the oob.
> 
> Ah, I see, thanks. Are you planning to implement in-kernel migration or
> use a user-space tool?

That's the kind of answer I was hoping to get from Shuo. :-)

Most likely is a firmware-based tool, but I'd like there to be some way
for the tool to mark that this has happened, so that the Linux driver
can refuse to do non-raw accesses to a chip that isn't marked as having
been migrated (or at least yell loudly in the log).

Speaking of raw accesses, these are currently broken in the eLBC
driver... we need some way for the generic layer to tell us what kind of
access it is before the transaction starts, not once it wants to read
out the buffer (unless we add more hacks to delay the start of a read
transaction until first buffer access...).  We'd be better off with a
high-level "read page/write page" function that does the whole thing
(not just buffer access, but command issuance as well).

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月13日 05:30, Scott Wood 写道:
> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>> NAND chips come from the factory with bad blocks marked at a certain
>>> offset into each page.  This offset is normally in the OOB area, but
>>> since we change the layout from "4k data, 128 byte oob" to "2k data, 64
>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
>>> first use we need to migrate the markers so that they are still in the oob.
>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>> use a user-space tool?
> That's the kind of answer I was hoping to get from Shuo. :-)
OK, I try to do this. Wait for a couple of days.

-LiuShuo
> Most likely is a firmware-based tool, but I'd like there to be some way
> for the tool to mark that this has happened, so that the Linux driver
> can refuse to do non-raw accesses to a chip that isn't marked as having
> been migrated (or at least yell loudly in the log).
>
> Speaking of raw accesses, these are currently broken in the eLBC
> driver... we need some way for the generic layer to tell us what kind of
> access it is before the transaction starts, not once it wants to read
> out the buffer (unless we add more hacks to delay the start of a read
> transaction until first buffer access...).  We'd be better off with a
> high-level "read page/write page" function that does the whole thing
> (not just buffer access, but command issuance as well).
>
> -Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月13日 10:46, LiuShuo 写道:
> 于 2011年12月13日 05:30, Scott Wood 写道:
>> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>>> NAND chips come from the factory with bad blocks marked at a certain
>>>> offset into each page.  This offset is normally in the OOB area, but
>>>> since we change the layout from "4k data, 128 byte oob" to "2k 
>>>> data, 64
>>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the 
>>>> oob.  On
>>>> first use we need to migrate the markers so that they are still in 
>>>> the oob.
>>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>>> use a user-space tool?
>> That's the kind of answer I was hoping to get from Shuo. :-)
> OK, I try to do this. Wait for a couple of days.
>
> -LiuShuo
I found it's too complex to do the migration in Linux driver.

Maybe we can add a uboot command (e.g. nand bbmigrate) to do it, once is 
enough.
And let user ensure it been completed before linux use the Nand flash chip.

Even if we don't do the migration, the bad block also can be marked as bad
by wearing. So, do we really need to take much time to implement it ?
(code looks too complex.)

-LiuShuo

>> Most likely is a firmware-based tool, but I'd like there to be some way
>> for the tool to mark that this has happened, so that the Linux driver
>> can refuse to do non-raw accesses to a chip that isn't marked as having
>> been migrated (or at least yell loudly in the log).
>>
>> Speaking of raw accesses, these are currently broken in the eLBC
>> driver... we need some way for the generic layer to tell us what kind of
>> access it is before the transaction starts, not once it wants to read
>> out the buffer (unless we add more hacks to delay the start of a read
>> transaction until first buffer access...).  We'd be better off with a
>> high-level "read page/write page" function that does the whole thing
>> (not just buffer access, but command issuance as well).
>>
>> -Scott
>

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/14/2011 02:41 AM, LiuShuo wrote:
> 于 2011年12月13日 10:46, LiuShuo 写道:
>> 于 2011年12月13日 05:30, Scott Wood 写道:
>>> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>>>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>>>> NAND chips come from the factory with bad blocks marked at a certain
>>>>> offset into each page.  This offset is normally in the OOB area, but
>>>>> since we change the layout from "4k data, 128 byte oob" to "2k
>>>>> data, 64
>>>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the
>>>>> oob.  On
>>>>> first use we need to migrate the markers so that they are still in
>>>>> the oob.
>>>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>>>> use a user-space tool?
>>> That's the kind of answer I was hoping to get from Shuo. :-)
>> OK, I try to do this. Wait for a couple of days.
>>
>> -LiuShuo
> I found it's too complex to do the migration in Linux driver.
> 
> Maybe we can add a uboot command (e.g. nand bbmigrate) to do it, once is
> enough.

Any reason not to do it automatically on the first U-Boot bad block
scan, if the flash isn't marked as already migrated?

Further discussion on the details of how to do it in U-Boot should move
to the U-Boot list.

> And let user ensure it been completed before linux use the Nand flash chip.

I don't want to trust the user here.  It's too easy to skip it, and
things will appear to work, but have subtle problems.

> Even if we don't do the migration, the bad block also can be marked as bad
> by wearing. So, do we really need to take much time to implement it ?
> (code looks too complex.)

It is not acceptable to ignore factory bad block markers just because
some methods of using the flash may eventually detect an error (possibly
after data is lost -- no guarantee that the badness is ECC-correctable)
and mark the block bad again.

If you don't feel up to the task, I can look at it, but won't have time
until January.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Li Yang]

On Thu, Dec 15, 2011 at 4:15 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/14/2011 02:41 AM, LiuShuo wrote:
>> 于 2011年12月13日 10:46, LiuShuo 写道:
>>> 于 2011年12月13日 05:30, Scott Wood 写道:
>>>> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>>>>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>>>>> NAND chips come from the factory with bad blocks marked at a certain
>>>>>> offset into each page.  This offset is normally in the OOB area, but
>>>>>> since we change the layout from "4k data, 128 byte oob" to "2k
>>>>>> data, 64
>>>>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the
>>>>>> oob.  On
>>>>>> first use we need to migrate the markers so that they are still in
>>>>>> the oob.
>>>>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>>>>> use a user-space tool?
>>>> That's the kind of answer I was hoping to get from Shuo. :-)
>>> OK, I try to do this. Wait for a couple of days.
>>>
>>> -LiuShuo
>> I found it's too complex to do the migration in Linux driver.
>>
>> Maybe we can add a uboot command (e.g. nand bbmigrate) to do it, once is
>> enough.
>
> Any reason not to do it automatically on the first U-Boot bad block
> scan, if the flash isn't marked as already migrated?
>
> Further discussion on the details of how to do it in U-Boot should move
> to the U-Boot list.

The limitation of the proposed bad block marker migration is that you
need to make sure the migration is done and only done once.  If it is
done more than once, the factory bad block marker is totally messed
up.  It requires a complex mechanism to automatically guarantee the
migration is only done once, and it still won't be 100% safe.

I would suggest we use a much easier compromise that we form the BBT
base on the factory bad block marker on first use of the flash, and
after that the factory bad block marker is dropped.  We just relies on
the BBT for information about bad blocks.  Although by doing so we
can't regenerate the BBT again,  as there is mirror for the BBT I
don't think we have too much risk.

- Leo

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/14/2011 10:59 PM, Li Yang wrote:
> The limitation of the proposed bad block marker migration is that you
> need to make sure the migration is done and only done once.  If it is
> done more than once, the factory bad block marker is totally messed
> up.  It requires a complex mechanism to automatically guarantee the
> migration is only done once, and it still won't be 100% safe.
> 
> I would suggest we use a much easier compromise that we form the BBT
> base on the factory bad block marker on first use of the flash, and
> after that the factory bad block marker is dropped.  We just relies on
> the BBT for information about bad blocks.  Although by doing so we
> can't regenerate the BBT again,  as there is mirror for the BBT I
> don't think we have too much risk.

I have corrupted the BBT too often during development (e.g. a bug makes
all accesses fail, so the upper layers decide to mark everything bad) to
be comfortable with this.

Elsewhere in the thread I suggested a way to let the marker be in either
the bbt or in a dedicated block, depending on whether it's a development
situation where the BBT needs to be erasable.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月15日 04:15, Scott Wood 写道:
> On 12/14/2011 02:41 AM, LiuShuo wrote:
>> 于 2011年12月13日 10:46, LiuShuo 写道:
>>> 于 2011年12月13日 05:30, Scott Wood 写道:
>>>> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>>>>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>>>>> NAND chips come from the factory with bad blocks marked at a certain
>>>>>> offset into each page.  This offset is normally in the OOB area, but
>>>>>> since we change the layout from "4k data, 128 byte oob" to "2k
>>>>>> data, 64
>>>>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the
>>>>>> oob.  On
>>>>>> first use we need to migrate the markers so that they are still in
>>>>>> the oob.
>>>>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>>>>> use a user-space tool?
>>>> That's the kind of answer I was hoping to get from Shuo. :-)
>>> OK, I try to do this. Wait for a couple of days.
>>>
>>> -LiuShuo
>> I found it's too complex to do the migration in Linux driver.
>>
>> Maybe we can add a uboot command (e.g. nand bbmigrate) to do it, once is
>> enough.
> Any reason not to do it automatically on the first U-Boot bad block
> scan, if the flash isn't marked as already migrated?
>
> Further discussion on the details of how to do it in U-Boot should move
> to the U-Boot list.
>
>> And let user ensure it been completed before linux use the Nand flash chip.
> I don't want to trust the user here.  It's too easy to skip it, and
> things will appear to work, but have subtle problems.
>
>> Even if we don't do the migration, the bad block also can be marked as bad
>> by wearing. So, do we really need to take much time to implement it ?
>> (code looks too complex.)
> It is not acceptable to ignore factory bad block markers just because
> some methods of using the flash may eventually detect an error (possibly
> after data is lost -- no guarantee that the badness is ECC-correctable)
> and mark the block bad again.
>
> If you don't feel up to the task, I can look at it, but won't have time
> until January.
hi Scott,
It's really hard to me and I have much other works to do now. Thanks for 
your help.

hi Artem,
Could this patch be applied now and we make a independent patch for  bad 
block information
migration later?

-LiuShuo

> -Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/15/2011 08:44 PM, LiuShuo wrote:
> hi Artem,
> Could this patch be applied now and we make a independent patch for  bad
> block information
> migration later?

This patch is not safe to use without migration.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Li Yang]

On Sat, Dec 17, 2011 at 1:59 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/15/2011 08:44 PM, LiuShuo wrote:
>> hi Artem,
>> Could this patch be applied now and we make a independent patch for  bad
>> block information
>> migration later?
>
> This patch is not safe to use without migration.

Hi Scott,

We agree it's not entirely safe without migrating the bad block flag.
But let's consider two sides of the situation.

Firstly, it's only unsafe when there is a need to re-built the Bad
Block Table from scratch(old BBT broken).  But currently there is no
easy way to do that(re-build BBT on demand), which means it's not a
common problem that we can easily address now.

Secondly, even if the previous said problem happens(BBT broken).  We
can still recover all the data if we overrule the bad block flag.
Only the card is not so good to be used again, however, it can be used
if we take the risk of losing data from errors that ECC can't
notice(low possibility too).

Finally, I don't think this is a blocker issue but a better to have enhancement.

- Leo

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/19/2011 05:05 AM, Li Yang wrote:
> On Sat, Dec 17, 2011 at 1:59 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 12/15/2011 08:44 PM, LiuShuo wrote:
>>> hi Artem,
>>> Could this patch be applied now and we make a independent patch for  bad
>>> block information
>>> migration later?
>>
>> This patch is not safe to use without migration.
> 
> Hi Scott,
> 
> We agree it's not entirely safe without migrating the bad block flag.
> But let's consider two sides of the situation.
> 
> Firstly, it's only unsafe when there is a need to re-built the Bad
> Block Table from scratch(old BBT broken).

No, it's unsafe in the presence of bad blocks.

The BBT erasure issue relates to how me mark the flash as migrated, not
whether we migrate in the first place.

>  But currently there is no
> easy way to do that(re-build BBT on demand),

You scrub the blocks with U-Boot.  It's not supposed to be *easy*, it's
a developer recovery mechanism.

> Secondly, even if the previous said problem happens(BBT broken).  We
> can still recover all the data if we overrule the bad block flag.

How so?  The bad block markers -- including ones legitimately written to
the BBT after the fact -- are used for block skipping with certain types
of writes.  Without the knowledge of which blocks were marked bad, how
do we know which blocks were skipped?

> Only the card is not so good to be used again,

That's a pretty crappy thing to happen every time you hit a bug during
development.

But again, that's irrelevant to whether this patch should be applied
as-is, because we currently don't have any bad block migration at all.

> however, it can be used
> if we take the risk of losing data from errors that ECC can't
> notice(low possibility too).

Can you quantify "low possibility" here?

Note that any block that *was* marked bad will have a multi-bit error
from the marker itself, since it will be embedded in the main data area.

> Finally, I don't think this is a blocker issue but a better to have enhancement.

No, it is not an enhancement.  Processing bad block markers correctly is
a fundamental requirement.  And if anyone *does* start using it right
away, then we'll have to deal with their complaints if we start checking
for a migration marker later.

Why is it so critical that it be merged now, and not in a few weeks (or
next merge window) when I have a chance to do the migration code
(assuming nobody else does it first) and add a suitable check for the
migration marker in the Linux driver?

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Li Yang]

On Tue, Dec 20, 2011 at 12:47 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 12/19/2011 05:05 AM, Li Yang wrote:
>> On Sat, Dec 17, 2011 at 1:59 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On 12/15/2011 08:44 PM, LiuShuo wrote:
>>>> hi Artem,
>>>> Could this patch be applied now and we make a independent patch for  bad
>>>> block information
>>>> migration later?
>>>
>>> This patch is not safe to use without migration.
>>
>> Hi Scott,
>>
>> We agree it's not entirely safe without migrating the bad block flag.
>> But let's consider two sides of the situation.
>>
>> Firstly, it's only unsafe when there is a need to re-built the Bad
>> Block Table from scratch(old BBT broken).
>
> No, it's unsafe in the presence of bad blocks.
>

Instead of migrating the factory bad block markers I proposed to
modify the code of building BBT to make it different for 4K page, so
that the default BBT can correctly covers the factory bad blocks.  It
is the easiest way with nearly no harm to the functionality.

If you look at nand_default_block_markbad() in current implementation
of Linux MTD.  If we have set NAND_BBT_USE_FLASH option, which we did,
the bad block information in only updated in BBT not the oob area of
the first two pages of the bad block.  That means we are currently
only relies on the BBT for bad blocks.  If the BBT is created, the
factory bad block markers can be ignored, IMO.

> The BBT erasure issue relates to how me mark the flash as migrated, not
> whether we migrate in the first place.

It is connected to whether we do the migration at all.  I mentioned in
earlier mail that if we are doing the migration, we need to make sure
the migration only happens once.  And it need to be done before the
flash is used for the first time and before BBT is created.  If we
can't guarantee these condition, we are marking good blocks as bad by
doing the migration.  Even worse than doing nothing.

>
>>  But currently there is no
>> easy way to do that(re-build BBT on demand),
>
> You scrub the blocks with U-Boot.  It's not supposed to be *easy*, it's
> a developer recovery mechanism.

Scrub clears the factory bad block markers also.  It is the same
result after scrub whether or not we migrated the factory bad block
markers.

>
>> Secondly, even if the previous said problem happens(BBT broken).  We
>> can still recover all the data if we overrule the bad block flag.
>
> How so?  The bad block markers -- including ones legitimately written to
> the BBT after the fact -- are used for block skipping with certain types
> of writes.  Without the knowledge of which blocks were marked bad, how
> do we know which blocks were skipped?

This is not supposed to be *easy*.  We might get more information in
the file system level.  Or we check the content of the blocks.

>
>> Only the card is not so good to be used again,
>
> That's a pretty crappy thing to happen every time you hit a bug during
> development.
>
> But again, that's irrelevant to whether this patch should be applied
> as-is, because we currently don't have any bad block migration at all.
>
>> however, it can be used
>> if we take the risk of losing data from errors that ECC can't
>> notice(low possibility too).
>
> Can you quantify "low possibility" here?
>
> Note that any block that *was* marked bad will have a multi-bit error
> from the marker itself, since it will be embedded in the main data area.

I found the definition of bad block from one NAND chip manual: Bad
Blocks are blocks that contain one or more invalid bits whose
reliability is not guaranteed.

There is no mentioning that the bad block has to have multi-bit error.
 Although the factory bad blocks might have worse error than wear-off
bad blocks, it's not what I can tell.

>
>> Finally, I don't think this is a blocker issue but a better to have enhancement.
>
> No, it is not an enhancement.  Processing bad block markers correctly is
> a fundamental requirement.  And if anyone *does* start using it right
> away, then we'll have to deal with their complaints if we start checking
> for a migration marker later.

I agree in some extend.  I suggested to have the code of creating
correct BBT for 4k page on first use, but not doing the migration.
Given the code we have right now.   We don't take more risk than
before, and take no functionality lose.

>
> Why is it so critical that it be merged now, and not in a few weeks (or
> next merge window) when I have a chance to do the migration code
> (assuming nobody else does it first) and add a suitable check for the
> migration marker in the Linux driver?

A few weeks might be ok.  But I feared that the merge can be further
delayed and might finally goes no where.  And as I argued above, I'm
not sure if migrating is necessary in the first place.

In general.  We are not trying to get unqualified code merged.  But I
also don't agree we need to perfect all things before any of the code
can be merged.  My understanding is that even if certain code is not
complete in feature or have certain drawbacks, if the current chunk
provided some useful features and the drawbacks are acceptable, we
should merge them and add more enhancements incrementally in the
future.  Some people don't have the luck to work on one thing for a
long time, and can't possibly finish all the enhancements in one go.
It's beneficial to merge part of the whole picture if it is acceptable
rather than wait for an uncertain time for all to be finished.

- Leo

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/20/2011 03:08 AM, Li Yang wrote:
> On Tue, Dec 20, 2011 at 12:47 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 12/19/2011 05:05 AM, Li Yang wrote:
>>> On Sat, Dec 17, 2011 at 1:59 AM, Scott Wood <scottwood@freescale.com> wrote:
>>>> On 12/15/2011 08:44 PM, LiuShuo wrote:
>>>>> hi Artem,
>>>>> Could this patch be applied now and we make a independent patch for  bad
>>>>> block information
>>>>> migration later?
>>>>
>>>> This patch is not safe to use without migration.
>>>
>>> Hi Scott,
>>>
>>> We agree it's not entirely safe without migrating the bad block flag.
>>> But let's consider two sides of the situation.
>>>
>>> Firstly, it's only unsafe when there is a need to re-built the Bad
>>> Block Table from scratch(old BBT broken).
>>
>> No, it's unsafe in the presence of bad blocks.
>>
> 
> Instead of migrating the factory bad block markers I proposed to
> modify the code of building BBT to make it different for 4K page, so
> that the default BBT can correctly covers the factory bad blocks.  It
> is the easiest way with nearly no harm to the functionality.

Even if we were to agree to that (I disagree with "nearly no harm"),
this patch doesn't implement that.  As is, this patch simply ignores the
issue.

Note that besides possibly tossing away bad block information during
development, the BBT-only approach will not work for booting from NAND,
as we don't use the BBT in that case (need to keep the code minimal to
fit the 4k boot block).  Yes, this ignores blocks that were marked bad
by software, but that's usually OK since that part of the chip isn't
managed by a software layer such as jffs2 that will mark blocks as bad.

>> The BBT erasure issue relates to how me mark the flash as migrated, not
>> whether we migrate in the first place.
> 
> It is connected to whether we do the migration at all.  I mentioned in
> earlier mail that if we are doing the migration, we need to make sure
> the migration only happens once.  And it need to be done before the
> flash is used for the first time and before BBT is created.  If we
> can't guarantee these condition, we are marking good blocks as bad by
> doing the migration.  Even worse than doing nothing.

You also can't do the special BBT scan once the flash has been written
to with normal data.  This patch does not implement the special BBT scan.

>>>  But currently there is no
>>> easy way to do that(re-build BBT on demand),
>>
>> You scrub the blocks with U-Boot.  It's not supposed to be *easy*, it's
>> a developer recovery mechanism.
> 
> Scrub clears the factory bad block markers also.  

Only if you scrub bad blocks.  I was talking about scrubbing the BBT
specifically, not the entire chip.

>>> Secondly, even if the previous said problem happens(BBT broken).  We
>>> can still recover all the data if we overrule the bad block flag.
>>
>> How so?  The bad block markers -- including ones legitimately written to
>> the BBT after the fact -- are used for block skipping with certain types
>> of writes.  Without the knowledge of which blocks were marked bad, how
>> do we know which blocks were skipped?
> 
> This is not supposed to be *easy*.  We might get more information in
> the file system level.  Or we check the content of the blocks.

If you need to take special, non-automatic steps to recover the data,
that counts as data loss.

>>> however, it can be used
>>> if we take the risk of losing data from errors that ECC can't
>>> notice(low possibility too).
>>
>> Can you quantify "low possibility" here?
>>
>> Note that any block that *was* marked bad will have a multi-bit error
>> from the marker itself, since it will be embedded in the main data area.
> 
> I found the definition of bad block from one NAND chip manual: Bad
> Blocks are blocks that contain one or more invalid bits whose
> reliability is not guaranteed.
> 
> There is no mentioning that the bad block has to have multi-bit error.

"It's not guaranteed to fail" is rather different from "low possibility
of failure".

>  Although the factory bad blocks might have worse error than wear-off
> bad blocks, it's not what I can tell.

Why would they have such a mechanism to mark blocks bad, if it's not needed?

>> Why is it so critical that it be merged now, and not in a few weeks (or
>> next merge window) when I have a chance to do the migration code
>> (assuming nobody else does it first) and add a suitable check for the
>> migration marker in the Linux driver?
> 
> A few weeks might be ok.  But I feared that the merge can be further
> delayed and might finally goes no where.

And I fear that the bad block handling will be forgotten about if enough
gets merged for this to sort-of work.

> And as I argued above, I'm not sure if migrating is necessary in the first place.
> 
> In general.  We are not trying to get unqualified code merged.  But I
> also don't agree we need to perfect all things before any of the code
> can be merged.

I'm not asking for it to be perfect -- this just seems like a difficult
thing to fix once people start using the feature (similar to getting a
userspace API merged, we want to get it right first), and I'm not
comfortable with the risks of people using it without bad block handling.

> My understanding is that even if certain code is not
> complete in feature or have certain drawbacks, if the current chunk
> provided some useful features and the drawbacks are acceptable,

Whether "the drawbacks are acceptable" is the issue.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]

On Mon, 2011-12-12 at 15:30 -0600, Scott Wood wrote:
> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
> > On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
> >> NAND chips come from the factory with bad blocks marked at a certain
> >> offset into each page.  This offset is normally in the OOB area, but
> >> since we change the layout from "4k data, 128 byte oob" to "2k data, 64
> >> byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
> >> first use we need to migrate the markers so that they are still in the oob.
> > 
> > Ah, I see, thanks. Are you planning to implement in-kernel migration or
> > use a user-space tool?
> 
> That's the kind of answer I was hoping to get from Shuo. :-)
> 
> Most likely is a firmware-based tool, but I'd like there to be some way
> for the tool to mark that this has happened, so that the Linux driver
> can refuse to do non-raw accesses to a chip that isn't marked as having
> been migrated (or at least yell loudly in the log).
> 
> Speaking of raw accesses, these are currently broken in the eLBC
> driver... we need some way for the generic layer to tell us what kind of
> access it is before the transaction starts, not once it wants to read
> out the buffer (unless we add more hacks to delay the start of a read
> transaction until first buffer access...).  We'd be better off with a
> high-level "read page/write page" function that does the whole thing
> (not just buffer access, but command issuance as well).

It looks like currently you can re-define chip->read_page, so I guess
you should rework MTD and make chip->write_page re-definable?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/17/2011 08:35 AM, Artem Bityutskiy wrote:
> On Mon, 2011-12-12 at 15:30 -0600, Scott Wood wrote:
>> On 12/12/2011 03:19 PM, Artem Bityutskiy wrote:
>>> On Mon, 2011-12-12 at 15:15 -0600, Scott Wood wrote:
>>>> NAND chips come from the factory with bad blocks marked at a certain
>>>> offset into each page.  This offset is normally in the OOB area, but
>>>> since we change the layout from "4k data, 128 byte oob" to "2k data, 64
>>>> byte oob, 2k data, 64 byte oob" the marker is no longer in the oob.  On
>>>> first use we need to migrate the markers so that they are still in the oob.
>>>
>>> Ah, I see, thanks. Are you planning to implement in-kernel migration or
>>> use a user-space tool?
>>
>> That's the kind of answer I was hoping to get from Shuo. :-)
>>
>> Most likely is a firmware-based tool, but I'd like there to be some way
>> for the tool to mark that this has happened, so that the Linux driver
>> can refuse to do non-raw accesses to a chip that isn't marked as having
>> been migrated (or at least yell loudly in the log).
>>
>> Speaking of raw accesses, these are currently broken in the eLBC
>> driver... we need some way for the generic layer to tell us what kind of
>> access it is before the transaction starts, not once it wants to read
>> out the buffer (unless we add more hacks to delay the start of a read
>> transaction until first buffer access...).  We'd be better off with a
>> high-level "read page/write page" function that does the whole thing
>> (not just buffer access, but command issuance as well).
> 
> It looks like currently you can re-define chip->read_page, so I guess
> you should rework MTD and make chip->write_page re-definable?

Unless something has changed very recently, there is no chip->read_page
or chip->write_page.  There is chip->ecc.read_page and
chip->ecc.write_page, but they are too low-level.  What we'd need to
replace is a portion of nand_do_read_ops()/nand_do_write_ops().

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/19/2011 12:38 PM, Scott Wood wrote:
> On 12/17/2011 08:35 AM, Artem Bityutskiy wrote:
>> It looks like currently you can re-define chip->read_page, so I guess
>> you should rework MTD and make chip->write_page re-definable?
> 
> Unless something has changed very recently, there is no chip->read_page
> or chip->write_page.  There is chip->ecc.read_page and
> chip->ecc.write_page, but they are too low-level.  What we'd need to
> replace is a portion of nand_do_read_ops()/nand_do_write_ops().

Sorry, chip->write_page does exist -- it's chip->read_page that would
need to be made similarly redefinable.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年12月13日 05:09, Artem Bityutskiy 写道:
> On Tue, 2011-12-06 at 18:09 -0600, Scott Wood wrote:
>> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>>> From: Liu Shuo<shuo.liu@freescale.com>
>>>
>>> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
>>> to support the Nand flash chip whose page size is larger than 2K bytes,
>>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>>> them to a large buffer.
>>>
>>> Signed-off-by: Liu Shuo<shuo.liu@freescale.com>
>>> ---
>>> v3:
>>>      -remove page_size of struct fsl_elbc_mtd.
>>>      -do a oob write by NAND_CMD_RNDIN.
>>>
>>>   drivers/mtd/nand/fsl_elbc_nand.c |  243 ++++++++++++++++++++++++++++++++++----
>>>   1 files changed, 218 insertions(+), 25 deletions(-)
>> What is the plan for bad block marker migration.
I think we can use a special bbt pattern to indicate whether migration 
has been done.
(we needn't to define another marker)

Do the migration our chip->scan_bbt as follow :

/*
  * this pattern indicate that the bad block information has been migrated,
  * if this isn't found, we do the migration.
  */
static u8 migrated_bbt_pattern[] = {'M', 'b', 'b', 't', '0' };

static int fsl_elbc_bbt(struct mtd_info *mtd)
{
         if (!check_migrated_bbt_pattern())
             bad_block_info_migtrate();

          nand_default_bbt(mtd); /* default function in nand_bbt.c */
}

- LiuShuo

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 12/13/2011 09:41 PM, LiuShuo wrote:
> 于 2011年12月13日 05:09, Artem Bityutskiy 写道:
>> On Tue, 2011-12-06 at 18:09 -0600, Scott Wood wrote:
>>> On 12/03/2011 10:31 PM, shuo.liu@freescale.com wrote:
>>>> From: Liu Shuo<shuo.liu@freescale.com>
>>>>
>>>> Freescale FCM controller has a 2K size limitation of buffer RAM. In
>>>> order
>>>> to support the Nand flash chip whose page size is larger than 2K bytes,
>>>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and
>>>> save
>>>> them to a large buffer.
>>>>
>>>> Signed-off-by: Liu Shuo<shuo.liu@freescale.com>
>>>> ---
>>>> v3:
>>>>      -remove page_size of struct fsl_elbc_mtd.
>>>>      -do a oob write by NAND_CMD_RNDIN.
>>>>
>>>>   drivers/mtd/nand/fsl_elbc_nand.c |  243
>>>> ++++++++++++++++++++++++++++++++++----
>>>>   1 files changed, 218 insertions(+), 25 deletions(-)
>>> What is the plan for bad block marker migration.
> I think we can use a special bbt pattern to indicate whether migration
> has been done.
> (we needn't to define another marker)
> 
> Do the migration our chip->scan_bbt as follow :
> 
> /*
>  * this pattern indicate that the bad block information has been migrated,
>  * if this isn't found, we do the migration.
>  */
> static u8 migrated_bbt_pattern[] = {'M', 'b', 'b', 't', '0' };
> 
> static int fsl_elbc_bbt(struct mtd_info *mtd)
> {
>         if (!check_migrated_bbt_pattern())
>             bad_block_info_migtrate();
> 
>          nand_default_bbt(mtd); /* default function in nand_bbt.c */
> }

Hmm.  This is OK as long as the bad block table never gets erased (which
could happen if a user wants it reconstructed, such as if buggy software
makes a mess of it on a developer's board).  If it gets erased, we'll
end up migrating again -- and the place that factory bad block markers
would have been in is now data, so all blocks that have been written to
will show up as bad unless they happen to have 0xff at the right place.

How about a marker that is compatible with the bbt, so the same block
can be used in production (where scrubbing the bbt should never happen),
but that does not have to imply that the block is a bbt (so a developer
that might want to erase the bbt can set the mark elsewhere, preferably
just before the bbt)?

Or have two versions of the marker, one that is also a bbt marker and
one that is not.

When scanning the bbt, the driver would look for one of these markers
from the end of the chip backward.  If not found, it concludes the chip
is unmigrated.  In U-Boot, this would trigger a migration (or a message
to run a migration command).  In Linux (and U-Boot if migration is a
separate command that has not been run) an unmigrated flash could be
read-only, with the possible exception of raw accesses if needed to
support an external migration tool.

-Scott

Re: [PATCH 1/3] mtd/nand : use elbc_fcm_ctrl->oob to set FPAR_MS bit of FPAR

[Artem Bityutskiy]

On Sun, 2011-12-04 at 12:31 +0800, shuo.liu@freescale.com wrote:
> From: Liu Shuo <b35362@freescale.com>
> 
> On both of large-page chip and small-page chip, we always should use
> 'elbc_fcm_ctrl->oob' to set the FPAR_LP_MS/FPAR_SP_MS bit of FPAR, don't
> use a overflowed 'column' to set it.
> 
> Signed-off-by: Liu Shuo <b35362@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>

Pushed patches 1 and 2 to l2-mtd-2.6.git, thanks!

Artem.

[PATCH 1/3] mtd/nand : use elbc_fcm_ctrl->oob to set FPAR_MS bit of FPAR

[b35362]

From: Liu Shuo <b35362@freescale.com>

On both of large-page chip and small-page chip, we always should use
'elbc_fcm_ctrl->oob' to set the FPAR_LP_MS/FPAR_SP_MS bit of FPAR, don't
use a overflowed 'column' to set it.

Signed-off-by: Liu Shuo <b35362@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/mtd/nand/fsl_elbc_nand.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index cc08a11..6fce7da 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -414,9 +414,17 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		         page_addr, column);
 
 		elbc_fcm_ctrl->column = column;
-		elbc_fcm_ctrl->oob = 0;
 		elbc_fcm_ctrl->use_mdr = 1;
 
+		if (column >= mtd->writesize) {
+			/* OOB area */
+			column -= mtd->writesize;
+			elbc_fcm_ctrl->oob = 1;
+		} else {
+			WARN_ON(column != 0);
+			elbc_fcm_ctrl->oob = 0;
+		}
+
 		fcr = (NAND_CMD_STATUS   << FCR_CMD1_SHIFT) |
 		      (NAND_CMD_SEQIN    << FCR_CMD2_SHIFT) |
 		      (NAND_CMD_PAGEPROG << FCR_CMD3_SHIFT);
@@ -441,16 +449,12 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 			         (FIR_OP_CW1 << FIR_OP6_SHIFT) |
 			         (FIR_OP_RS  << FIR_OP7_SHIFT));
 
-			if (column >= mtd->writesize) {
+			if (elbc_fcm_ctrl->oob)
 				/* OOB area --> READOOB */
-				column -= mtd->writesize;
 				fcr |= NAND_CMD_READOOB << FCR_CMD0_SHIFT;
-				elbc_fcm_ctrl->oob = 1;
-			} else {
-				WARN_ON(column != 0);
+			else
 				/* First 256 bytes --> READ0 */
 				fcr |= NAND_CMD_READ0 << FCR_CMD0_SHIFT;
-			}
 		}
 
 		out_be32(&lbc->fcr, fcr);
-- 
1.7.1

[PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[b35362]

From: Liu Shuo <b35362@freescale.com>

Freescale FCM controller has a 2K size limitation of buffer RAM. In order
to support the Nand flash chip whose page size is larger than 2K bytes,
we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
them to a large buffer.

Signed-off-by: Liu Shuo <b35362@freescale.com>
Signed-off-by: Li Yang <leoli@freescale.com>
---
 drivers/mtd/nand/fsl_elbc_nand.c |  211 +++++++++++++++++++++++++++++++++++---
 1 files changed, 194 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index d634c5f..c96e714 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -55,7 +55,9 @@ struct fsl_elbc_mtd {
 	struct device *dev;
 	int bank;               /* Chip select bank number           */
 	u8 __iomem *vbase;      /* Chip select base virtual address  */
-	int page_size;          /* NAND page size (0=512, 1=2048)    */
+	int page_size;          /* NAND page size, the mutiple of 2048.
+				 * (0=512, 1=2048, 2=4096, 4=8192....)
+				 */
 	unsigned int fmr;       /* FCM Flash Mode Register value     */
 };
 
@@ -75,6 +77,8 @@ struct fsl_elbc_fcm_ctrl {
 	unsigned int use_mdr;    /* Non zero if the MDR is to be set      */
 	unsigned int oob;        /* Non zero if operating on OOB data     */
 	unsigned int counter;	 /* counter for the initializations	  */
+
+	char *buffer;            /* just be used when pagesize > 2048     */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -150,6 +154,42 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 };
 
 /*=================================*/
+static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
+	void *src, *dst;
+	int len = (oob ? 64 : 2048);
+
+	if (oob)
+		dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
+	else
+		dst = elbc_fcm_ctrl->buffer + subpage * 2048;
+
+	src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
+	memcpy_fromio(dst, src, len);
+}
+
+static void buffer_to_io(struct mtd_info *mtd, int subpage, int oob)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
+	void *src, *dst;
+	int len = (oob ? 64 : 2048);
+
+	if (oob)
+		src = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
+	else
+		src = elbc_fcm_ctrl->buffer + subpage * 2048;
+
+	dst = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
+	memcpy_toio(dst, src, len);
+
+	/* See the in_8() in fsl_elbc_write_buf() */
+	in_8(elbc_fcm_ctrl->addr);
+}
 
 /*
  * Set up the FCM hardware block and page address fields, and the fcm
@@ -193,7 +233,7 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 
 	/* for OOB data point to the second half of the buffer */
 	if (oob)
-		elbc_fcm_ctrl->index += priv->page_size ? 2048 : 512;
+		elbc_fcm_ctrl->index += mtd->writesize;
 
 	dev_vdbg(priv->dev, "set_addr: bank=%d, "
 			    "elbc_fcm_ctrl->addr=0x%p (0x%p), "
@@ -311,6 +351,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+	int i;
 
 	elbc_fcm_ctrl->use_mdr = 0;
 
@@ -339,6 +380,26 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 
 		fsl_elbc_do_read(chip, 0);
 		fsl_elbc_run_command(mtd);
+
+		if (priv->page_size <= 1)
+			return;
+
+		/* Continue to read the rest bytes if writesize > 2048 */
+		io_to_buffer(mtd, 0, 0);
+		io_to_buffer(mtd, 0, 1);
+
+		out_be32(&lbc->fir, FIR_OP_RB << FIR_OP1_SHIFT);
+
+		for (i = 1; i < priv->page_size; i++) {
+			/*
+			 * Maybe there are some reasons of FCM hardware timing,
+			 * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB.
+			 */
+			fsl_elbc_run_command(mtd);
+			io_to_buffer(mtd, i, 0);
+			io_to_buffer(mtd, i, 1);
+		}
+
 		return;
 
 	/* READOOB reads only the OOB because no ECC is performed. */
@@ -347,13 +408,36 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		         "fsl_elbc_cmdfunc: NAND_CMD_READOOB, page_addr:"
 			 " 0x%x, column: 0x%x.\n", page_addr, column);
 
-		out_be32(&lbc->fbcr, mtd->oobsize - column);
-		set_addr(mtd, column, page_addr, 1);
+		if (priv->page_size <= 1) {
+			out_be32(&lbc->fbcr, mtd->oobsize - column);
+			set_addr(mtd, column, page_addr, 1);
+		} else {
+			out_be32(&lbc->fbcr, 64);
+			set_addr(mtd, 0, page_addr, 1);
+			elbc_fcm_ctrl->index += column;
+		}
 
 		elbc_fcm_ctrl->read_bytes = mtd->writesize + mtd->oobsize;
 
 		fsl_elbc_do_read(chip, 1);
 		fsl_elbc_run_command(mtd);
+
+		if (priv->page_size <= 1)
+			return;
+
+		if (column < 64)
+			io_to_buffer(mtd, 0, 1);
+
+		out_be32(&lbc->fbcr, 2112);
+		out_be32(&lbc->fir, FIR_OP_RB << FIR_OP1_SHIFT);
+		out_be32(&lbc->fpar, in_be32(&lbc->fpar) & ~FPAR_LP_MS);
+
+		for (i = 1; i < priv->page_size; i++) {
+			fsl_elbc_run_command(mtd);
+			if (column < (64 * (i + 1)))
+				io_to_buffer(mtd, i, 1);
+		}
+
 		return;
 
 	/* READID must read all 5 possible bytes while CEB is active */
@@ -429,7 +513,14 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		      (NAND_CMD_SEQIN    << FCR_CMD2_SHIFT) |
 		      (NAND_CMD_PAGEPROG << FCR_CMD3_SHIFT);
 
-		if (priv->page_size) {
+		if (priv->page_size > 1) {
+			/* writesize > 2048 */
+			out_be32(&lbc->fir,
+				 (FIR_OP_CM2 << FIR_OP0_SHIFT) |
+				 (FIR_OP_CA  << FIR_OP1_SHIFT) |
+				 (FIR_OP_PA  << FIR_OP2_SHIFT) |
+				 (FIR_OP_WB  << FIR_OP3_SHIFT));
+		} else if (priv->page_size) {
 			out_be32(&lbc->fir,
 			         (FIR_OP_CM2 << FIR_OP0_SHIFT) |
 			         (FIR_OP_CA  << FIR_OP1_SHIFT) |
@@ -458,7 +549,13 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		}
 
 		out_be32(&lbc->fcr, fcr);
-		set_addr(mtd, column, page_addr, elbc_fcm_ctrl->oob);
+		if (elbc_fcm_ctrl->oob && priv->page_size > 1) {
+			set_addr(mtd, 0, page_addr, 0);
+			elbc_fcm_ctrl->index = elbc_fcm_ctrl->column;
+		} else {
+			set_addr(mtd, column, page_addr, elbc_fcm_ctrl->oob);
+		}
+
 		return;
 	}
 
@@ -472,14 +569,59 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		 * then set the exact length, otherwise use a full page
 		 * write so the HW generates the ECC.
 		 */
-		if (elbc_fcm_ctrl->oob || elbc_fcm_ctrl->column != 0 ||
-		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize)
+		if (elbc_fcm_ctrl->oob) {
+			int len;
+			/* write oob */
+			if (priv->page_size > 1) {
+				/* when pagesize of chip is greater than 2048,
+				 * we have to write full page to write spare
+				 * region, so we fill '0xff' to main region
+				 * and some bytes of spare region which we
+				 * don't want to rewrite.
+				 * (write '1' won't change the original value)
+				 */
+				memset(elbc_fcm_ctrl->buffer, 0xff,
+						elbc_fcm_ctrl->column);
+				len = 2112;
+			} else {
+				len = elbc_fcm_ctrl->index -
+						elbc_fcm_ctrl->column;
+			}
+			out_be32(&lbc->fbcr, len);
+		} else if (elbc_fcm_ctrl->column != 0 ||
+		    elbc_fcm_ctrl->index != mtd->writesize + mtd->oobsize) {
 			out_be32(&lbc->fbcr,
 				elbc_fcm_ctrl->index - elbc_fcm_ctrl->column);
-		else
+		} else {
 			out_be32(&lbc->fbcr, 0);
+		}
+
+		if (priv->page_size > 1) {
+			buffer_to_io(mtd, 0, 0);
+			buffer_to_io(mtd, 0, 1);
+		}
 
 		fsl_elbc_run_command(mtd);
+
+		if (priv->page_size <= 1)
+			return;
+
+		out_be32(&lbc->fir, FIR_OP_WB << FIR_OP1_SHIFT);
+		for (i = 1; i < priv->page_size; i++) {
+			elbc_fcm_ctrl->use_mdr = 1;
+			/* For the last subpage */
+			if (i == priv->page_size - 1)
+				out_be32(&lbc->fir,
+					(FIR_OP_WB  << FIR_OP1_SHIFT) |
+					(FIR_OP_CM3 << FIR_OP2_SHIFT) |
+					(FIR_OP_CW1 << FIR_OP3_SHIFT) |
+					(FIR_OP_RS  << FIR_OP4_SHIFT));
+
+			buffer_to_io(mtd, i, 0);
+			buffer_to_io(mtd, i, 1);
+			fsl_elbc_run_command(mtd);
+		}
+
 		return;
 	}
 
@@ -548,7 +690,14 @@ static void fsl_elbc_write_buf(struct mtd_info *mtd, const u8 *buf, int len)
 		len = bufsize - elbc_fcm_ctrl->index;
 	}
 
-	memcpy_toio(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], buf, len);
+	if (mtd->writesize > 2048) {
+		memcpy(&elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index],
+				buf, len);
+	} else {
+		memcpy_toio(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index],
+				buf, len);
+	}
+
 	/*
 	 * This is workaround for the weird elbc hangs during nand write,
 	 * Scott Wood says: "...perhaps difference in how long it takes a
@@ -594,7 +743,13 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 
 	avail = min((unsigned int)len,
 			elbc_fcm_ctrl->read_bytes - elbc_fcm_ctrl->index);
-	memcpy_fromio(buf, &elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index], avail);
+	if (mtd->writesize > 2048) {
+		memcpy(buf, &elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index],
+				avail);
+	} else {
+		memcpy_fromio(buf, &elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index],
+				avail);
+	}
 	elbc_fcm_ctrl->index += avail;
 
 	if (len > avail)
@@ -630,10 +785,17 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < len; i++)
-		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
-				!= buf[i])
-			break;
+	if (mtd->writesize > 2048) {
+		for (i = 0; i < len; i++)
+			if (elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index + i]
+					!= buf[i])
+				break;
+	} else {
+		for (i = 0; i < len; i++)
+			if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
+					!= buf[i])
+				break;
+	}
 
 	elbc_fcm_ctrl->index += len;
 	return i == len && elbc_fcm_ctrl->status == LTESR_CC ? 0 : -EIO;
@@ -716,8 +878,9 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	if (mtd->writesize == 512) {
 		priv->page_size = 0;
 		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
-	} else if (mtd->writesize == 2048) {
-		priv->page_size = 1;
+	} else if (mtd->writesize >= 2048 && mtd->writesize <= 16 * 1024) {
+		priv->page_size = mtd->writesize / 2048;
+
 		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
 		/* adjust ecc setup if needed */
 		if ((in_be32(&lbc->bank[priv->bank].br) & BR_DECC) ==
@@ -891,6 +1054,19 @@ static int __devinit fsl_elbc_nand_probe(struct platform_device *pdev)
 			goto err;
 		}
 		elbc_fcm_ctrl->counter++;
+		/*
+		 * Freescale FCM controller has a 2K size limitation of buffer
+		 * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
+		 * of chip is greater than 2048.
+		 * We malloc a large enough buffer (maximum page size is 16K).
+		 */
+		elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024, GFP_KERNEL);
+		if (!elbc_fcm_ctrl->buffer) {
+			dev_err(dev, "failed to allocate memory\n");
+			mutex_unlock(&fsl_elbc_nand_mutex);
+			ret = -ENOMEM;
+			goto err;
+		}
 
 		spin_lock_init(&elbc_fcm_ctrl->controller.lock);
 		init_waitqueue_head(&elbc_fcm_ctrl->controller.wq);
@@ -960,6 +1136,7 @@ static int fsl_elbc_nand_remove(struct platform_device *pdev)
 	elbc_fcm_ctrl->counter--;
 	if (!elbc_fcm_ctrl->counter) {
 		fsl_lbc_ctrl_dev->nand = NULL;
+		kfree(elbc_fcm_ctrl->buffer);
 		kfree(elbc_fcm_ctrl);
 	}
 	mutex_unlock(&fsl_elbc_nand_mutex);
-- 
1.7.1

RE: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Li Yang-R58472]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2211 bytes --]

> Subject: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support
> large-page Nand chip
> 
> From: Liu Shuo <b35362@freescale.com>
> 
> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> to support the Nand flash chip whose page size is larger than 2K bytes,
> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> them to a large buffer.
> 
> Signed-off-by: Liu Shuo <b35362@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c |  211
> +++++++++++++++++++++++++++++++++++---
>  1 files changed, 194 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index d634c5f..c96e714 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c

[snip]

> +static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct fsl_elbc_mtd *priv = chip->priv;
> +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
> +	void *src, *dst;
> +	int len = (oob ? 64 : 2048);
> +
> +	if (oob)
> +		dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
> +	else
> +		dst = elbc_fcm_ctrl->buffer + subpage * 2048;
> +
> +	src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
> +	memcpy_fromio(dst, src, len);

Might be safer to use _memcpy_fromio()

> +}
> +
> +static void buffer_to_io(struct mtd_info *mtd, int subpage, int oob)
> +{
> +	struct nand_chip *chip = mtd->priv;
> +	struct fsl_elbc_mtd *priv = chip->priv;
> +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
> +	void *src, *dst;
> +	int len = (oob ? 64 : 2048);
> +
> +	if (oob)
> +		src = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
> +	else
> +		src = elbc_fcm_ctrl->buffer + subpage * 2048;
> +
> +	dst = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
> +	memcpy_toio(dst, src, len);
> +
> +	/* See the in_8() in fsl_elbc_write_buf() */
> +	in_8(elbc_fcm_ctrl->addr);

Should be safer to read back the last char.

- Leo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 11/24/2011 01:37 AM, Li Yang-R58472 wrote:
>> +static void io_to_buffer(struct mtd_info *mtd, int subpage, int oob)
>> +{
>> +	struct nand_chip *chip = mtd->priv;
>> +	struct fsl_elbc_mtd *priv = chip->priv;
>> +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = priv->ctrl->nand;
>> +	void *src, *dst;
>> +	int len = (oob ? 64 : 2048);
>> +
>> +	if (oob)
>> +		dst = elbc_fcm_ctrl->buffer + mtd->writesize + subpage * 64;
>> +	else
>> +		dst = elbc_fcm_ctrl->buffer + subpage * 2048;
>> +
>> +	src = elbc_fcm_ctrl->addr + (oob ? 2048 : 0);
>> +	memcpy_fromio(dst, src, len);
> 
> Might be safer to use _memcpy_fromio()

How so?

memcpy_fromio() is the public interface that will end up calling
_memcpy_fromio() on powerpc.

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On Thu, 2011-11-24 at 08:41 +0800, b35362@freescale.com wrote:
> +               /*
> +                * Freescale FCM controller has a 2K size limitation of buffer
> +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> +                * of chip is greater than 2048.
> +                * We malloc a large enough buffer (maximum page size is 16K).
> +                */
> +               elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024, GFP_KERNEL); 

Are there NANDs with 16KiB page size?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Li Yang-R58472]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 951 bytes --]

> Subject: Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support
> large-page Nand chip
> 
> On Thu, 2011-11-24 at 08:41 +0800, b35362@freescale.com wrote:
> > +               /*
> > +                * Freescale FCM controller has a 2K size limitation of buffer
> > +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> > +                * of chip is greater than 2048.
> > +                * We malloc a large enough buffer (maximum page size is
> 16K).
> > +                */
> > +               elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024,
> GFP_KERNEL);
> 
> Are there NANDs with 16KiB page size?

We are not sure, but are there possibility that chip with 16K page will appear?  Or maybe we can add a MACRO for the maximum page size?

- Leo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

On Thu, 2011-11-24 at 07:49 +0000, Li Yang-R58472 wrote:
> > Subject: Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support
> > large-page Nand chip
> > 
> > On Thu, 2011-11-24 at 08:41 +0800, b35362@freescale.com wrote:
> > > +               /*
> > > +                * Freescale FCM controller has a 2K size limitation of buffer
> > > +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> > > +                * of chip is greater than 2048.
> > > +                * We malloc a large enough buffer (maximum page size is
> > 16K).
> > > +                */
> > > +               elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024,
> > GFP_KERNEL);
> > 
> > Are there NANDs with 16KiB page size?
> 
> We are not sure, but are there possibility that chip with 16K page will appear?  Or maybe we can add a MACRO for the maximum page size?

I do not know, but I know that allocating 32KiB of contiguous physical
RAM may cause unneeded memory pressure and even fail if the memory is
too fragmented. So I would not go for this unless this is necessary.

Did you try to look how the NAND base interface could be changed to
avoid re-allocation altogether, BTW?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[LiuShuo]

于 2011年11月24日 16:16, Artem Bityutskiy 写道:
> On Thu, 2011-11-24 at 07:49 +0000, Li Yang-R58472 wrote:
>>> Subject: Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support
>>> large-page Nand chip
>>>
>>> On Thu, 2011-11-24 at 08:41 +0800, b35362@freescale.com wrote:
>>>> +               /*
>>>> +                * Freescale FCM controller has a 2K size limitation of buffer
>>>> +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
>>>> +                * of chip is greater than 2048.
>>>> +                * We malloc a large enough buffer (maximum page size is
>>> 16K).
>>>> +                */
>>>> +               elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024,
>>> GFP_KERNEL);
>>>
>>> Are there NANDs with 16KiB page size?
>> We are not sure, but are there possibility that chip with 16K page will appear?  Or maybe we can add a MACRO for the maximum page size?
> I do not know, but I know that allocating 32KiB of contiguous physical
> RAM may cause unneeded memory pressure and even fail if the memory is
> too fragmented. So I would not go for this unless this is necessary.
What is your suggestion ? 8k is enough ?
> Did you try to look how the NAND base interface could be changed to
> avoid re-allocation altogether, BTW?
This buffer is a controller-wide resource( as Scott said), I only 
allocate buffer one time in this version.
It should be a large enough buffer for all chips.

-Liu Shuo

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Thu, 2011-11-24 at 18:02 +0800, LiuShuo wrote:
> 于 2011年11月24日 16:16, Artem Bityutskiy 写道:
> > On Thu, 2011-11-24 at 07:49 +0000, Li Yang-R58472 wrote:
> >>> Subject: Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support
> >>> large-page Nand chip
> >>>
> >>> On Thu, 2011-11-24 at 08:41 +0800, b35362@freescale.com wrote:
> >>>> +               /*
> >>>> +                * Freescale FCM controller has a 2K size limitation of buffer
> >>>> +                * RAM, so elbc_fcm_ctrl->buffer have to be used if writesize
> >>>> +                * of chip is greater than 2048.
> >>>> +                * We malloc a large enough buffer (maximum page size is
> >>> 16K).
> >>>> +                */
> >>>> +               elbc_fcm_ctrl->buffer = kmalloc(1024 * 16 + 1024,
> >>> GFP_KERNEL);
> >>>
> >>> Are there NANDs with 16KiB page size?
> >> We are not sure, but are there possibility that chip with 16K page will appear?  Or maybe we can add a MACRO for the maximum page size?
> > I do not know, but I know that allocating 32KiB of contiguous physical
> > RAM may cause unneeded memory pressure and even fail if the memory is
> > too fragmented. So I would not go for this unless this is necessary.
> What is your suggestion ? 8k is enough ?

Up to you, I do not have suggestions, just expressed a concern. I'd keep
the buffer smaller if possible. By the time 16KiB pages appear, your HW
may retire already :-)

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 11/23/2011 06:41 PM, b35362@freescale.com wrote:
> From: Liu Shuo <b35362@freescale.com>
> 
> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
> to support the Nand flash chip whose page size is larger than 2K bytes,
> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
> them to a large buffer.
> 
> Signed-off-by: Liu Shuo <b35362@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c |  211 +++++++++++++++++++++++++++++++++++---
>  1 files changed, 194 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index d634c5f..c96e714 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -55,7 +55,9 @@ struct fsl_elbc_mtd {
>  	struct device *dev;
>  	int bank;               /* Chip select bank number           */
>  	u8 __iomem *vbase;      /* Chip select base virtual address  */
> -	int page_size;          /* NAND page size (0=512, 1=2048)    */
> +	int page_size;          /* NAND page size, the mutiple of 2048.
> +				 * (0=512, 1=2048, 2=4096, 4=8192....)
> +				 */

Again, please remove this.  It was sort-of reasonable when it was a
boolean that selected between slightly different programming models.  It
doesn't make sense as "mtd->writesize == 512 ? 0 : mtd->writesize / 512".

What is the plan for migrating bad block markers on first use?

-Scott

Re: [PATCH 3/3] mtd/nand : workaround for Freescale FCM to support large-page Nand chip

[Scott Wood]

On 11/28/2011 03:48 PM, Scott Wood wrote:
> On 11/23/2011 06:41 PM, b35362@freescale.com wrote:
>> From: Liu Shuo <b35362@freescale.com>
>>
>> Freescale FCM controller has a 2K size limitation of buffer RAM. In order
>> to support the Nand flash chip whose page size is larger than 2K bytes,
>> we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and save
>> them to a large buffer.
>>
>> Signed-off-by: Liu Shuo <b35362@freescale.com>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>>  drivers/mtd/nand/fsl_elbc_nand.c |  211 +++++++++++++++++++++++++++++++++++---
>>  1 files changed, 194 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
>> index d634c5f..c96e714 100644
>> --- a/drivers/mtd/nand/fsl_elbc_nand.c
>> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
>> @@ -55,7 +55,9 @@ struct fsl_elbc_mtd {
>>  	struct device *dev;
>>  	int bank;               /* Chip select bank number           */
>>  	u8 __iomem *vbase;      /* Chip select base virtual address  */
>> -	int page_size;          /* NAND page size (0=512, 1=2048)    */
>> +	int page_size;          /* NAND page size, the mutiple of 2048.
>> +				 * (0=512, 1=2048, 2=4096, 4=8192....)
>> +				 */
> 
> Again, please remove this.  It was sort-of reasonable when it was a
> boolean that selected between slightly different programming models.  It
> doesn't make sense as "mtd->writesize == 512 ? 0 : mtd->writesize / 512".

Sorry, I meant "mtd->writesize == 512 ? 0 : mtd->writesize / 2048".

-Scott