=E4=BA=8E 2011=E5=B9=B411=E6=9C=8823=E6=97=A5 07:55, Scott Wood =E5=86=99=
=E9=81=93:

On 11/15/2011 03:29 AM, b35362@freescale.com wrote:

quoted

From: Liu Shuo<redacted>

Freescale FCM controller has a 2K size limitation of buffer RAM. In or=

der

quoted

to support the Nand flash chip whose page size is larger than 2K bytes=

,

quoted

we read/write 2k data repeatedly by issuing FIR_OP_RB/FIR_OP_WB and sa=

ve

quoted

them to a large buffer.

Signed-off-by: Liu Shuo<redacted>
Signed-off-by: Shengzhou Liu<redacted>
Signed-off-by: Li Yang<redacted>
---
  drivers/mtd/nand/fsl_elbc_nand.c |  216 ++++++++++++++++++++++++++++=

+++++++---

quoted

  1 files changed, 199 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_e=

lbc_nand.c

quoted

index c2c231b..415f87e 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=3D512, 1=3D2048)    */
+	int page_size;          /* NAND page size (0=3D512, 1=3D2048, 2=3D40=

96...),

quoted

+				 * the mutiple of 2048.
+				 */

That "..." isn't very descriptive.  What happens with 8192-byte pages?
Is it 3 or 4?

Please just get rid of this and use mtd->writesize.

quoted

