[PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[b35362]

From: Liu Shuo <b35362@freescale.com>

The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
if elbc_fcm_ctrl->counter is zero.

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

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 0bb254c..a212116 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -829,7 +829,6 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
 
 	elbc_fcm_ctrl->chips[priv->bank] = NULL;
 	kfree(priv);
-	kfree(elbc_fcm_ctrl);
 	return 0;
 }
 
-- 
1.7.1

[PATCH 2/2] 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 divide a page into multi-2K pages for MTD layer driver. In that case,
we force to set the page size to 2K bytes. We convert the page address of
MTD layer driver to a real page address in flash chips and a column index
in fsl_elbc driver. We can issue any column address by UA instruction of
elbc controller.

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

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index a212116..eea7a22 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -76,6 +76,10 @@ struct fsl_elbc_fcm_ctrl {
 	unsigned int oob;        /* Non zero if operating on OOB data     */
 	unsigned int counter;	 /* counter for the initializations	  */
 	char *oob_poi;           /* Place to write ECC after read back    */
+
+	int subpage_shift;       /* If writesize > 2048, these two members*/
+	int subpage_mask;        /* are used to calculate the real page   */
+	                         /* address and real column address       */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -164,18 +168,27 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
 	int buf_num;
+	u32 real_ca = column;
 
-	elbc_fcm_ctrl->page = page_addr;
+	if (priv->page_size && elbc_fcm_ctrl->subpage_shift) {
+		real_ca = (page_addr & elbc_fcm_ctrl->subpage_mask) * 2112;
+		page_addr >>= elbc_fcm_ctrl->subpage_shift;
+	}
 
-	out_be32(&lbc->fbar,
-	         page_addr >> (chip->phys_erase_shift - chip->page_shift));
+	elbc_fcm_ctrl->page = page_addr;
 
 	if (priv->page_size) {
+		real_ca += (oob ? 2048 : 0);
+		elbc_fcm_ctrl->use_mdr = 1;
+		elbc_fcm_ctrl->mdr = real_ca;
+
+		out_be32(&lbc->fbar, page_addr >> 6);
 		out_be32(&lbc->fpar,
 		         ((page_addr << FPAR_LP_PI_SHIFT) & FPAR_LP_PI) |
 		         (oob ? FPAR_LP_MS : 0) | column);
 		buf_num = (page_addr & 1) << 2;
 	} else {
+		out_be32(&lbc->fbar, page_addr >> 5);
 		out_be32(&lbc->fpar,
 		         ((page_addr << FPAR_SP_PI_SHIFT) & FPAR_SP_PI) |
 		         (oob ? FPAR_SP_MS : 0) | column);
@@ -256,10 +269,11 @@ static void fsl_elbc_do_read(struct nand_chip *chip, int oob)
 	if (priv->page_size) {
 		out_be32(&lbc->fir,
 		         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
-		         (FIR_OP_CA  << FIR_OP1_SHIFT) |
-		         (FIR_OP_PA  << FIR_OP2_SHIFT) |
-		         (FIR_OP_CM1 << FIR_OP3_SHIFT) |
-		         (FIR_OP_RBW << FIR_OP4_SHIFT));
+		         (FIR_OP_UA  << FIR_OP1_SHIFT) |
+		         (FIR_OP_UA  << FIR_OP2_SHIFT) |
+		         (FIR_OP_PA  << FIR_OP3_SHIFT) |
+		         (FIR_OP_CM1 << FIR_OP4_SHIFT) |
+		         (FIR_OP_RBW << FIR_OP5_SHIFT));
 
 		out_be32(&lbc->fcr, (NAND_CMD_READ0 << FCR_CMD0_SHIFT) |
 		                    (NAND_CMD_READSTART << FCR_CMD1_SHIFT));
@@ -399,12 +413,13 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		if (priv->page_size) {
 			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) |
-			         (FIR_OP_CM3 << FIR_OP4_SHIFT) |
-			         (FIR_OP_CW1 << FIR_OP5_SHIFT) |
-			         (FIR_OP_RS  << FIR_OP6_SHIFT));
+			         (FIR_OP_UA  << FIR_OP1_SHIFT) |
+			         (FIR_OP_UA  << FIR_OP2_SHIFT) |
+			         (FIR_OP_PA  << 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));
 		} else {
 			out_be32(&lbc->fir,
 			         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
@@ -453,6 +468,9 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 			full_page = 1;
 		}
 
+		if (priv->page_size)
+			elbc_fcm_ctrl->use_mdr = 1;
+
 		fsl_elbc_run_command(mtd);
 
 		/* Read back the page in order to fill in the ECC for the
@@ -654,9 +672,26 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
 	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;
 	unsigned int al;
 
+	/* Hack for supporting the flash chip whose writesize is
+	 * larger than 2K bytes.
+	 */
+	if (mtd->writesize > 2048) {
+		elbc_fcm_ctrl->subpage_shift = ffs(mtd->writesize >> 11) - 1;
+		elbc_fcm_ctrl->subpage_mask =
+			(1 << elbc_fcm_ctrl->subpage_shift) - 1;
+		/* Rewrite mtd->writesize, mtd->oobsize, chip->page_shift
+		 * and chip->pagemask.
+		 */
+		mtd->writesize = 2048;
+		mtd->oobsize = 64;
+		chip->page_shift = ffs(mtd->writesize) - 1;
+		chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
+	}
+
 	/* calculate FMR Address Length field */
 	al = 0;
 	if (chip->pagemask & 0xffff0000)
-- 
1.7.1

[PATCH 2/2] 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 divide a page into multi-2K pages for MTD layer driver. In that case,
we force to set the page size to 2K bytes. We convert the page address of
MTD layer driver to a real page address in flash chips and a column index
in fsl_elbc driver. We can issue any column address by UA instruction of
elbc controller.

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

diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index a212116..eea7a22 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -76,6 +76,10 @@ struct fsl_elbc_fcm_ctrl {
 	unsigned int oob;        /* Non zero if operating on OOB data     */
 	unsigned int counter;	 /* counter for the initializations	  */
 	char *oob_poi;           /* Place to write ECC after read back    */
+
+	int subpage_shift;       /* If writesize > 2048, these two members*/
+	int subpage_mask;        /* are used to calculate the real page   */
+	                         /* address and real column address       */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -164,18 +168,27 @@ static void set_addr(struct mtd_info *mtd, int column, int page_addr, int oob)
 	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
 	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
 	int buf_num;
+	u32 real_ca = column;
 
-	elbc_fcm_ctrl->page = page_addr;
+	if (priv->page_size && elbc_fcm_ctrl->subpage_shift) {
+		real_ca = (page_addr & elbc_fcm_ctrl->subpage_mask) * 2112;
+		page_addr >>= elbc_fcm_ctrl->subpage_shift;
+	}
 
-	out_be32(&lbc->fbar,
-	         page_addr >> (chip->phys_erase_shift - chip->page_shift));
+	elbc_fcm_ctrl->page = page_addr;
 
 	if (priv->page_size) {
+		real_ca += (oob ? 2048 : 0);
+		elbc_fcm_ctrl->use_mdr = 1;
+		elbc_fcm_ctrl->mdr = real_ca;
+
+		out_be32(&lbc->fbar, page_addr >> 6);
 		out_be32(&lbc->fpar,
 		         ((page_addr << FPAR_LP_PI_SHIFT) & FPAR_LP_PI) |
 		         (oob ? FPAR_LP_MS : 0) | column);
 		buf_num = (page_addr & 1) << 2;
 	} else {
+		out_be32(&lbc->fbar, page_addr >> 5);
 		out_be32(&lbc->fpar,
 		         ((page_addr << FPAR_SP_PI_SHIFT) & FPAR_SP_PI) |
 		         (oob ? FPAR_SP_MS : 0) | column);
@@ -256,10 +269,11 @@ static void fsl_elbc_do_read(struct nand_chip *chip, int oob)
 	if (priv->page_size) {
 		out_be32(&lbc->fir,
 		         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
-		         (FIR_OP_CA  << FIR_OP1_SHIFT) |
-		         (FIR_OP_PA  << FIR_OP2_SHIFT) |
-		         (FIR_OP_CM1 << FIR_OP3_SHIFT) |
-		         (FIR_OP_RBW << FIR_OP4_SHIFT));
+		         (FIR_OP_UA  << FIR_OP1_SHIFT) |
+		         (FIR_OP_UA  << FIR_OP2_SHIFT) |
+		         (FIR_OP_PA  << FIR_OP3_SHIFT) |
+		         (FIR_OP_CM1 << FIR_OP4_SHIFT) |
+		         (FIR_OP_RBW << FIR_OP5_SHIFT));
 
 		out_be32(&lbc->fcr, (NAND_CMD_READ0 << FCR_CMD0_SHIFT) |
 		                    (NAND_CMD_READSTART << FCR_CMD1_SHIFT));
@@ -399,12 +413,13 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 		if (priv->page_size) {
 			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) |
-			         (FIR_OP_CM3 << FIR_OP4_SHIFT) |
-			         (FIR_OP_CW1 << FIR_OP5_SHIFT) |
-			         (FIR_OP_RS  << FIR_OP6_SHIFT));
+			         (FIR_OP_UA  << FIR_OP1_SHIFT) |
+			         (FIR_OP_UA  << FIR_OP2_SHIFT) |
+			         (FIR_OP_PA  << 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));
 		} else {
 			out_be32(&lbc->fir,
 			         (FIR_OP_CM0 << FIR_OP0_SHIFT) |
@@ -453,6 +468,9 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
 			full_page = 1;
 		}
 
+		if (priv->page_size)
+			elbc_fcm_ctrl->use_mdr = 1;
+
 		fsl_elbc_run_command(mtd);
 
 		/* Read back the page in order to fill in the ECC for the
@@ -654,9 +672,26 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	struct nand_chip *chip = mtd->priv;
 	struct fsl_elbc_mtd *priv = chip->priv;
 	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;
 	unsigned int al;
 
+	/* Hack for supporting the flash chip whose writesize is
+	 * larger than 2K bytes.
+	 */
+	if (mtd->writesize > 2048) {
+		elbc_fcm_ctrl->subpage_shift = ffs(mtd->writesize >> 11) - 1;
+		elbc_fcm_ctrl->subpage_mask =
+			(1 << elbc_fcm_ctrl->subpage_shift) - 1;
+		/* Rewrite mtd->writesize, mtd->oobsize, chip->page_shift
+		 * and chip->pagemask.
+		 */
+		mtd->writesize = 2048;
+		mtd->oobsize = 64;
+		chip->page_shift = ffs(mtd->writesize) - 1;
+		chip->pagemask = (chip->chipsize >> chip->page_shift) - 1;
+	}
+
 	/* calculate FMR Address Length field */
 	al = 0;
 	if (chip->pagemask & 0xffff0000)
-- 
1.7.1

RE: [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Mike Hench]

Any boot ideas ?
Will the FCM load 2k and run it?

Thanks for any insight you might have.

RE: [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Mike Hench]

Any boot ideas ?
Will the FCM load 2k and run it?

Thanks for any insight you might have.

Re: [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Scott Wood]

On Tue, 28 Jun 2011 11:35:12 -0400
Mike Hench <mhench@elutions.com> wrote:

> 
> Any boot ideas ?
> Will the FCM load 2k and run it?

The 4K boot region will have to be split over pages 0 and 2 (2k view) or
the first half of pages 0 and 1 (4k view).

-Scott

[U-Boot] [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Matthew L. Creech]

On Tue, Jun 28, 2011 at 12:30 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Tue, 28 Jun 2011 11:35:12 -0400
> Mike Hench <mhench@elutions.com> wrote:
>
>>
>> Any boot ideas ?
>> Will the FCM load 2k and run it?
>
> The 4K boot region will have to be split over pages 0 and 2 (2k view) or
> the first half of pages 0 and 1 (4k view).
>

(Redirecting to the U-Boot list)

Hi Scott,

Does this kind of page-splitting only apply to the IPL and nothing
else?  If so, it seems that if:

1. modifications are made to U-Boot's fsl_elbc_nand driver similar to
Liu Shuo's kernel mods, and
2. an option is added to generate u-boot-nand.bin with its IPL split
into two 2k chunks

then we can boot and run entirely from a 4k page device on MPC 83xx, correct?

That would be very good news, as it seems that [some] 2k NAND parts
are starting to go EOL.

Thanks!

-- 
Matthew L. Creech

[U-Boot] [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Scott Wood]

On Tue, 5 Jul 2011 19:08:21 -0400
"Matthew L. Creech" <mlcreech@gmail.com> wrote:

> On Tue, Jun 28, 2011 at 12:30 PM, Scott Wood <scottwood@freescale.com> wrote:
> > On Tue, 28 Jun 2011 11:35:12 -0400
> > Mike Hench <mhench@elutions.com> wrote:
> >
> >>
> >> Any boot ideas ?
> >> Will the FCM load 2k and run it?
> >
> > The 4K boot region will have to be split over pages 0 and 2 (2k view) or
> > the first half of pages 0 and 1 (4k view).
> >
> 
> (Redirecting to the U-Boot list)
> 
> Hi Scott,
> 
> Does this kind of page-splitting only apply to the IPL and nothing
> else?

Yes, because that's loaded directly by the hardware which doesn't
implement this workaround.

> If so, it seems that if:
> 
> 1. modifications are made to U-Boot's fsl_elbc_nand driver similar to
> Liu Shuo's kernel mods, and
> 2. an option is added to generate u-boot-nand.bin with its IPL split
> into two 2k chunks
> 
> then we can boot and run entirely from a 4k page device on MPC 83xx, correct?

That's the plan, though I don't know whether it's been tried yet.

-Scott

[U-Boot] [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Mike Hench]

It works. It is awesome to be able to do this.
If you are interested, and this isn't pretty
This a patch against 2010-06 uboot.
(been busy, but this was too good to ignore)

I will try to find some time to make it better
But for now comments are appreciated.
The Makefile and environment variable hack is ugly.

I Was going to add error checking and 2 copies of u-boot
(if a page fails, get it from the second copy)
This has an additional hack to fake the subpages.

diff -purN orig/drivers/mtd/nand/fsl_elbc_nand.c
u-boot-2010.06/drivers/mtd/nand/fsl_elbc_nand.c
--- orig/drivers/mtd/nand/fsl_elbc_nand.c	2011-06-30
14:11:27.304294055 -0500
+++ u-boot-2010.06/drivers/mtd/nand/fsl_elbc_nand.c	2011-06-30
14:10:46.880516050 -0500
@@ -86,6 +86,10 @@ struct fsl_elbc_ctrl {
 	unsigned int use_mdr;    /* Non zero if the MDR is to be set
*/
 	unsigned int oob;        /* Non zero if operating on OOB data
*/
 	uint8_t *oob_poi;        /* Place to write ECC after read back
*/
+
+	int subpage_shift;       /* If writesize > 2048, these two
members*/
+	int subpage_mask;        /* are used to calculate the real page
*/
+                            /* address and real column address       */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -173,10 +177,20 @@ static void set_addr(struct mtd_info *mt
 	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 	fsl_lbus_t *lbc = ctrl->regs;
 	int buf_num;
+	u32 real_ca = column;
+
+	if (priv->page_size && ctrl->subpage_shift) {
+		real_ca = (page_addr & ctrl->subpage_mask) * 2112;
+		page_addr >>= ctrl->subpage_shift;
+	}
 
 	ctrl->page = page_addr;
 
 	if (priv->page_size) {
+		real_ca += (oob ? 2048 : 0);
+		ctrl->use_mdr = 1;
+		ctrl->mdr = real_ca;
+
 		out_be32(&lbc->fbar, page_addr >> 6);
 		out_be32(&lbc->fpar,
 			 ((page_addr << FPAR_LP_PI_SHIFT) & FPAR_LP_PI)
|
@@ -265,11 +279,12 @@ static void fsl_elbc_do_read(struct nand
 
 	if (priv->page_size) {
 		out_be32(&lbc->fir,
-			 (FIR_OP_CW0 << FIR_OP0_SHIFT) |
-			 (FIR_OP_CA  << FIR_OP1_SHIFT) |
-			 (FIR_OP_PA  << FIR_OP2_SHIFT) |
-			 (FIR_OP_CW1 << FIR_OP3_SHIFT) |
-			 (FIR_OP_RBW << FIR_OP4_SHIFT));
+			(FIR_OP_CW0 << FIR_OP0_SHIFT) |
+			(FIR_OP_UA  << FIR_OP1_SHIFT) |
+			(FIR_OP_UA  << FIR_OP2_SHIFT) |
+			(FIR_OP_PA  << FIR_OP3_SHIFT) |
+			(FIR_OP_CW1 << FIR_OP4_SHIFT) |
+			(FIR_OP_RBW << FIR_OP5_SHIFT));
 
 		out_be32(&lbc->fcr, (NAND_CMD_READ0 << FCR_CMD0_SHIFT) |
 				    (NAND_CMD_READSTART <<
FCR_CMD1_SHIFT));
@@ -399,10 +414,11 @@ static void fsl_elbc_cmdfunc(struct mtd_
 
 			out_be32(&lbc->fir,
 				 (FIR_OP_CW0 << FIR_OP0_SHIFT) |
-				 (FIR_OP_CA  << FIR_OP1_SHIFT) |
-				 (FIR_OP_PA  << FIR_OP2_SHIFT) |
-				 (FIR_OP_WB  << FIR_OP3_SHIFT) |
-				 (FIR_OP_CW1 << FIR_OP4_SHIFT));
+				 (FIR_OP_UA  << FIR_OP1_SHIFT) |
+				 (FIR_OP_UA  << FIR_OP2_SHIFT) |
+				 (FIR_OP_PA  << FIR_OP3_SHIFT) |
+				 (FIR_OP_WB  << FIR_OP4_SHIFT) |
+				 (FIR_OP_CW1 << FIR_OP5_SHIFT));
 		} else {
 			fcr = (NAND_CMD_PAGEPROG << FCR_CMD1_SHIFT) |
 			      (NAND_CMD_SEQIN << FCR_CMD2_SHIFT);
@@ -453,6 +469,10 @@ static void fsl_elbc_cmdfunc(struct mtd_
 			full_page = 1;
 		}
 
+		if (priv->page_size)
+			ctrl->use_mdr = 1;
+
+
 		fsl_elbc_run_command(mtd);
 
 		ctrl->oob_poi = NULL;
@@ -808,3 +828,29 @@ int board_nand_init(struct nand_chip *na
 
 	return 0;
 }
+
+int board_nand_init_tail(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
+
+	/* Hack for supporting the flash chip whose writesize is
+	 * larger than 2K bytes.
+	 */
+	if (mtd->writesize > 2048) {
+		ctrl->subpage_shift = ffs(mtd->writesize >> 11) - 1;
+		ctrl->subpage_mask =
+			(1 << ctrl->subpage_shift) - 1;
+		/* Rewrite mtd->writesize, mtd->oobsize,
chip->page_shift
+		 * and chip->pagemask.
+		 */
+		mtd->writesize = 2048;
+		mtd->oobsize = 64;
+		chip->page_shift = ffs(mtd->writesize) - 1;
+		chip->pagemask = (chip->chipsize >> chip->page_shift) -
1;
+		mtd->subpage_sft--;
+	}
+}
+
+
diff -purN orig/drivers/mtd/nand/nand_base.c
u-boot-2010.06/drivers/mtd/nand/nand_base.c
--- orig/drivers/mtd/nand/nand_base.c	2011-06-30 14:11:27.408242045
-0500
+++ u-boot-2010.06/drivers/mtd/nand/nand_base.c	2011-06-30
14:10:46.920496045 -0500
@@ -2998,6 +2998,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 		}
 	}
 	chip->subpagesize = mtd->writesize >> mtd->subpage_sft;
+printf("sps=%d\n", chip->subpagesize);
 
 	/* Initialize state */
 	chip->state = FL_READY;
diff -purN orig/drivers/mtd/nand/nand.c
u-boot-2010.06/drivers/mtd/nand/nand.c
--- orig/drivers/mtd/nand/nand.c	2011-06-30 14:11:27.452220050
-0500
+++ u-boot-2010.06/drivers/mtd/nand/nand.c	2011-06-30
14:10:46.948482047 -0500
@@ -74,6 +74,9 @@ static void nand_init_chip(struct mtd_in
 		mtd->name = NULL;
 		mtd->size = 0;
 	}
+#ifdef CONFIG_SYS_NAND_INIT_TAIL
+	board_nand_init_tail(mtd);
+#endif
 
 }
 
diff -purN orig/include/nand.h u-boot-2010.06/include/nand.h
--- orig/include/nand.h	2010-06-29 16:28:28.000000000 -0500
+++ u-boot-2010.06/include/nand.h	2011-06-30 14:10:46.888512049
-0500
@@ -31,7 +31,9 @@ extern void nand_init(void);
 #include <linux/mtd/nand.h>
 
 extern int board_nand_init(struct nand_chip *nand);
-
+#ifdef CONFIG_SYS_NAND_INIT_TAIL
+extern int board_nand_init_tail(struct mtd_info *mtd);
+#endif
 typedef struct mtd_info nand_info_t;
 
 extern int nand_curr_device;
diff -purN orig/Makefile u-boot-2010.06/Makefile
--- orig/Makefile	2010-06-29 16:28:28.000000000 -0500
+++ u-boot-2010.06/Makefile	2011-06-30 14:10:46.892510049 -0500
@@ -371,7 +371,7 @@ $(NAND_SPL):	$(TIMESTAMP_FILE) $(VERSION
 		$(MAKE) -C nand_spl/board/$(BOARDDIR) all
 
 $(U_BOOT_NAND):	$(NAND_SPL) $(obj)u-boot.bin
-		cat $(obj)nand_spl/u-boot-spl-16k.bin $(obj)u-boot.bin >
$(obj)u-boot-nand.bin
+		cat $(obj)nand_spl/u-boot-spl-pad.bin $(obj)u-boot.bin >
$(obj)u-boot-nand.bin
 
 $(ONENAND_IPL):	$(TIMESTAMP_FILE) $(VERSION_FILE)
$(obj)include/autoconf.mk
 		$(MAKE) -C onenand_ipl/board/$(BOARDDIR) all
diff -purN orig/nand_spl/board/freescale/mpc8313erdb/Makefile
u-boot-2010.06/nand_spl/board/freescale/mpc8313erdb/Makefile
--- orig/nand_spl/board/freescale/mpc8313erdb/Makefile	2011-06-30
14:11:27.196348060 -0500
+++ u-boot-2010.06/nand_spl/board/freescale/mpc8313erdb/Makefile
2011-06-30 14:10:46.900506048 -0500
@@ -24,7 +24,17 @@
 
 NAND_SPL := y
 TEXT_BASE := 0xfff00000
-PAD_TO := 0xfff04000
+
+NAND_BLOCK_SIZE ?= 128K
+NAND_PAGE_SIZE ?= 2K
+
+ifeq ($(NAND_BLOCK_SIZE),256K)
+PAD_TO := 0xfff40000
+endif
+ifeq ($(NAND_BLOCK_SIZE),128K)
+PAD_TO := 0xfff20000
+endif
+PAD_TO ?= 0xfff04000
 
 include $(TOPDIR)/config.mk
 
@@ -44,12 +54,21 @@ LNDIR	:= $(OBJTREE)/nand_spl/board/$(BOA
 
 nandobj	:= $(OBJTREE)/nand_spl/
 
-ALL	= $(nandobj)u-boot-spl $(nandobj)u-boot-spl.bin
$(nandobj)u-boot-spl-16k.bin
+ALL	= $(nandobj)u-boot-spl $(nandobj)u-boot-spl.bin
$(nandobj)u-boot-spl-pad.bin
 
 all:	$(obj).depend $(ALL)
 
-$(nandobj)u-boot-spl-16k.bin: $(nandobj)u-boot-spl
+$(nandobj)u-boot-spl-pad.bin: $(nandobj)u-boot-spl
 	$(OBJCOPY) ${OBJCFLAGS} --pad-to=$(PAD_TO) -O binary $< $@
+ifeq ($(NAND_PAGE_SIZE),4K)
+	dd if=$@ bs=2048 count=1 > p0
+	dd if=$@ bs=2048 count=1 skip=1 > p1
+	dd if=$@ bs=2048 count=1 skip=2 > p2
+	dd if=$@ bs=2048 count=1 skip=3 > p3
+	dd if=$@ bs=2048 skip=4 > p4
+	cat p0 p2 p1 p3 p4 > $@
+	rm -f p0 p1 p2 p3 p4
+endif
 
 $(nandobj)u-boot-spl.bin:	$(nandobj)u-boot-spl
 	$(OBJCOPY) ${OBJCFLAGS} -O binary $< $@
diff -purN orig/nand_spl/nand_boot_fsl_elbc.c
u-boot-2010.06/nand_spl/nand_boot_fsl_elbc.c
--- orig/nand_spl/nand_boot_fsl_elbc.c	2010-06-29 16:28:28.000000000
-0500
+++ u-boot-2010.06/nand_spl/nand_boot_fsl_elbc.c	2011-06-30
14:10:56.715596060 -0500
@@ -47,6 +47,59 @@ static void nand_wait(void)
 	}
 }
 
+#if CONFIG_SYS_NAND_PAGE_SIZE > 2048
+#define PAGE_MULT (CONFIG_SYS_NAND_PAGE_SIZE/2048)
+#define BLOCK_PAGES
(CONFIG_SYS_NAND_BLOCK_SIZE/CONFIG_SYS_NAND_PAGE_SIZE)
+static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
+{
+	fsl_lbus_t *regs = (fsl_lbus_t *)(CONFIG_SYS_IMMR + 0x5000);
+	uchar *buf = (uchar *)CONFIG_SYS_NAND_BASE;
+	int fmr = (15 << FMR_CWTO_SHIFT) | (2 << FMR_AL_SHIFT) | 2;
+	int page_addr2k = offs/2048;
+	int j, page_in_block, col, buf_ofs;
+	int pos = 0;
+
+	fmr |= FMR_ECCM;
+	out_be32(&regs->fcr, (NAND_CMD_READ0 << FCR_CMD0_SHIFT) |
+	                     (NAND_CMD_READSTART << FCR_CMD1_SHIFT));
+	out_be32(&regs->fir,
+	         (FIR_OP_CW0 << FIR_OP0_SHIFT) |
+	         (FIR_OP_UA  << FIR_OP1_SHIFT) |
+	         (FIR_OP_UA  << FIR_OP2_SHIFT) |
+	         (FIR_OP_PA  << FIR_OP3_SHIFT) |
+	         (FIR_OP_CW1 << FIR_OP4_SHIFT) |
+	         (FIR_OP_RBW << FIR_OP5_SHIFT));
+
+	out_be32(&regs->fbcr, 0);
+	clrsetbits_be32(&regs->bank[0].br, BR_DECC, BR_DECC_CHK_GEN);
+
+	while (pos < uboot_size) {
+		page_in_block = page_addr2k/PAGE_MULT & (BLOCK_PAGES-1);
+		col = (page_addr2k & (PAGE_MULT-1))*2112;
+
+		if(page_in_block == 0 && col == 0)
+			out_be32(&regs->fbar,
+
page_addr2k/(CONFIG_SYS_NAND_BLOCK_SIZE/2048)
+			);
+
+		out_be32(&regs->ltesr, ~0);
+		out_be32(&regs->lteatr, 0);
+		out_be32(&regs->fpar, page_in_block << 12);
+		out_be32(&regs->fmr, fmr);
+		out_be32(&regs->mdr, col);
+		out_be32(&regs->lsor, 0);
+		nand_wait();
+
+		buf_ofs = (page_in_block & 1) * 4096;
+
+		for (j = 0; j < 2048; j++)
+			dst[pos + j] = buf[buf_ofs + j];
+
+		page_addr2k++;
+		pos += 2048;
+	}
+}
+#else
 static void nand_load(unsigned int offs, int uboot_size, uchar *dst)
 {
 	fsl_lbus_t *regs = (fsl_lbus_t *)(CONFIG_SYS_IMMR + 0x5000);
@@ -122,6 +175,7 @@ static void nand_load(unsigned int offs,
 		} while ((offs & (block_size - 1)) && (pos <
uboot_size));
 	}
 }
+#endif
 
 /*
  * The main entry for NAND booting. It's necessary that SDRAM is
already


-----Original Message-----
From: Scott Wood [mailto:scottwood at freescale.com] 
Sent: Tuesday, July 05, 2011 6:13 PM
To: Matthew L. Creech
Cc: Mike Hench; b35362 at freescale.com; u-boot at lists.denx.de
Subject: Re: [PATCH 2/2] mtd/nand : workaround for Freescale FCM to
supportlarge-page Nand chip

On Tue, 5 Jul 2011 19:08:21 -0400
"Matthew L. Creech" <mlcreech@gmail.com> wrote:

> On Tue, Jun 28, 2011 at 12:30 PM, Scott Wood <scottwood@freescale.com>
wrote:
> > On Tue, 28 Jun 2011 11:35:12 -0400
> > Mike Hench <mhench@elutions.com> wrote:
> >
> >>
> >> Any boot ideas ?
> >> Will the FCM load 2k and run it?
> >
> > The 4K boot region will have to be split over pages 0 and 2 (2k
view) or
> > the first half of pages 0 and 1 (4k view).
> >
> 
> (Redirecting to the U-Boot list)
> 
> Hi Scott,
> 
> Does this kind of page-splitting only apply to the IPL and nothing
> else?

Yes, because that's loaded directly by the hardware which doesn't
implement this workaround.

> If so, it seems that if:
> 
> 1. modifications are made to U-Boot's fsl_elbc_nand driver similar to
> Liu Shuo's kernel mods, and
> 2. an option is added to generate u-boot-nand.bin with its IPL split
> into two 2k chunks
> 
> then we can boot and run entirely from a 4k page device on MPC 83xx,
correct?

That's the plan, though I don't know whether it's been tried yet.

-Scott

[U-Boot] [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Mike Hench]

Ok, so to port Liu Shuo's huge page nand to fake 2k page patch to uboot
We either need to introduce a callback after mtd initialization
(Which would have to be outside the driver) or lie about the ID info
coming from the driver.
I choose the second.
Does this look reasonable to you guys?
The IPL boot loader part is NOT included here.
The new stuff is fix_id_info()


diff -purN orig/drivers/mtd/nand/fsl_elbc_nand.c
u-boot-2011.06/drivers/mtd/nand/fsl_elbc_nand.c
--- orig/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-08
14:18:18.634544056 -0500
+++ u-boot-2011.06/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-08
14:17:58.592570060 -0500
@@ -86,6 +86,10 @@ struct fsl_elbc_ctrl {
 	unsigned int use_mdr;    /* Non zero if the MDR is to be set
*/
 	unsigned int oob;        /* Non zero if operating on OOB data
*/
 	uint8_t *oob_poi;        /* Place to write ECC after read back
*/
+
+	int subpage_shift;       /* If writesize > 2048, these two
members*/
+	int subpage_mask;        /* are used to calculate the real page
*/
+                            /* address and real column address       */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -173,10 +177,20 @@ static void set_addr(struct mtd_info *mt
 	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
 	fsl_lbc_t *lbc = ctrl->regs;
 	int buf_num;
+	u32 real_ca = column;
+
+	if (priv->page_size && ctrl->subpage_shift) {
+		real_ca = (page_addr & ctrl->subpage_mask) * 2112;
+		page_addr >>= ctrl->subpage_shift;
+	}
 
 	ctrl->page = page_addr;
 
 	if (priv->page_size) {
+		real_ca += (oob ? 2048 : 0);
+		ctrl->use_mdr = 1;
+		ctrl->mdr = real_ca;
+
 		out_be32(&lbc->fbar, page_addr >> 6);
 		out_be32(&lbc->fpar,
 			 ((page_addr << FPAR_LP_PI_SHIFT) & FPAR_LP_PI)
|
@@ -265,11 +279,12 @@ static void fsl_elbc_do_read(struct nand
 
 	if (priv->page_size) {
 		out_be32(&lbc->fir,
-			 (FIR_OP_CW0 << FIR_OP0_SHIFT) |
-			 (FIR_OP_CA  << FIR_OP1_SHIFT) |
-			 (FIR_OP_PA  << FIR_OP2_SHIFT) |
-			 (FIR_OP_CW1 << FIR_OP3_SHIFT) |
-			 (FIR_OP_RBW << FIR_OP4_SHIFT));
+			(FIR_OP_CW0 << FIR_OP0_SHIFT) |
+			(FIR_OP_UA  << FIR_OP1_SHIFT) |
+			(FIR_OP_UA  << FIR_OP2_SHIFT) |
+			(FIR_OP_PA  << FIR_OP3_SHIFT) |
+			(FIR_OP_CW1 << FIR_OP4_SHIFT) |
+			(FIR_OP_RBW << FIR_OP5_SHIFT));
 
 		out_be32(&lbc->fcr, (NAND_CMD_READ0 << FCR_CMD0_SHIFT) |
 				    (NAND_CMD_READSTART <<
FCR_CMD1_SHIFT));
@@ -288,6 +303,28 @@ static void fsl_elbc_do_read(struct nand
 	}
 }
 
+#ifdef CONFIG_NAND_FSL_ELBC_HUGE_PAGE_WORKAROUND
+void fix_id_info(struct fsl_elbc_ctrl *ctrl)
+{
+	int ext = in_8(&ctrl->addr[ctrl->index+3]);
+
+	/* Hack for supporting the flash chip whose writesize is
+	 * larger than 2K bytes. if the page size is 4k or greater
+	 * say it is 2k.
+	 */
+	if(ext == 0xff)
+		return;
+	if((ext & 0x3) >= 2) {
+		ctrl->subpage_shift = (ext & 0x3) - 1;
+		ctrl->subpage_mask = (1 << ctrl->subpage_shift) - 1;
+		ext &= ~0x3;
+		ext |= 1;
+		out_8(&ctrl->addr[ctrl->index+3], ext);
+		ext = in_8(&ctrl->addr[ctrl->index+3]);
+	}
+}
+#endif
+
 /* cmdfunc send commands to the FCM */
 static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int
command,
 			     int column, int page_addr)
@@ -355,6 +392,9 @@ static void fsl_elbc_cmdfunc(struct mtd_
 
 		set_addr(mtd, 0, 0, 0);
 		fsl_elbc_run_command(mtd);
+#ifdef CONFIG_NAND_FSL_ELBC_HUGE_PAGE_WORKAROUND
+		fix_id_info(ctrl);
+#endif
 		return;
 
 	/* ERASE1 stores the block and page address */
@@ -399,10 +439,11 @@ static void fsl_elbc_cmdfunc(struct mtd_
 
 			out_be32(&lbc->fir,
 				 (FIR_OP_CW0 << FIR_OP0_SHIFT) |
-				 (FIR_OP_CA  << FIR_OP1_SHIFT) |
-				 (FIR_OP_PA  << FIR_OP2_SHIFT) |
-				 (FIR_OP_WB  << FIR_OP3_SHIFT) |
-				 (FIR_OP_CW1 << FIR_OP4_SHIFT));
+				 (FIR_OP_UA  << FIR_OP1_SHIFT) |
+				 (FIR_OP_UA  << FIR_OP2_SHIFT) |
+				 (FIR_OP_PA  << FIR_OP3_SHIFT) |
+				 (FIR_OP_WB  << FIR_OP4_SHIFT) |
+				 (FIR_OP_CW1 << FIR_OP5_SHIFT));
 		} else {
 			fcr = (NAND_CMD_PAGEPROG << FCR_CMD1_SHIFT) |
 			      (NAND_CMD_SEQIN << FCR_CMD2_SHIFT);
@@ -453,6 +494,10 @@ static void fsl_elbc_cmdfunc(struct mtd_
 			full_page = 1;
 		}
 
+		if (priv->page_size)
+			ctrl->use_mdr = 1;
+
+
 		fsl_elbc_run_command(mtd);
 
 		ctrl->oob_poi = NULL;

[U-Boot] [PATCH 2/2] mtd/nand : workaround for Freescale FCM to supportlarge-page Nand chip

[Scott Wood]

On Fri, 8 Jul 2011 15:10:03 -0400
Mike Hench <mhench@elutions.com> wrote:

> Ok, so to port Liu Shuo's huge page nand to fake 2k page patch to uboot
> We either need to introduce a callback after mtd initialization
> (Which would have to be outside the driver) or lie about the ID info
> coming from the driver.
> I choose the second.

I'd prefer the first.  It's long overdue for U-Boot to start letting
drivers use the scan/tail split that is already present in the code from
Linux, and useful more than just here.

Fixing up the ID info directly also seems more fragile, given the
variations (existing and future) that there seem to be in ID formats.

-Scott

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

[Artem Bityutskiy]

On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> +	/* Hack for supporting the flash chip whose writesize is
> +	 * larger than 2K bytes.
> +	 */

Please, use proper kernel multi-line comments. Please, make sure
checkpatch.pl does not generate 13 errors with this patch.

-- 
Best Regards,
Artem Bityutskiy

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

[Artem Bityutskiy]

On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> +	/* Hack for supporting the flash chip whose writesize is
> +	 * larger than 2K bytes.
> +	 */

Please, use proper kernel multi-line comments. Please, make sure
checkpatch.pl does not generate 13 errors with this patch.

-- 
Best Regards,
Artem Bityutskiy

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

[Scott Wood]

On Wed, 29 Jun 2011 09:22:04 +0300
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> > +	/* Hack for supporting the flash chip whose writesize is
> > +	 * larger than 2K bytes.
> > +	 */
> 
> Please, use proper kernel multi-line comments. Please, make sure
> checkpatch.pl does not generate 13 errors with this patch.

Most of the checkpatch complaints are about existing style in the file --
particularly, the use of tabs only for indentation, with spaces used for
alignment beyond the indentation point.

-Scott

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

[Artem Bityutskiy]

On Wed, 2011-06-29 at 11:43 -0500, Scott Wood wrote:
> On Wed, 29 Jun 2011 09:22:04 +0300
> Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 
> > On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> > > +	/* Hack for supporting the flash chip whose writesize is
> > > +	 * larger than 2K bytes.
> > > +	 */
> > 
> > Please, use proper kernel multi-line comments. Please, make sure
> > checkpatch.pl does not generate 13 errors with this patch.
> 
> Most of the checkpatch complaints are about existing style in the file --
> particularly, the use of tabs only for indentation, with spaces used for
> alignment beyond the indentation point.

OK, fair enough, but the multi-line comment is a valid complain,
although minor. And you guys could just fix the style issues as well in
a separate patch-set.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Artem Bityutskiy]

On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> From: Liu Shuo <b35362@freescale.com>
> 
> The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
> fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
> if elbc_fcm_ctrl->counter is zero.
> 
> Signed-off-by: Liu Shuo <b35362@freescale.com>
> ---
>  drivers/mtd/nand/fsl_elbc_nand.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index 0bb254c..a212116 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -829,7 +829,6 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
>  
>  	elbc_fcm_ctrl->chips[priv->bank] = NULL;
>  	kfree(priv);
> -	kfree(elbc_fcm_ctrl);
>  	return 0;
>  }

Do we have to assign fsl_lbc_ctrl_dev->nand to NULL in
fsl_elbc_nand_remove() then? I think that assignment can be killed then.

        if (!elbc_fcm_ctrl->counter) {
                fsl_lbc_ctrl_dev->nand = NULL;
                kfree(elbc_fcm_ctrl);
        }

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Scott Wood]

On Wed, 29 Jun 2011 09:20:25 +0300
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> > From: Liu Shuo <b35362@freescale.com>
> > 
> > The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
> > fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
> > if elbc_fcm_ctrl->counter is zero.
> > 
> > Signed-off-by: Liu Shuo <b35362@freescale.com>
> > ---
> >  drivers/mtd/nand/fsl_elbc_nand.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> > index 0bb254c..a212116 100644
> > --- a/drivers/mtd/nand/fsl_elbc_nand.c
> > +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> > @@ -829,7 +829,6 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
> >  
> >  	elbc_fcm_ctrl->chips[priv->bank] = NULL;
> >  	kfree(priv);
> > -	kfree(elbc_fcm_ctrl);
> >  	return 0;
> >  }
> 
> Do we have to assign fsl_lbc_ctrl_dev->nand to NULL in
> fsl_elbc_nand_remove() then? I think that assignment can be killed then.
> 
>         if (!elbc_fcm_ctrl->counter) {
>                 fsl_lbc_ctrl_dev->nand = NULL;
>                 kfree(elbc_fcm_ctrl);
>         }
> 

If we're freeing fsl_lbc_ctrl, we'd better get rid of references to it...

-Scott

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Artem Bityutskiy]

