[U-Boot] [PATCH v2] nand: Hack to support 4k page in fsl_elbc_nand
Freescale FCM controller has a 2K size limitation of buffer RAM. In order to support the Nand flash chip with pagesize 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. Because of this, the in flash layout of the oob is different from the default for 4096kiB page sizes. Therefore, we need to migrate the factory bad block markers from the original position to the new layout.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@freescale.com Signed-off-by: Liu Shuo b35362@freescale.com Signed-off-by: Rafael Beims rafael.beims@datacom.ind.br --- Changes in v2: - Added check to disallow the migration code to run in devices with page size <= 2048
drivers/mtd/nand/fsl_elbc_nand.c | 447 +++++++++++++++++++++++++++++++++++--- 1 files changed, 419 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 9076ad4..3b6bb1d 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -76,15 +76,17 @@ struct fsl_elbc_ctrl {
/* device info */ fsl_lbc_t *regs; - u8 __iomem *addr; /* Address of assigned FCM buffer */ - unsigned int page; /* Last page written to / read from */ - unsigned int read_bytes; /* Number of bytes read during command */ - unsigned int column; /* Saved column from SEQIN */ - unsigned int index; /* Pointer to next byte to 'read' */ - unsigned int status; /* status read from LTESR after last op */ - unsigned int mdr; /* UPM/FCM Data Register value */ - unsigned int use_mdr; /* Non zero if the MDR is to be set */ - unsigned int oob; /* Non zero if operating on OOB data */ + u8 __iomem *addr; /* Address of assigned FCM buffer */ + unsigned int page; /* Last page written to / read from */ + unsigned int read_bytes; /* Number of bytes read during command */ + unsigned int column; /* Saved column from SEQIN */ + unsigned int index; /* Pointer to next byte to 'read' */ + unsigned int status; /* status read from LTESR after last op */ + unsigned int mdr; /* UPM/FCM Data Register value */ + unsigned int use_mdr; /* Non zero if the MDR is to be set */ + unsigned int oob; /* Non zero if operating on OOB data */ + char *buffer; /* Just used when pagesize is greater */ + /* than FCM RAM 2K limitation */ };
/* These map to the positions used by the FCM hardware ECC generator */ @@ -131,6 +133,15 @@ static struct nand_bbt_descr largepage_memorybased = { .pattern = scan_ff_pattern, };
+static u8 migrated_pattern[] = { 0xde, 0xad, 0xde, 0xad }; +static struct nand_bbt_descr largepage_migrated = { + .options = 0, + .offs = 1, + .len = 4, + .maxblocks = 3, + .pattern = migrated_pattern, +}; + /* * ELBC may use HW ECC, so that OOB offsets, that NAND core uses for bbt, * interfere with ECC positions, that's why we implement our own descriptors. @@ -159,6 +170,35 @@ static struct nand_bbt_descr bbt_mirror_descr = { .pattern = mirror_pattern, };
+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_ctrl *ctrl = priv->ctrl; + void *src, *dst; + int len = oob ? 64 : 2048; + + /* for emulating 4096+ bytes NAND using 2048-byte FCM RAM */ + dst = ctrl->buffer + (oob ? mtd->writesize : 0) + subpage * len; + src = 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_ctrl *ctrl = priv->ctrl; + void *src, *dst; + int len = oob ? 64 : 2048; + + src = ctrl->buffer + (oob ? mtd->writesize : 0) + subpage * len; + dst = ctrl->addr + (oob ? 2048 : 0); + memcpy_toio(dst, src, len); + /* See the in_8() in fsl_elbc_write_buf() */ + in_8(ctrl->addr); +} + /*=================================*/
/* @@ -194,7 +234,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) - ctrl->index += priv->page_size ? 2048 : 512; + ctrl->index += mtd->writesize;
vdbg("set_addr: bank=%d, ctrl->addr=0x%p (0x%p), " "index %x, pes %d ps %d\n", @@ -256,13 +296,14 @@ static int fsl_elbc_run_command(struct mtd_info *mtd) return ctrl->status == LTESR_CC ? 0 : -EIO; }
-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_elbc_ctrl *ctrl = priv->ctrl; fsl_lbc_t *lbc = ctrl->regs;
- if (priv->page_size) { + if (mtd->writesize >= 2048) { out_be32(&lbc->fir, (FIR_OP_CW0 << FIR_OP0_SHIFT) | (FIR_OP_CA << FIR_OP1_SHIFT) | @@ -295,6 +336,7 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command, struct fsl_elbc_mtd *priv = chip->priv; struct fsl_elbc_ctrl *ctrl = priv->ctrl; fsl_lbc_t *lbc = ctrl->regs; + int i, nps = mtd->writesize / 2048;
ctrl->use_mdr = 0;
@@ -319,8 +361,28 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command, ctrl->read_bytes = mtd->writesize + mtd->oobsize; 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); + /* + * Maybe there are some reasons of FCM hardware timing, + * we must insert a FIR_OP_NOP(0x00) before FIR_OP_RB. + */ + out_be32(&lbc->fir, FIR_OP_NOP << FIR_OP0_SHIFT | + FIR_OP_RB << FIR_OP1_SHIFT); + + for (i = 1; i < nps; i++) { + 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. */ @@ -328,14 +390,35 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command, vdbg("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); + ctrl->index += column; + }
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->fpar, in_be32(&lbc->fpar) & ~FPAR_LP_MS); + out_be32(&lbc->fir, FIR_OP_RB << FIR_OP1_SHIFT); + out_be32(&lbc->fbcr, 2112); + + for (i = 1; i < nps; 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 */ @@ -357,6 +440,8 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command, ctrl->mdr = column; set_addr(mtd, 0, 0, 0); fsl_elbc_run_command(mtd); + if (mtd->writesize > 2048) + memcpy_fromio(ctrl->buffer, ctrl->addr, 256); return;
/* ERASE1 stores the block and page address */ @@ -394,8 +479,28 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command,
ctrl->column = column; ctrl->oob = 0; + if (column >= mtd->writesize) { + /* OOB area */ + column -= mtd->writesize; + ctrl->oob = 1; + } else { + ctrl->oob = 0; + }
- if (priv->page_size) { + if (mtd->writesize > 2048) { + /* writesize > 2048 */ + fcr = (NAND_CMD_STATUS << FCR_CMD1_SHIFT) | + (NAND_CMD_SEQIN << FCR_CMD2_SHIFT) | + (NAND_CMD_PAGEPROG << FCR_CMD3_SHIFT); + if (ctrl->oob) + fcr |= NAND_CMD_RNDIN << FCR_CMD0_SHIFT; + + 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 (mtd->writesize == 2048) { fcr = (NAND_CMD_SEQIN << FCR_CMD0_SHIFT) | (NAND_CMD_PAGEPROG << FCR_CMD1_SHIFT);
@@ -438,6 +543,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; vdbg("fsl_elbc_cmdfunc: NAND_CMD_PAGEPROG " "writing %d bytes.\n", ctrl->index);
@@ -445,14 +551,70 @@ 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 (ctrl->oob || ctrl->column != 0 || - ctrl->index != mtd->writesize + mtd->oobsize) - out_be32(&lbc->fbcr, ctrl->index); - else + ctrl->index != mtd->writesize + mtd->oobsize) { + if (ctrl->oob && mtd->writesize > 2048) { + out_be32(&lbc->fbcr, 64); + } else { + out_be32(&lbc->fbcr, ctrl->index - + ctrl->column); + } + } else { out_be32(&lbc->fbcr, 0); + } + + if (mtd->writesize > 2048) { + if (!ctrl->oob) + buffer_to_io(mtd, 0, 0); + buffer_to_io(mtd, 0, 1); + }
fsl_elbc_run_command(mtd);
+ if (mtd->writesize <= 2048) + return; + + if (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 < nps; i++) { + pos += 2112; + ctrl->mdr = pos; + ctrl->use_mdr = 1; + if (i == nps - 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 < nps; i++) { + if (i == nps - 1) { + 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; }
@@ -473,6 +635,8 @@ static void fsl_elbc_cmdfunc(struct mtd_info *mtd, unsigned int command, * write-protected, even when it is not. */ out_8(ctrl->addr, in_8(ctrl->addr) | NAND_STATUS_WP); + if (mtd->writesize > 2048) + ctrl->buffer[0] = in_8(ctrl->addr); return;
/* RESET without waiting for the ready line */ @@ -519,7 +683,11 @@ static void fsl_elbc_write_buf(struct mtd_info *mtd, const u8 *buf, int len) len = bufsize - ctrl->index; }
- memcpy_toio(&ctrl->addr[ctrl->index], buf, len); + if (mtd->writesize > 2048) + memcpy(&ctrl->buffer[ctrl->index], buf, len); + else + memcpy_toio(&ctrl->addr[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 @@ -543,8 +711,13 @@ static u8 fsl_elbc_read_byte(struct mtd_info *mtd) struct fsl_elbc_ctrl *ctrl = priv->ctrl;
/* If there are still bytes in the FCM, then use the next byte. */ - if (ctrl->index < ctrl->read_bytes) - return in_8(&ctrl->addr[ctrl->index++]); + if (ctrl->index < ctrl->read_bytes) { + int index = ctrl->index++; + if (mtd->writesize > 2048) + return ctrl->buffer[index]; + else + return in_8(&ctrl->addr[index]); + }
printf("read_byte beyond end of buffer\n"); return ERR_BYTE; @@ -564,7 +737,10 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 *buf, int len) return;
avail = min((unsigned int)len, ctrl->read_bytes - ctrl->index); - memcpy_fromio(buf, &ctrl->addr[ctrl->index], avail); + if (mtd->writesize > 2048) + memcpy(buf, &ctrl->buffer[ctrl->index], avail); + else + memcpy_fromio(buf, &ctrl->addr[ctrl->index], avail); ctrl->index += avail;
if (len > avail) @@ -598,9 +774,17 @@ static int fsl_elbc_verify_buf(struct mtd_info *mtd, return -EINVAL; }
- for (i = 0; i < len; i++) - if (in_8(&ctrl->addr[ctrl->index + i]) != buf[i]) - break; + if (mtd->writesize > 2048) { + for (i = 0; i < len; i++) { + if (ctrl->buffer[ctrl->index + i] != buf[i]) + break; + } + } else { + for (i = 0; i < len; i++) { + if (in_8(&ctrl->addr[ctrl->index + i]) != buf[i]) + break; + } + }
ctrl->index += len; return i == len && ctrl->status == LTESR_CC ? 0 : -EIO; @@ -637,6 +821,8 @@ static int fsl_elbc_wait(struct mtd_info *mtd, struct nand_chip *chip) * write-protected, even when it is not. */ out_8(ctrl->addr, in_8(ctrl->addr) | NAND_STATUS_WP); + if (mtd->writesize > 2048) + ctrl->buffer[0] = in_8(ctrl->addr); return fsl_elbc_read_byte(mtd); }
@@ -686,6 +872,187 @@ static void fsl_elbc_ctrl_init(void) elbc_ctrl->addr = NULL; }
+static int fsl_elbc_check_pattern(struct mtd_info *mtd, uint8_t *buf, + struct nand_bbt_descr *bd) +{ + uint8_t *p = buf; + int i; + + /* Compare the pattern */ + for (i = 0; i < bd->len ; i++) { + if (p[i+bd->offs] != bd->pattern[i]) + return -1; + } + + return 0; +} + +/* + * Scan read raw data from flash + */ +static int fsl_elbc_scan_read_raw_oob(struct mtd_info *mtd, uint8_t *buf, + loff_t offs, size_t len, struct nand_bbt_descr *bd) +{ + fsl_elbc_cmdfunc(mtd, NAND_CMD_READOOB, 0, offs); + fsl_elbc_read_buf(mtd, buf, mtd->oobsize); + + if (fsl_elbc_check_pattern(mtd, buf, bd)) + return 1; + + return 0; +} + + +static int fsl_elbc_migrate_badblocks(struct mtd_info *mtd, + struct nand_bbt_descr *bd) +{ + struct nand_chip *this = mtd->priv; + int len, numblocks, i; + int startblock; + loff_t from; + uint8_t *newoob, *buf; + + struct fsl_elbc_mtd *priv = this->priv; + struct fsl_elbc_ctrl *ctrl = priv->ctrl; + + int num_subpages = mtd->writesize / 2048-1; + len = mtd->writesize + mtd->oobsize; + numblocks = this->chipsize >> (this->phys_erase_shift - 1); + startblock = 0; + from = 0; + + newoob = vmalloc(mtd->oobsize); + memset(newoob, 0xff, mtd->writesize+mtd->oobsize); + + for (i = startblock; i < numblocks; i += 2) { + int page = (from >> this->page_shift) & this->pagemask; + fsl_elbc_cmdfunc(mtd, NAND_CMD_READ0, 0, page); + + /* As we are already using the hack to read the bytes + * from NAND, the original badblock marker is offset + * from its original location in the internal buffer. + * This is because the hack reads 2048 + 64 and already + * positions the spare in the correct region + * (after the 4096 offset) + */ + uint8_t *badblock_pattern = (ctrl->buffer+ + mtd->writesize-(num_subpages*64))+bd->offs; + if (fsl_elbc_check_pattern(mtd, badblock_pattern, bd)) { + printf("Badblock found at %08x, migrating...\n", page); + memcpy(newoob, badblock_pattern , bd->len); + fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN, + mtd->writesize, page); + fsl_elbc_write_buf(mtd, newoob, mtd->oobsize); + fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); + } + + from += (1 << this->phys_erase_shift); + } + + vfree(newoob); + return 0; +} + +static int fsl_elbc_write_migration_marker(struct mtd_info *mtd, + uint8_t *buf, int len, struct nand_bbt_descr *bd) +{ + struct nand_chip *this = mtd->priv; + struct nand_bbt_descr *td = this->bbt_td; + int startblock; + int dir, i; + int blocks_to_write = 2; + + /* Start below maximum bbt */ + startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks; + dir = -1; + + for (i = 0; i < bd->maxblocks && blocks_to_write != 0; i++) { + int actblock = startblock + dir*i; + loff_t offs = (loff_t)actblock << this->phys_erase_shift; + int page = (offs >> this->page_shift) & this->pagemask; + + /* Avoid badblocks writing the marker... */ + if (!fsl_elbc_scan_read_raw_oob(mtd, buf, page, mtd->writesize, + &largepage_memorybased)) { + + /* We are reusing this buffer, reset it */ + memset(buf, 0xff, len); + memcpy(buf+bd->offs, bd->pattern, bd->len); + + /* Mark the block as bad the same way that + * it's done for the bbt. This should avoid + * this block being overwritten + */ + memset(buf+this->badblockpos, 0x02, 1); + + fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN, + mtd->writesize, page); + fsl_elbc_write_buf(mtd, buf, mtd->oobsize); + fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1); + blocks_to_write--; + printf("Wrote migration marker to offset: %x\n", page); + } + } + + if (blocks_to_write != 0) + printf("Could not write migration marker due to badblocks\n"); + + return 0; +} + +static int fsl_elbc_scan_bbt(struct mtd_info *mtd) +{ + struct nand_chip *this = mtd->priv; + struct nand_bbt_descr *bd = &largepage_migrated; + struct nand_bbt_descr *td = this->bbt_td; + int len; + int startblock, block, dir; + uint8_t *buf; + int migrate = 0; + + if (mtd->writesize > 2048) { + /* Start below maximum bbt */ + startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks; + dir = -1; + + /* Allocate a temporary buffer for one eraseblock incl. oob */ + len = (1 << this->phys_erase_shift); + len += (len >> this->page_shift) * mtd->oobsize; + buf = vmalloc(len); + if (!buf) { + printk(KERN_ERR "fsl_elbc_nand: Out of memory\n"); + kfree(this->bbt); + this->bbt = NULL; + return -ENOMEM; + } + + for (block = 0; block < td->maxblocks; block++) { + int actblock = startblock + dir * block; + loff_t offs = (loff_t)actblock << this->phys_erase_shift; + int page = (offs >> this->page_shift) & this->pagemask; + + migrate = fsl_elbc_scan_read_raw_oob(mtd, buf, page, + mtd->writesize, &largepage_migrated); + + /* We found the migration marker, get out of here */ + if (migrate == 0) + break; + } + + if (migrate) { + printf("Moving factory marked badblocks to new oob\n"); + fsl_elbc_migrate_badblocks(mtd, &largepage_memorybased); + fsl_elbc_write_migration_marker(mtd, buf, len, + &largepage_migrated); + } + + vfree(buf); + } + /* Now that we checked and possibly migrated badblock + * markers, continue with default bbt scanning */ + return nand_scan_bbt(mtd, this->badblock_pattern); +} + static int fsl_elbc_chip_init(int devnum, u8 *addr) { struct mtd_info *mtd = &nand_info[devnum]; @@ -741,6 +1108,7 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr) nand->select_chip = fsl_elbc_select_chip; nand->cmdfunc = fsl_elbc_cmdfunc; nand->waitfunc = fsl_elbc_wait; + nand->scan_bbt = fsl_elbc_scan_bbt;
/* set up nand options */ nand->bbt_td = &bbt_main_descr; @@ -804,6 +1172,29 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr) if (ret) return ret;
+ /* + * Freescale FCM controller has a 2KB size limitation of buffer RAM, + * so elbc_ctrl->buffer has to be used if pagesize of NAND devices + * chip greater than 2048. + */ + if (mtd->writesize > 2048) { + elbc_ctrl->buffer = + kmalloc(mtd->writesize + mtd->oobsize, GFP_KERNEL); + memset(elbc_ctrl->buffer, 0xff, mtd->writesize + mtd->oobsize); + if (!elbc_ctrl->buffer) { + printf("failed to allocate memory for elbc buffer\n"); + return -ENOMEM; + } + + /* We can use only n x 64 bytes of the spare area + * where n is the number of 2048k pages used to read + * the full device page + */ + mtd->oobsize = (mtd->writesize / 2048) * 64; + } else { + elbc_ctrl->buffer = NULL; + } + ret = nand_scan_tail(mtd); if (ret) return ret;
On 06/28/2012 03:47 PM, Rafael Beims wrote:
Freescale FCM controller has a 2K size limitation of buffer RAM. In order to support the Nand flash chip with pagesize 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. Because of this, the in flash layout of the oob is different from the default for 4096kiB page sizes. Therefore, we need to migrate the factory bad block markers from the original position to the new layout.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@freescale.com Signed-off-by: Liu Shuo b35362@freescale.com Signed-off-by: Rafael Beims rafael.beims@datacom.ind.br
Changes in v2:
- Added check to disallow the migration code to run in devices with
page size <= 2048
drivers/mtd/nand/fsl_elbc_nand.c | 447 +++++++++++++++++++++++++++++++++++--- 1 files changed, 419 insertions(+), 28 deletions(-)
Thanks for working on this! I've been meaning to for a while, but have been busy with other things.
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 9076ad4..3b6bb1d 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -76,15 +76,17 @@ struct fsl_elbc_ctrl {
/* device info */ fsl_lbc_t *regs;
- u8 __iomem *addr; /* Address of assigned FCM buffer */
- unsigned int page; /* Last page written to / read from */
- unsigned int read_bytes; /* Number of bytes read during command */
- unsigned int column; /* Saved column from SEQIN */
- unsigned int index; /* Pointer to next byte to 'read' */
- unsigned int status; /* status read from LTESR after last op */
- unsigned int mdr; /* UPM/FCM Data Register value */
- unsigned int use_mdr; /* Non zero if the MDR is to be set */
- unsigned int oob; /* Non zero if operating on OOB data */
- u8 __iomem *addr; /* Address of assigned FCM buffer */
- unsigned int page; /* Last page written to / read from */
- unsigned int read_bytes; /* Number of bytes read during command */
- unsigned int column; /* Saved column from SEQIN */
- unsigned int index; /* Pointer to next byte to 'read' */
- unsigned int status; /* status read from LTESR after last op */
- unsigned int mdr; /* UPM/FCM Data Register value */
- unsigned int use_mdr; /* Non zero if the MDR is to be set */
- unsigned int oob; /* Non zero if operating on OOB data */
- char *buffer; /* Just used when pagesize is greater */
/* than FCM RAM 2K limitation */};
/* These map to the positions used by the FCM hardware ECC generator */ @@ -131,6 +133,15 @@ static struct nand_bbt_descr largepage_memorybased = { .pattern = scan_ff_pattern, };
+static u8 migrated_pattern[] = { 0xde, 0xad, 0xde, 0xad };
Let's generate a proper random number here -- or at least a meaningful ASCII string. Things like the above are too common and invite magic number collision.
+static int fsl_elbc_migrate_badblocks(struct mtd_info *mtd,
struct nand_bbt_descr *bd)+{
- struct nand_chip *this = mtd->priv;
- int len, numblocks, i;
- int startblock;
- loff_t from;
- uint8_t *newoob, *buf;
- struct fsl_elbc_mtd *priv = this->priv;
- struct fsl_elbc_ctrl *ctrl = priv->ctrl;
- int num_subpages = mtd->writesize / 2048-1;
- len = mtd->writesize + mtd->oobsize;
- numblocks = this->chipsize >> (this->phys_erase_shift - 1);
- startblock = 0;
- from = 0;
- newoob = vmalloc(mtd->oobsize);
Even if this were Linux, oobsize should never be big enough to need vmalloc.
- memset(newoob, 0xff, mtd->writesize+mtd->oobsize);
You're writing beyond the end of that buffer.
- for (i = startblock; i < numblocks; i += 2) {
int page = (from >> this->page_shift) & this->pagemask;fsl_elbc_cmdfunc(mtd, NAND_CMD_READ0, 0, page);/* As we are already using the hack to read the bytes* from NAND, the original badblock marker is offset* from its original location in the internal buffer.* This is because the hack reads 2048 + 64 and already* positions the spare in the correct region* (after the 4096 offset)*/uint8_t *badblock_pattern = (ctrl->buffer+mtd->writesize-(num_subpages*64))+bd->offs;
Spaces around operators.
I think that should be (num_subpages - 1) * 64.
if (fsl_elbc_check_pattern(mtd, badblock_pattern, bd)) {printf("Badblock found at %08x, migrating...\n", page);memcpy(newoob, badblock_pattern , bd->len);
No space before ,
+static int fsl_elbc_write_migration_marker(struct mtd_info *mtd,
uint8_t *buf, int len, struct nand_bbt_descr *bd)+{
- struct nand_chip *this = mtd->priv;
- struct nand_bbt_descr *td = this->bbt_td;
- int startblock;
- int dir, i;
- int blocks_to_write = 2;
- /* Start below maximum bbt */
- startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;
- dir = -1;
If you want startblock below the bbt area, I think you're off by one.
- for (i = 0; i < bd->maxblocks && blocks_to_write != 0; i++) {
int actblock = startblock + dir*i;loff_t offs = (loff_t)actblock << this->phys_erase_shift;int page = (offs >> this->page_shift) & this->pagemask;/* Avoid badblocks writing the marker... */if (!fsl_elbc_scan_read_raw_oob(mtd, buf, page, mtd->writesize,&largepage_memorybased)) {/* We are reusing this buffer, reset it */memset(buf, 0xff, len);memcpy(buf+bd->offs, bd->pattern, bd->len);/* Mark the block as bad the same way that* it's done for the bbt. This should avoid* this block being overwritten*/memset(buf+this->badblockpos, 0x02, 1);fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN,mtd->writesize, page);fsl_elbc_write_buf(mtd, buf, mtd->oobsize);fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);blocks_to_write--;printf("Wrote migration marker to offset: %x\n", page);}
Why are we writing the marker twice?
+static int fsl_elbc_scan_bbt(struct mtd_info *mtd) +{
- struct nand_chip *this = mtd->priv;
- struct nand_bbt_descr *bd = &largepage_migrated;
- struct nand_bbt_descr *td = this->bbt_td;
- int len;
- int startblock, block, dir;
- uint8_t *buf;
- int migrate = 0;
- if (mtd->writesize > 2048) {
/* Start below maximum bbt */startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;
Again, off by one.
dir = -1;
dir never seems to get set to anything else, so why a variable?
I think we should start with the last block, and search backwards until we find it, or until we exceed some reasonable limit, such as td->maxblocks * 2 (Is that really enough? How many contiguous bad blocks can we see?). Actually writing a joint bbt/migration marker (which was requested in earlier discussion to avoid wasting an erase block, but I don't want it to be mandatory) can come later, but we want to recognize it now.
/* Allocate a temporary buffer for one eraseblock incl. oob */len = (1 << this->phys_erase_shift);len += (len >> this->page_shift) * mtd->oobsize;buf = vmalloc(len);
This code isn't going to be shared with Linux, so just malloc(). Likewise printf(...) instead of printk(KERN_ERR ...).
BTW, should mention at least in the changelog that this is partially derived from code in nand_bbt.c. Also, using this code means we need to remove the "or later" from fsl_elbc_nand.c, because nand_bbt.c is GPL v2 only. Wolfgang probably won't be pleased. It might be better to just write it from scratch -- I'm not sure how much it really helps to be mimicking the bbt code here (without actually being able to share it), versus straightforwardly coding this specific task.
if (!buf) {printk(KERN_ERR "fsl_elbc_nand: Out of memory\n");kfree(this->bbt);this->bbt = NULL;return -ENOMEM;}
Why are we freeing the bbt here? When did we allocate it?
for (block = 0; block < td->maxblocks; block++) {int actblock = startblock + dir * block;loff_t offs = (loff_t)actblock << this->phys_erase_shift;int page = (offs >> this->page_shift) & this->pagemask;migrate = fsl_elbc_scan_read_raw_oob(mtd, buf, page,mtd->writesize, &largepage_migrated);/* We found the migration marker, get out of here */if (migrate == 0)break;}if (migrate) {printf("Moving factory marked badblocks to new oob\n");fsl_elbc_migrate_badblocks(mtd, &largepage_memorybased);fsl_elbc_write_migration_marker(mtd, buf, len,&largepage_migrated);}vfree(buf);- }
- /* Now that we checked and possibly migrated badblock
* markers, continue with default bbt scanning */
/* * U-Boot multiline comment style * is like this. */
Again, thanks for working on this. To properly test it I need to get raw reads/writes working properly with eLBC, though. Once I do that, I'll fix this patch up (unless you want to do a v3 before then).
-Scott
On Thu, Jun 28, 2012 at 10:36 PM, Scott Wood scottwood@freescale.com wrote:
On 06/28/2012 03:47 PM, Rafael Beims wrote:
Freescale FCM controller has a 2K size limitation of buffer RAM. In order to support the Nand flash chip with pagesize 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. Because of this, the in flash layout of the oob is different from the default for 4096kiB page sizes. Therefore, we need to migrate the factory bad block markers from the original position to the new layout.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@freescale.com Signed-off-by: Liu Shuo b35362@freescale.com Signed-off-by: Rafael Beims rafael.beims@datacom.ind.br
Changes in v2: - Added check to disallow the migration code to run in devices with page size <= 2048
drivers/mtd/nand/fsl_elbc_nand.c | 447 +++++++++++++++++++++++++++++++++++--- 1 files changed, 419 insertions(+), 28 deletions(-)
Thanks for working on this! I've been meaning to for a while, but have been busy with other things.
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 9076ad4..3b6bb1d 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -76,15 +76,17 @@ struct fsl_elbc_ctrl {
/* device info */ fsl_lbc_t *regs;
- u8 __iomem *addr; /* Address of assigned FCM buffer */
- unsigned int page; /* Last page written to / read from */
- unsigned int read_bytes; /* Number of bytes read during command */
- unsigned int column; /* Saved column from SEQIN */
- unsigned int index; /* Pointer to next byte to 'read' */
- unsigned int status; /* status read from LTESR after last op */
- unsigned int mdr; /* UPM/FCM Data Register value */
- unsigned int use_mdr; /* Non zero if the MDR is to be set */
- unsigned int oob; /* Non zero if operating on OOB data */
- u8 __iomem *addr; /* Address of assigned FCM buffer */
- unsigned int page; /* Last page written to / read from */
- unsigned int read_bytes; /* Number of bytes read during command */
- unsigned int column; /* Saved column from SEQIN */
- unsigned int index; /* Pointer to next byte to 'read' */
- unsigned int status; /* status read from LTESR after last op */
- unsigned int mdr; /* UPM/FCM Data Register value */
- unsigned int use_mdr; /* Non zero if the MDR is to be set */
- unsigned int oob; /* Non zero if operating on OOB data */
- char *buffer; /* Just used when pagesize is greater */
- /* than FCM RAM 2K limitation */
};
/* These map to the positions used by the FCM hardware ECC generator */ @@ -131,6 +133,15 @@ static struct nand_bbt_descr largepage_memorybased = { .pattern = scan_ff_pattern, };
+static u8 migrated_pattern[] = { 0xde, 0xad, 0xde, 0xad };
Let's generate a proper random number here -- or at least a meaningful ASCII string. Things like the above are too common and invite magic number collision.
Will do it.
+static int fsl_elbc_migrate_badblocks(struct mtd_info *mtd,
- struct nand_bbt_descr *bd)
+{
- struct nand_chip *this = mtd->priv;
- int len, numblocks, i;
- int startblock;
- loff_t from;
- uint8_t *newoob, *buf;
- struct fsl_elbc_mtd *priv = this->priv;
- struct fsl_elbc_ctrl *ctrl = priv->ctrl;
- int num_subpages = mtd->writesize / 2048-1;
- len = mtd->writesize + mtd->oobsize;
- numblocks = this->chipsize >> (this->phys_erase_shift - 1);
- startblock = 0;
- from = 0;
- newoob = vmalloc(mtd->oobsize);
Even if this were Linux, oobsize should never be big enough to need vmalloc.
- memset(newoob, 0xff, mtd->writesize+mtd->oobsize);
You're writing beyond the end of that buffer.
I should have reviewed this code better... Will fix that.
- for (i = startblock; i < numblocks; i += 2) {
- int page = (from >> this->page_shift) & this->pagemask;
- fsl_elbc_cmdfunc(mtd, NAND_CMD_READ0, 0, page);
- /* As we are already using the hack to read the bytes
- * from NAND, the original badblock marker is offset
- * from its original location in the internal buffer.
- * This is because the hack reads 2048 + 64 and already
- * positions the spare in the correct region
- * (after the 4096 offset)
- */
- uint8_t *badblock_pattern = (ctrl->buffer+
- mtd->writesize-(num_subpages*64))+bd->offs;
Spaces around operators.
I think that should be (num_subpages - 1) * 64.
OK.
- if (fsl_elbc_check_pattern(mtd, badblock_pattern, bd)) {
- printf("Badblock found at %08x, migrating...\n", page);
- memcpy(newoob, badblock_pattern , bd->len);
No space before ,
+static int fsl_elbc_write_migration_marker(struct mtd_info *mtd,
- uint8_t *buf, int len, struct nand_bbt_descr *bd)
+{
- struct nand_chip *this = mtd->priv;
- struct nand_bbt_descr *td = this->bbt_td;
- int startblock;
- int dir, i;
- int blocks_to_write = 2;
- /* Start below maximum bbt */
- startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;
- dir = -1;
If you want startblock below the bbt area, I think you're off by one.
- for (i = 0; i < bd->maxblocks && blocks_to_write != 0; i++) {
- int actblock = startblock + dir*i;
- loff_t offs = (loff_t)actblock << this->phys_erase_shift;
- int page = (offs >> this->page_shift) & this->pagemask;
- /* Avoid badblocks writing the marker... */
- if (!fsl_elbc_scan_read_raw_oob(mtd, buf, page, mtd->writesize,
- &largepage_memorybased)) {
- /* We are reusing this buffer, reset it */
- memset(buf, 0xff, len);
- memcpy(buf+bd->offs, bd->pattern, bd->len);
- /* Mark the block as bad the same way that
- * it's done for the bbt. This should avoid
- * this block being overwritten
- */
- memset(buf+this->badblockpos, 0x02, 1);
- fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN,
- mtd->writesize, page);
- fsl_elbc_write_buf(mtd, buf, mtd->oobsize);
- fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
- blocks_to_write--;
- printf("Wrote migration marker to offset: %x\n", page);
- }
Why are we writing the marker twice?
Should have gone out of the loop after writing the first block. The loop is for finding a good block to write...
+static int fsl_elbc_scan_bbt(struct mtd_info *mtd) +{
- struct nand_chip *this = mtd->priv;
- struct nand_bbt_descr *bd = &largepage_migrated;
- struct nand_bbt_descr *td = this->bbt_td;
- int len;
- int startblock, block, dir;
- uint8_t *buf;
- int migrate = 0;
- if (mtd->writesize > 2048) {
- /* Start below maximum bbt */
- startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;
Again, off by one.
- dir = -1;
dir never seems to get set to anything else, so why a variable?
I think we should start with the last block, and search backwards until we find it, or until we exceed some reasonable limit, such as td->maxblocks * 2 (Is that really enough? How many contiguous bad blocks can we see?). Actually writing a joint bbt/migration marker (which was requested in earlier discussion to avoid wasting an erase block, but I don't want it to be mandatory) can come later, but we want to recognize it now.
It should be simple enough to change that and avoid starting after the last bbt block.
- /* Allocate a temporary buffer for one eraseblock incl. oob */
- len = (1 << this->phys_erase_shift);
- len += (len >> this->page_shift) * mtd->oobsize;
- buf = vmalloc(len);
This code isn't going to be shared with Linux, so just malloc(). Likewise printf(...) instead of printk(KERN_ERR ...).
BTW, should mention at least in the changelog that this is partially derived from code in nand_bbt.c. Also, using this code means we need to remove the "or later" from fsl_elbc_nand.c, because nand_bbt.c is GPL v2 only. Wolfgang probably won't be pleased. It might be better to just write it from scratch -- I'm not sure how much it really helps to be mimicking the bbt code here (without actually being able to share it), versus straightforwardly coding this specific task.
I'm not shure I follow here. Is the use of the bbt descriptor a problem? Aside from that, the code indeed is somewhat based on the code on bbt (I used nand_bbt.c extensively for example code), but much of it is just plain running through the blocks and searching for patterns. I just want to understand what needs to be different, so I can work on it.
- if (!buf) {
- printk(KERN_ERR "fsl_elbc_nand: Out of memory\n");
- kfree(this->bbt);
- this->bbt = NULL;
- return -ENOMEM;
- }
Why are we freeing the bbt here? When did we allocate it?
Right. This should not be here...
- for (block = 0; block < td->maxblocks; block++) {
- int actblock = startblock + dir * block;
- loff_t offs = (loff_t)actblock << this->phys_erase_shift;
- int page = (offs >> this->page_shift) & this->pagemask;
- migrate = fsl_elbc_scan_read_raw_oob(mtd, buf, page,
- mtd->writesize, &largepage_migrated);
- /* We found the migration marker, get out of here */
- if (migrate == 0)
- break;
- }
- if (migrate) {
- printf("Moving factory marked badblocks to new oob\n");
- fsl_elbc_migrate_badblocks(mtd, &largepage_memorybased);
- fsl_elbc_write_migration_marker(mtd, buf, len,
- &largepage_migrated);
- }
- vfree(buf);
- }
- /* Now that we checked and possibly migrated badblock
- * markers, continue with default bbt scanning */
/* * U-Boot multiline comment style * is like this. */
OK.
Again, thanks for working on this. To properly test it I need to get raw reads/writes working properly with eLBC, though. Once I do that, I'll fix this patch up (unless you want to do a v3 before then).
-Scott
No problem. I'm very happy to be able to contribute somehow. Aren't raw writes and reads working correctly with this driver? I used them also to do my tests, as I already had scrubbed my nand chip before starting this. What's the problem? I should be able to get a "pristine" nand chip installed in my test board, but this would take some time also, and being able to simulate bad blocks using raw writes is very good for development.
-- Rafael
On 06/28/2012 09:13 PM, Rafael Beims wrote:
On Thu, Jun 28, 2012 at 10:36 PM, Scott Wood scottwood@freescale.com wrote:
On 06/28/2012 03:47 PM, Rafael Beims wrote:
Freescale FCM controller has a 2K size limitation of buffer RAM. In order to support the Nand flash chip with pagesize 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. Because of this, the in flash layout of the oob is different from the default for 4096kiB page sizes. Therefore, we need to migrate the factory bad block markers from the original position to the new layout.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@freescale.com Signed-off-by: Liu Shuo b35362@freescale.com Signed-off-by: Rafael Beims rafael.beims@datacom.ind.br
Changes in v2:
- Added check to disallow the migration code to run in devices with
page size <= 2048
drivers/mtd/nand/fsl_elbc_nand.c | 447 +++++++++++++++++++++++++++++++++++--- 1 files changed, 419 insertions(+), 28 deletions(-)
Thanks for working on this! I've been meaning to for a while, but have been busy with other things.
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 9076ad4..3b6bb1d 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -76,15 +76,17 @@ struct fsl_elbc_ctrl {
/* device info */ fsl_lbc_t *regs;
u8 __iomem *addr; /* Address of assigned FCM buffer */unsigned int page; /* Last page written to / read from */unsigned int read_bytes; /* Number of bytes read during command */unsigned int column; /* Saved column from SEQIN */unsigned int index; /* Pointer to next byte to 'read' */unsigned int status; /* status read from LTESR after last op */unsigned int mdr; /* UPM/FCM Data Register value */unsigned int use_mdr; /* Non zero if the MDR is to be set */unsigned int oob; /* Non zero if operating on OOB data */
u8 __iomem *addr; /* Address of assigned FCM buffer */unsigned int page; /* Last page written to / read from */unsigned int read_bytes; /* Number of bytes read during command */unsigned int column; /* Saved column from SEQIN */unsigned int index; /* Pointer to next byte to 'read' */unsigned int status; /* status read from LTESR after last op */unsigned int mdr; /* UPM/FCM Data Register value */unsigned int use_mdr; /* Non zero if the MDR is to be set */unsigned int oob; /* Non zero if operating on OOB data */char *buffer; /* Just used when pagesize is greater *//* than FCM RAM 2K limitation */};
/* These map to the positions used by the FCM hardware ECC generator */ @@ -131,6 +133,15 @@ static struct nand_bbt_descr largepage_memorybased = { .pattern = scan_ff_pattern, };
+static u8 migrated_pattern[] = { 0xde, 0xad, 0xde, 0xad };
Let's generate a proper random number here -- or at least a meaningful ASCII string. Things like the above are too common and invite magic number collision.
Will do it.
+static int fsl_elbc_migrate_badblocks(struct mtd_info *mtd,
struct nand_bbt_descr *bd)+{
struct nand_chip *this = mtd->priv;int len, numblocks, i;int startblock;loff_t from;uint8_t *newoob, *buf;struct fsl_elbc_mtd *priv = this->priv;struct fsl_elbc_ctrl *ctrl = priv->ctrl;int num_subpages = mtd->writesize / 2048-1;len = mtd->writesize + mtd->oobsize;numblocks = this->chipsize >> (this->phys_erase_shift - 1);startblock = 0;from = 0;newoob = vmalloc(mtd->oobsize);Even if this were Linux, oobsize should never be big enough to need vmalloc.
memset(newoob, 0xff, mtd->writesize+mtd->oobsize);You're writing beyond the end of that buffer.
I should have reviewed this code better... Will fix that.
for (i = startblock; i < numblocks; i += 2) {int page = (from >> this->page_shift) & this->pagemask;fsl_elbc_cmdfunc(mtd, NAND_CMD_READ0, 0, page);/* As we are already using the hack to read the bytes* from NAND, the original badblock marker is offset* from its original location in the internal buffer.* This is because the hack reads 2048 + 64 and already* positions the spare in the correct region* (after the 4096 offset)*/uint8_t *badblock_pattern = (ctrl->buffer+mtd->writesize-(num_subpages*64))+bd->offs;Spaces around operators.
I think that should be (num_subpages - 1) * 64.
OK.
if (fsl_elbc_check_pattern(mtd, badblock_pattern, bd)) {printf("Badblock found at %08x, migrating...\n", page);memcpy(newoob, badblock_pattern , bd->len);No space before ,
+static int fsl_elbc_write_migration_marker(struct mtd_info *mtd,
uint8_t *buf, int len, struct nand_bbt_descr *bd)+{
struct nand_chip *this = mtd->priv;struct nand_bbt_descr *td = this->bbt_td;int startblock;int dir, i;int blocks_to_write = 2;/* Start below maximum bbt */startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;dir = -1;If you want startblock below the bbt area, I think you're off by one.
for (i = 0; i < bd->maxblocks && blocks_to_write != 0; i++) {int actblock = startblock + dir*i;loff_t offs = (loff_t)actblock << this->phys_erase_shift;int page = (offs >> this->page_shift) & this->pagemask;/* Avoid badblocks writing the marker... */if (!fsl_elbc_scan_read_raw_oob(mtd, buf, page, mtd->writesize,&largepage_memorybased)) {/* We are reusing this buffer, reset it */memset(buf, 0xff, len);memcpy(buf+bd->offs, bd->pattern, bd->len);/* Mark the block as bad the same way that* it's done for the bbt. This should avoid* this block being overwritten*/memset(buf+this->badblockpos, 0x02, 1);fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN,mtd->writesize, page);fsl_elbc_write_buf(mtd, buf, mtd->oobsize);fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);blocks_to_write--;printf("Wrote migration marker to offset: %x\n", page);}Why are we writing the marker twice?
Should have gone out of the loop after writing the first block. The loop is for finding a good block to write...
This raises the question of whether we should write the marker multiple times, or otherwise deal with the possibility that the marker gets bitflipped. We don't want the migration to run again, finding bogus bad block markes in what is now being used as a data area.
/* Allocate a temporary buffer for one eraseblock incl. oob */len = (1 << this->phys_erase_shift);len += (len >> this->page_shift) * mtd->oobsize;buf = vmalloc(len);This code isn't going to be shared with Linux, so just malloc(). Likewise printf(...) instead of printk(KERN_ERR ...).
BTW, should mention at least in the changelog that this is partially derived from code in nand_bbt.c. Also, using this code means we need to remove the "or later" from fsl_elbc_nand.c, because nand_bbt.c is GPL v2 only. Wolfgang probably won't be pleased. It might be better to just write it from scratch -- I'm not sure how much it really helps to be mimicking the bbt code here (without actually being able to share it), versus straightforwardly coding this specific task.
I'm not shure I follow here. Is the use of the bbt descriptor a problem? Aside from that, the code indeed is somewhat based on the code on bbt (I used nand_bbt.c extensively for example code), but much of it is just plain running through the blocks and searching for patterns. I just want to understand what needs to be different, so I can work on it.
The license on nand_bbt.c is GPLv2 only. The license on fsl_elbc_nand.c is GPLv2 or later. So if we do use nand_bbt derived code, we need to change the license on fsl_elbc_nand.c, or move this code into a different v2-only file. The only reason I suggested that it might be better to just write new code is that the bbt code seems to be an imperfect fit, and a bit more complicated than it needs to be.
Again, thanks for working on this. To properly test it I need to get raw reads/writes working properly with eLBC, though. Once I do that, I'll fix this patch up (unless you want to do a v3 before then).
-Scott
No problem. I'm very happy to be able to contribute somehow. Aren't raw writes and reads working correctly with this driver? I used them also to do my tests, as I already had scrubbed my nand chip before starting this. What's the problem?
The problem is ECC will still be performed. We need to know before we start that it will be a raw access, but currently the generic layer only lets us know by calling a different page transfer function, after issuing the command.
-Scott
On Fri, Jun 29, 2012 at 8:06 PM, Scott Wood scottwood@freescale.com wrote:
On 06/28/2012 09:13 PM, Rafael Beims wrote:
On Thu, Jun 28, 2012 at 10:36 PM, Scott Wood scottwood@freescale.com wrote:
On 06/28/2012 03:47 PM, Rafael Beims wrote:
Freescale FCM controller has a 2K size limitation of buffer RAM. In order to support the Nand flash chip with pagesize 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. Because of this, the in flash layout of the oob is different from the default for 4096kiB page sizes. Therefore, we need to migrate the factory bad block markers from the original position to the new layout.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@freescale.com Signed-off-by: Liu Shuo b35362@freescale.com Signed-off-by: Rafael Beims rafael.beims@datacom.ind.br
Changes in v2:
- Added check to disallow the migration code to run in devices with
page size <= 2048
drivers/mtd/nand/fsl_elbc_nand.c | 447 +++++++++++++++++++++++++++++++++++--- 1 files changed, 419 insertions(+), 28 deletions(-)
Thanks for working on this! I've been meaning to for a while, but have been busy with other things.
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c index 9076ad4..3b6bb1d 100644 --- a/drivers/mtd/nand/fsl_elbc_nand.c +++ b/drivers/mtd/nand/fsl_elbc_nand.c @@ -76,15 +76,17 @@ struct fsl_elbc_ctrl {
/* device info */ fsl_lbc_t *regs;
u8 __iomem *addr; /* Address of assigned FCM buffer */unsigned int page; /* Last page written to / read from */unsigned int read_bytes; /* Number of bytes read during command */unsigned int column; /* Saved column from SEQIN */unsigned int index; /* Pointer to next byte to 'read' */unsigned int status; /* status read from LTESR after last op */unsigned int mdr; /* UPM/FCM Data Register value */unsigned int use_mdr; /* Non zero if the MDR is to be set */unsigned int oob; /* Non zero if operating on OOB data */
u8 __iomem *addr; /* Address of assigned FCM buffer */unsigned int page; /* Last page written to / read from */unsigned int read_bytes; /* Number of bytes read during command */unsigned int column; /* Saved column from SEQIN */unsigned int index; /* Pointer to next byte to 'read' */unsigned int status; /* status read from LTESR after last op */unsigned int mdr; /* UPM/FCM Data Register value */unsigned int use_mdr; /* Non zero if the MDR is to be set */unsigned int oob; /* Non zero if operating on OOB data */char *buffer; /* Just used when pagesize is greater *//* than FCM RAM 2K limitation */};
/* These map to the positions used by the FCM hardware ECC generator */ @@ -131,6 +133,15 @@ static struct nand_bbt_descr largepage_memorybased = { .pattern = scan_ff_pattern, };
+static u8 migrated_pattern[] = { 0xde, 0xad, 0xde, 0xad };
Let's generate a proper random number here -- or at least a meaningful ASCII string. Things like the above are too common and invite magic number collision.
Will do it.
+static int fsl_elbc_migrate_badblocks(struct mtd_info *mtd,
struct nand_bbt_descr *bd)+{
struct nand_chip *this = mtd->priv;int len, numblocks, i;int startblock;loff_t from;uint8_t *newoob, *buf;struct fsl_elbc_mtd *priv = this->priv;struct fsl_elbc_ctrl *ctrl = priv->ctrl;int num_subpages = mtd->writesize / 2048-1;len = mtd->writesize + mtd->oobsize;numblocks = this->chipsize >> (this->phys_erase_shift - 1);startblock = 0;from = 0;newoob = vmalloc(mtd->oobsize);Even if this were Linux, oobsize should never be big enough to need vmalloc.
memset(newoob, 0xff, mtd->writesize+mtd->oobsize);You're writing beyond the end of that buffer.
I should have reviewed this code better... Will fix that.
for (i = startblock; i < numblocks; i += 2) {int page = (from >> this->page_shift) & this->pagemask;fsl_elbc_cmdfunc(mtd, NAND_CMD_READ0, 0, page);/* As we are already using the hack to read the bytes* from NAND, the original badblock marker is offset* from its original location in the internal buffer.* This is because the hack reads 2048 + 64 and already* positions the spare in the correct region* (after the 4096 offset)*/uint8_t *badblock_pattern = (ctrl->buffer+mtd->writesize-(num_subpages*64))+bd->offs;Spaces around operators.
I think that should be (num_subpages - 1) * 64.
OK.
if (fsl_elbc_check_pattern(mtd, badblock_pattern, bd)) {printf("Badblock found at %08x, migrating...\n", page);memcpy(newoob, badblock_pattern , bd->len);No space before ,
+static int fsl_elbc_write_migration_marker(struct mtd_info *mtd,
uint8_t *buf, int len, struct nand_bbt_descr *bd)+{
struct nand_chip *this = mtd->priv;struct nand_bbt_descr *td = this->bbt_td;int startblock;int dir, i;int blocks_to_write = 2;/* Start below maximum bbt */startblock = (mtd->size >> this->phys_erase_shift) - td->maxblocks;dir = -1;If you want startblock below the bbt area, I think you're off by one.
for (i = 0; i < bd->maxblocks && blocks_to_write != 0; i++) {int actblock = startblock + dir*i;loff_t offs = (loff_t)actblock << this->phys_erase_shift;int page = (offs >> this->page_shift) & this->pagemask;/* Avoid badblocks writing the marker... */if (!fsl_elbc_scan_read_raw_oob(mtd, buf, page, mtd->writesize,&largepage_memorybased)) {/* We are reusing this buffer, reset it */memset(buf, 0xff, len);memcpy(buf+bd->offs, bd->pattern, bd->len);/* Mark the block as bad the same way that* it's done for the bbt. This should avoid* this block being overwritten*/memset(buf+this->badblockpos, 0x02, 1);fsl_elbc_cmdfunc(mtd, NAND_CMD_SEQIN,mtd->writesize, page);fsl_elbc_write_buf(mtd, buf, mtd->oobsize);fsl_elbc_cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);blocks_to_write--;printf("Wrote migration marker to offset: %x\n", page);}Why are we writing the marker twice?
Should have gone out of the loop after writing the first block. The loop is for finding a good block to write...
This raises the question of whether we should write the marker multiple times, or otherwise deal with the possibility that the marker gets bitflipped. We don't want the migration to run again, finding bogus bad block markes in what is now being used as a data area.
Looking at the code again I realized that the intention was to have a backup, as you said. So I guess it would be good to write the marker more than once in other blocks for redundancy. As for the question of whether 2 blocks is enough, I think I don't have enough experience to tell that...
/* Allocate a temporary buffer for one eraseblock incl. oob */len = (1 << this->phys_erase_shift);len += (len >> this->page_shift) * mtd->oobsize;buf = vmalloc(len);This code isn't going to be shared with Linux, so just malloc(). Likewise printf(...) instead of printk(KERN_ERR ...).
BTW, should mention at least in the changelog that this is partially derived from code in nand_bbt.c. Also, using this code means we need to remove the "or later" from fsl_elbc_nand.c, because nand_bbt.c is GPL v2 only. Wolfgang probably won't be pleased. It might be better to just write it from scratch -- I'm not sure how much it really helps to be mimicking the bbt code here (without actually being able to share it), versus straightforwardly coding this specific task.
I'm not shure I follow here. Is the use of the bbt descriptor a problem? Aside from that, the code indeed is somewhat based on the code on bbt (I used nand_bbt.c extensively for example code), but much of it is just plain running through the blocks and searching for patterns. I just want to understand what needs to be different, so I can work on it.
The license on nand_bbt.c is GPLv2 only. The license on fsl_elbc_nand.c is GPLv2 or later. So if we do use nand_bbt derived code, we need to change the license on fsl_elbc_nand.c, or move this code into a different v2-only file. The only reason I suggested that it might be better to just write new code is that the bbt code seems to be an imperfect fit, and a bit more complicated than it needs to be.
I tried to remove all references to the nand_bbt code in the last version of the patch (will follow after this). I hope that what I done is enough.
Again, thanks for working on this. To properly test it I need to get raw reads/writes working properly with eLBC, though. Once I do that, I'll fix this patch up (unless you want to do a v3 before then).
-Scott
No problem. I'm very happy to be able to contribute somehow. Aren't raw writes and reads working correctly with this driver? I used them also to do my tests, as I already had scrubbed my nand chip before starting this. What's the problem?
The problem is ECC will still be performed. We need to know before we start that it will be a raw access, but currently the generic layer only lets us know by calling a different page transfer function, after issuing the command.
-Scott
But at least we can be sure that we will be able to write directly to the oob, and with the exception of the bytes controlled directly by ECC code (or HW), I think we can make tests. Anyway, that is the way I tested the code until now. In the following I will try to test it with a new chip that has not been tampered in any way to see if the migration detects and moves the blocks correctly.
-- Rafael
On 07/02/2012 04:00 PM, Rafael Beims wrote:
On Fri, Jun 29, 2012 at 8:06 PM, Scott Wood scottwood@freescale.com wrote:
This raises the question of whether we should write the marker multiple times, or otherwise deal with the possibility that the marker gets bitflipped. We don't want the migration to run again, finding bogus bad block markes in what is now being used as a data area.
Looking at the code again I realized that the intention was to have a backup, as you said. So I guess it would be good to write the marker more than once in other blocks for redundancy. As for the question of whether 2 blocks is enough, I think I don't have enough experience to tell that...
Regardless of how many extras we have, if we find one but not all of the markers intact, we should repair it... which means we really need to dedicate a block to this.
Simplest would be to write the marker in all pages of the block, if we're consuming the entire block anyway.
Of course, to continue down the path of paranoia, what if we lose power during said repair, after erasing but before writing a marker? Make it two blocks. :-(
/* Allocate a temporary buffer for one eraseblock incl. oob */len = (1 << this->phys_erase_shift);len += (len >> this->page_shift) * mtd->oobsize;buf = vmalloc(len);This code isn't going to be shared with Linux, so just malloc(). Likewise printf(...) instead of printk(KERN_ERR ...).
BTW, should mention at least in the changelog that this is partially derived from code in nand_bbt.c. Also, using this code means we need to remove the "or later" from fsl_elbc_nand.c, because nand_bbt.c is GPL v2 only. Wolfgang probably won't be pleased. It might be better to just write it from scratch -- I'm not sure how much it really helps to be mimicking the bbt code here (without actually being able to share it), versus straightforwardly coding this specific task.
I'm not shure I follow here. Is the use of the bbt descriptor a problem? Aside from that, the code indeed is somewhat based on the code on bbt (I used nand_bbt.c extensively for example code), but much of it is just plain running through the blocks and searching for patterns. I just want to understand what needs to be different, so I can work on it.
The license on nand_bbt.c is GPLv2 only. The license on fsl_elbc_nand.c is GPLv2 or later. So if we do use nand_bbt derived code, we need to change the license on fsl_elbc_nand.c, or move this code into a different v2-only file. The only reason I suggested that it might be better to just write new code is that the bbt code seems to be an imperfect fit, and a bit more complicated than it needs to be.
I tried to remove all references to the nand_bbt code in the last version of the patch (will follow after this). I hope that what I done is enough.
It needs to be not derived from the nand_bbt code if we're going to keep v2-or-later -- not just references removed.
-Scott
participants (2)
-
Rafael Beims -
Scott Wood