+		for (i =3D 1; i<  priv->page_size; i++) {
+			/*
+			 * Maybe there are some reasons of FCM hardware timming,
+			 * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB.
+			 */

s/timming/timing/

quoted

  	/* PAGEPROG reuses all of the setup from SEQIN and adds the length =

*/

quoted

  	case NAND_CMD_PAGEPROG: {
+		int len;
  		dev_vdbg(priv->dev,
  			 "fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG "
  			 "writing %d bytes.\n", elbc_fcm_ctrl->index);
-
  		/* if the write did not start at 0 or is not a full page
  		 * 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 !=3D 0 ||
+		if (elbc_fcm_ctrl->column>=3D mtd->writesize) {
+			/* 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);

I don't like relying on this -- can we use RNDIN instead to do a
discontiguous write?

quoted

+				len =3D 2112;

len =3D min(elbc_fcm_ctrl->index, 2112);

quoted

+			} else
+				len =3D mtd->writesize + mtd->oobsize -
+					elbc_fcm_ctrl->column;
+			out_be32(&lbc->fbcr, len);

len =3D elbc_fcm_ctrl->index - elbc_fcm_ctrl->column;

Use braces on both sides of the if/else if it's needed on one side.

quoted

+		} else if (elbc_fcm_ctrl->column !=3D 0 ||
  		    elbc_fcm_ctrl->index !=3D mtd->writesize + mtd->oobsize)
  			out_be32(&lbc->fbcr, elbc_fcm_ctrl->index);

This should have set fbcr to index - column as well (after adjusting --
though really it's not a supported use case.  We should only be seeing
column !=3D 0 for oob.

quoted

@@ -625,10 +776,16 @@ static int fsl_elbc_verify_buf(struct mtd_info *=

mtd, const u_char *buf, int len)

quoted

  		return -EINVAL;
  	}

-	for (i =3D 0; i<  len; i++)
-		if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
-				!=3D buf[i])
-			break;
+	if (mtd->writesize>  2048)
+		for (i =3D 0; i<  len; i++)
+			if (elbc_fcm_ctrl->buffer[elbc_fcm_ctrl->index + i]
+					!=3D buf[i])
+				break;
+	else
+		for (i =3D 0; i<  len; i++)
+			if (in_8(&elbc_fcm_ctrl->addr[elbc_fcm_ctrl->index + i])
+					!=3D buf[i])
+				break;

Please use braces around multiline if/for bodies, even if they're
technically a single statement -- especially when you've got a dangling
else.

quoted

  	elbc_fcm_ctrl->index +=3D len;
  	return i =3D=3D len&&  elbc_fcm_ctrl->status =3D=3D LTESR_CC ? 0 : =

-EIO;

quoted

@@ -657,6 +814,7 @@ static int fsl_elbc_chip_init_tail(struct mtd_info=

 *mtd)

quoted

  	struct fsl_elbc_mtd *priv =3D chip->priv;
  	struct fsl_lbc_ctrl *ctrl =3D priv->ctrl;
  	struct fsl_lbc_regs __iomem *lbc =3D ctrl->regs;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl =3D ctrl->nand;
  	unsigned int al;

  	/* calculate FMR Address Length field */

@@ -707,12 +865,17 @@ static int fsl_elbc_chip_init_tail(struct mtd_in=

fo *mtd)

quoted

  	dev_dbg(priv->dev, "fsl_elbc_init: mtd->oobsize =3D %d\n",
  		mtd->oobsize);

+	kfree(elbc_fcm_ctrl->buffer);
+	elbc_fcm_ctrl->buffer =3D NULL;
+
  	/* adjust Option Register and ECC to match Flash page size */
  	if (mtd->writesize =3D=3D 512) {
  		priv->page_size =3D 0;
  		clrbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
-	} else if (mtd->writesize =3D=3D 2048) {
-		priv->page_size =3D 1;
+	} else if (mtd->writesize>=3D 2048) {
+		/* page_size =3D writesize / 2048 */
+		priv->page_size =3D mtd->writesize>>  11;

Why not just write priv->page_size =3D mtd->writesize / 2048 in the cod=

e

rather than the comment (it compiles to the same code)?  Other than
because you were requested to remove priv->page_size, that is. :-)

quoted

  		setbits32(&lbc->bank[priv->bank].or, OR_FCM_PGS);
  		/* adjust ecc setup if needed */
  		if ((in_be32(&lbc->bank[priv->bank].br)&  BR_DECC) =3D=3D

@@ -723,6 +886,14 @@ static int fsl_elbc_chip_init_tail(struct mtd_inf=

o *mtd)

quoted

  					&fsl_elbc_oob_lp_eccm0;
  			chip->badblock_pattern =3D&largepage_memorybased;
  		}
+
+		/* re-malloc if pagesize>  2048*/
+		if (mtd->writesize>  2048) {
+			elbc_fcm_ctrl->buffer =3D kmalloc(mtd->writesize +
+						    mtd->oobsize, GFP_KERNEL);
+			if (!elbc_fcm_ctrl->buffer)
+				return -ENOMEM;
+		}
  	} else {
  		dev_err(priv->dev,
  			"fsl_elbc_init: page size %d is not supported\n",

buffer is a controller-wide resource, but you're setting it based on th=

e

most-recently-probed NAND chip.  It should be large enough to
accommodate the largest page size in use on any connected chip.  Even i=

f

all chips in the system have the same page size, you're introducing a
race if a previously-probed chip is already in use (the controller lock
is not held here).

Just allocate a buffer large enough for a 16K page size in the main ini=

t

function, and print an error in the tail if you encounter a larger page
size.

quoted

@@ -886,6 +1057,17 @@ static int __devinit fsl_elbc_nand_probe(struct =

platform_device *pdev)

quoted

  			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 at this point, because we
+		 * don't know writesize before calling nand_scan(). We will
+		 * re-malloc later if needed.
+		 */
+		elbc_fcm_ctrl->buffer =3D kmalloc(4096 * 6, GFP_KERNEL);
+		if (!elbc_fcm_ctrl->buffer)
+			return -ENOMEM;

Clean up properly if you fail to allocate the buffer.  This includes
freeing elbc_fcm_ctrl, setting fsl_lbc_ctrl_dev->nand to NULL, and
releasing fsl_elbc_nand_mutex.

-Scott

Thanks for your suggestions. I will make it better.

-LiuShuo