On Wed, 2011-06-29 at 11:45 -0500, Scott Wood wrote:
> On Wed, 29 Jun 2011 09:20:25 +0300
> Artem Bityutskiy <dedekind1@gmail.com> wrote:
> 
> > On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> > > From: Liu Shuo <b35362@freescale.com>
> > > 
> > > The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
> > > fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
> > > if elbc_fcm_ctrl->counter is zero.
> > > 
> > > Signed-off-by: Liu Shuo <b35362@freescale.com>
> > > ---
> > >  drivers/mtd/nand/fsl_elbc_nand.c |    1 -
> > >  1 files changed, 0 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> > > index 0bb254c..a212116 100644
> > > --- a/drivers/mtd/nand/fsl_elbc_nand.c
> > > +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> > > @@ -829,7 +829,6 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
> > >  
> > >  	elbc_fcm_ctrl->chips[priv->bank] = NULL;
> > >  	kfree(priv);
> > > -	kfree(elbc_fcm_ctrl);
> > >  	return 0;
> > >  }
> > 
> > Do we have to assign fsl_lbc_ctrl_dev->nand to NULL in
> > fsl_elbc_nand_remove() then? I think that assignment can be killed then.
> > 
> >         if (!elbc_fcm_ctrl->counter) {
> >                 fsl_lbc_ctrl_dev->nand = NULL;
> >                 kfree(elbc_fcm_ctrl);
> >         }
> > 
> 
> If we're freeing fsl_lbc_ctrl, we'd better get rid of references to it...

