| Message ID | 1489513548-3248-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
Hi Boris, 2017-03-15 16:55 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>: > On Wed, 15 Mar 2017 09:55:13 +0900 > Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > >> Hi Boris, >> >> Thanks for your review. >> >> 2017-03-15 5:58 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>: >> > On Wed, 15 Mar 2017 02:45:48 +0900 >> > Masahiro Yamada <yamada.masahiro@socionext.com> wrote: >> > >> >> The nand_default_block_markbad() is the default implementation of >> >> chip->block_markbad(). This is called for marking a block as bad. >> >> It invokes nand_do_write_oob(), then calls a higher level accessor >> >> ecc->write_oob(). >> >> >> >> On the other hand, when reading BBM from the OOB, chip->block_bad() >> >> is called, and nand_block_bad() is the default implementation. This >> >> function calls a lower level chip->cmdfunc(). If a driver wants to >> >> re-use nand_block_bad(), it is required to support NAND_CMD_READOOB >> >> in its cmdfunc(). I just noticed duplicated efforts for reading BBM. When creating BBT at initialization, functions are called as follows: check_create() create_bbt() scan_block_fast() scan_block_fast() calls high-level API mtd_read_oob() to check BBM. On the other hand, we have nand_block_bad() implemented with lower API. Perhaps, we can merge them. So, do you want to align to the scan_block_fast approach (high level API)? -- Best Regards Masahiro Yamada ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index a3c0f47..78c5cd6 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -354,9 +354,9 @@ static void nand_read_buf16(struct mtd_info *mtd, uint8_t *buf, int len) */ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs) { - int page, res = 0, i = 0; + int page, res = 0, ret = 0, i = 0; struct nand_chip *chip = mtd_to_nand(mtd); - u16 bad; + u8 bad; if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) ofs += mtd->erasesize - mtd->writesize; @@ -364,30 +364,26 @@ static int nand_block_bad(struct mtd_info *mtd, loff_t ofs) page = (int)(ofs >> chip->page_shift) & chip->pagemask; do { - if (chip->options & NAND_BUSWIDTH_16) { - chip->cmdfunc(mtd, NAND_CMD_READOOB, - chip->badblockpos & 0xFE, page); - bad = cpu_to_le16(chip->read_word(mtd)); - if (chip->badblockpos & 0x1) - bad >>= 8; + res = chip->ecc.read_oob(mtd, chip, page); + if (!res) { + bad = chip->oob_poi[chip->badblockpos]; + + if (likely(chip->badblockbits == 8)) + res = bad != 0xFF; else - bad &= 0xFF; - } else { - chip->cmdfunc(mtd, NAND_CMD_READOOB, chip->badblockpos, - page); - bad = chip->read_byte(mtd); + res = hweight8(bad) < chip->badblockbits; + if (res) + return res; } - if (likely(chip->badblockbits == 8)) - res = bad != 0xFF; - else - res = hweight8(bad) < chip->badblockbits; - ofs += mtd->writesize; - page = (int)(ofs >> chip->page_shift) & chip->pagemask; + if (!ret) + ret = res; + + page++; i++; - } while (!res && i < 2 && (chip->bbt_options & NAND_BBT_SCAN2NDPAGE)); + } while (chip->bbt_options & NAND_BBT_SCAN2NDPAGE && i < 2); - return res; + return ret; } /**
The nand_default_block_markbad() is the default implementation of chip->block_markbad(). This is called for marking a block as bad. It invokes nand_do_write_oob(), then calls a higher level accessor ecc->write_oob(). On the other hand, when reading BBM from the OOB, chip->block_bad() is called, and nand_block_bad() is the default implementation. This function calls a lower level chip->cmdfunc(). If a driver wants to re-use nand_block_bad(), it is required to support NAND_CMD_READOOB in its cmdfunc(). This is strange. If the controller supports optimized read operation and the driver has its own ecc->read_oob(), it should be able to use it. Besides, NAND_CMD_READOOB (0x50) is not a real command for large page devices. So, recent drivers may not be happy to handle this command. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- At first, I tried to call nand_do_read_oob() from nand_block_bad() in order to make to make things symmetry, but it did not work. chip->select_chip() is handled outside of nand_block_bad(), so nand_do_read_oob() can not be used there. drivers/mtd/nand/nand_base.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) -- 2.7.4 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/