Yes, on the one hand this is a good defensive programming practice, on
the other hand it hides double-free bugs. Like this patch fixes a
double-free bug, and why it was noticed before? I thought may be because
of this NULL assignment?

I do not insist though, that was just a suggestion/question.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Scott Wood]

On Thu, 30 Jun 2011 14:53:13 +0300
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Wed, 2011-06-29 at 11:45 -0500, Scott Wood wrote:
> > If we're freeing fsl_lbc_ctrl, we'd better get rid of references to it...
> 
> Yes, on the one hand this is a good defensive programming practice, on
> the other hand it hides double-free bugs. Like this patch fixes a
> double-free bug, and why it was noticed before? I thought may be because
> of this NULL assignment?

I'm not sure how the NULL assignment was hiding anything here.  It was
probably hidden only because nobody tested it with suitable debug options
enabled since the code was last reorganized.

If the NULL assignment is dropped, consider what happens if the
fsl_elbc_nand module is removed then reinserted.  On reinsertion, it will
see a non-NULL fsl_lbc_ctrl_dev->nand, and will skip allocating a new one.
Then you're referencing freed memory.

Looking more closely, the MAX_BANKS loop should be removed.  Since the
reorganization, the platform device represents one chip, not the
controller, so we should only be removing that one chip.

-Scott

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Artem Bityutskiy]

On Thu, 2011-06-30 at 11:26 -0500, Scott Wood wrote:
> If the NULL assignment is dropped, consider what happens if the
> fsl_elbc_nand module is removed then reinserted.  On reinsertion, it
> will
> see a non-NULL fsl_lbc_ctrl_dev->nand, and will skip allocating a new
> one.
> Then you're referencing freed memory.

Oh, then this sounds like a separate bug. Removing the module should
kill everything, and re-inserging the module should have zero
dependencies on the previous states...

Anyway, if you think the original patch is OK, I can put it to my tree.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Scott Wood]

On Fri, 1 Jul 2011 08:40:21 +0300
Artem Bityutskiy <dedekind1@gmail.com> wrote:

> On Thu, 2011-06-30 at 11:26 -0500, Scott Wood wrote:
> > If the NULL assignment is dropped, consider what happens if the
> > fsl_elbc_nand module is removed then reinserted.  On reinsertion, it
> > will
> > see a non-NULL fsl_lbc_ctrl_dev->nand, and will skip allocating a new
> > one.
> > Then you're referencing freed memory.
> 
> Oh, then this sounds like a separate bug. Removing the module should
> kill everything, and re-inserging the module should have zero
> dependencies on the previous states...

fsl_lbc_ctrl_dev (and thus the fsl_lbc_ctrl_dev->nand pointer) is not part
of the module, it is part of arch/powerpc/sysdev/fsl_lbc.c.  NAND isn't the
only thing that elbc does.  Since there can be multiple NAND chips, which
are separately probed, the first chip (under a lock) creates the NAND state
that is shared among the chips, and the last one removed destroys it.

> Anyway, if you think the original patch is OK, I can put it to my tree.

I think it's OK.  The loop also needs to be removed, so the remove callback
operates only on the particular chip it's called on, but that's a separate
bug.

-Scott

Re: [PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()

[Artem Bityutskiy]

On Tue, 2011-06-28 at 09:50 +0800, b35362@freescale.com wrote:
> From: Liu Shuo <b35362@freescale.com>
> 
> The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
> fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
> if elbc_fcm_ctrl->counter is zero.
> 
> Signed-off-by: Liu Shuo <b35362@freescale.com>

Changed the subject to something shorted and pushed to l2-mtd-2.6.git,
thanks.

-- 
Best Regards,
Artem Bityutskiy