Message ID | 20190130130111.32724-2-frieder.schrempf@kontron.de |
---|---|
State | Changes Requested |
Headers | show |
Series | mtd: rawnand: Support bad block markers in first, second or last page | expand |
On Wed, 30 Jan 2019 13:01:44 +0000 Schrempf Frieder <frieder.schrempf@kontron.de> wrote: > From: Frieder Schrempf <frieder.schrempf@kontron.de> > > The information about where the manufacturer puts the bad block > markers inside the bad block and in the OOB data is stored in > different places. Let's move this information to nand_chip.options > and nand_chip.badblockpos. > > As this chip-specific information is not directly related to the > bad block table (BBT), we also rename the flags to NAND_BBM_*. > > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> > --- > drivers/mtd/nand/onenand/onenand_base.c | 2 +- > drivers/mtd/nand/onenand/onenand_bbt.c | 2 +- > drivers/mtd/nand/raw/nand_amd.c | 2 +- > drivers/mtd/nand/raw/nand_base.c | 12 ++++++------ > drivers/mtd/nand/raw/nand_bbt.c | 4 ++-- > drivers/mtd/nand/raw/nand_esmt.c | 2 +- > drivers/mtd/nand/raw/nand_hynix.c | 4 ++-- > drivers/mtd/nand/raw/nand_macronix.c | 2 +- > drivers/mtd/nand/raw/nand_micron.c | 2 +- > drivers/mtd/nand/raw/nand_samsung.c | 4 ++-- > drivers/mtd/nand/raw/nand_toshiba.c | 2 +- > drivers/mtd/nand/raw/sh_flctl.c | 4 ++-- > include/linux/mtd/bbm.h | 14 +------------- > include/linux/mtd/rawnand.h | 15 +++++++++++++++ Might be better to split that in 2 patches: - update onenand logic - update rawnand logic > 14 files changed, 37 insertions(+), 34 deletions(-) > > diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c > index 4ca4b194e7d7..d6701b8f031f 100644 > --- a/drivers/mtd/nand/onenand/onenand_base.c > +++ b/drivers/mtd/nand/onenand/onenand_base.c > @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) > bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); > > /* We write two bytes, so we don't have to mess with 16-bit access */ > - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); > + ofs += mtd->oobsize + (this->badblockpos & ~0x01); Looks like the ->badblockpos field does not exist in struct onenand_chip, which means you didn't compile test this part ;-). > /* FIXME : What to do when marking SLC block in partition > * with MLC erasesize? For now, it is not advisable to > * create partitions containing both SLC and MLC regions. > diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c > index dde20487937d..880b0abd36c8 100644 > --- a/drivers/mtd/nand/onenand/onenand_bbt.c > +++ b/drivers/mtd/nand/onenand/onenand_bbt.c > @@ -191,7 +191,7 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) > return -ENOMEM; > > /* Set the bad block position */ > - bbm->badblockpos = ONENAND_BADBLOCK_POS; > + this->badblockpos = NAND_BBM_POS_ONENAND; This should be done in onenand_base.c not onenand_bbt.c. > > /* Set erase shift */ > bbm->bbt_erase_shift = this->erase_shift; ... > diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c > index 1b722fe9213c..862eaa3a0552 100644 > --- a/drivers/mtd/nand/raw/nand_bbt.c > +++ b/drivers/mtd/nand/raw/nand_bbt.c > @@ -466,7 +466,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, > > pr_info("Scanning device for bad blocks\n"); > > - if (bd->options & NAND_BBT_SCAN2NDPAGE) > + if (this->options & NAND_BBM_SECONDPAGE) > numpages = 2; > else > numpages = 1; > @@ -487,7 +487,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, > from = (loff_t)startblock << this->bbt_erase_shift; > } > > - if (this->bbt_options & NAND_BBT_SCANLASTPAGE) > + if (this->bbt_options & NAND_BBM_LASTPAGE) You're not checking the right field here: if (this->options & NAND_BBM_LASTPAGE) > from += mtd->erasesize - (mtd->writesize * numpages); > > for (i = startblock; i < numblocks; i++) {
Hi Boris, Thanks for reviewing. On 30.01.19 14:28, Boris Brezillon wrote: > On Wed, 30 Jan 2019 13:01:44 +0000 > Schrempf Frieder <frieder.schrempf@kontron.de> wrote: > >> From: Frieder Schrempf <frieder.schrempf@kontron.de> >> >> The information about where the manufacturer puts the bad block >> markers inside the bad block and in the OOB data is stored in >> different places. Let's move this information to nand_chip.options >> and nand_chip.badblockpos. >> >> As this chip-specific information is not directly related to the >> bad block table (BBT), we also rename the flags to NAND_BBM_*. >> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de> >> --- >> drivers/mtd/nand/onenand/onenand_base.c | 2 +- >> drivers/mtd/nand/onenand/onenand_bbt.c | 2 +- >> drivers/mtd/nand/raw/nand_amd.c | 2 +- >> drivers/mtd/nand/raw/nand_base.c | 12 ++++++------ >> drivers/mtd/nand/raw/nand_bbt.c | 4 ++-- >> drivers/mtd/nand/raw/nand_esmt.c | 2 +- >> drivers/mtd/nand/raw/nand_hynix.c | 4 ++-- >> drivers/mtd/nand/raw/nand_macronix.c | 2 +- >> drivers/mtd/nand/raw/nand_micron.c | 2 +- >> drivers/mtd/nand/raw/nand_samsung.c | 4 ++-- >> drivers/mtd/nand/raw/nand_toshiba.c | 2 +- >> drivers/mtd/nand/raw/sh_flctl.c | 4 ++-- >> include/linux/mtd/bbm.h | 14 +------------- >> include/linux/mtd/rawnand.h | 15 +++++++++++++++ > > Might be better to split that in 2 patches: > - update onenand logic > - update rawnand logic Ok > >> 14 files changed, 37 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c >> index 4ca4b194e7d7..d6701b8f031f 100644 >> --- a/drivers/mtd/nand/onenand/onenand_base.c >> +++ b/drivers/mtd/nand/onenand/onenand_base.c >> @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) >> bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); >> >> /* We write two bytes, so we don't have to mess with 16-bit access */ >> - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); >> + ofs += mtd->oobsize + (this->badblockpos & ~0x01); > > Looks like the ->badblockpos field does not exist in struct > onenand_chip, which means you didn't compile test this part ;-). Hm, indeed. I forgot to enable onenand in my config. I will add the field to the onenand_chip struct, to be consistent with rawnand. > >> /* FIXME : What to do when marking SLC block in partition >> * with MLC erasesize? For now, it is not advisable to >> * create partitions containing both SLC and MLC regions. >> diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c >> index dde20487937d..880b0abd36c8 100644 >> --- a/drivers/mtd/nand/onenand/onenand_bbt.c >> +++ b/drivers/mtd/nand/onenand/onenand_bbt.c >> @@ -191,7 +191,7 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) >> return -ENOMEM; >> >> /* Set the bad block position */ >> - bbm->badblockpos = ONENAND_BADBLOCK_POS; >> + this->badblockpos = NAND_BBM_POS_ONENAND; > > This should be done in onenand_base.c not onenand_bbt.c. Ok. > >> >> /* Set erase shift */ >> bbm->bbt_erase_shift = this->erase_shift; > > ... > >> diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c >> index 1b722fe9213c..862eaa3a0552 100644 >> --- a/drivers/mtd/nand/raw/nand_bbt.c >> +++ b/drivers/mtd/nand/raw/nand_bbt.c >> @@ -466,7 +466,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, >> >> pr_info("Scanning device for bad blocks\n"); >> >> - if (bd->options & NAND_BBT_SCAN2NDPAGE) >> + if (this->options & NAND_BBM_SECONDPAGE) >> numpages = 2; >> else >> numpages = 1; >> @@ -487,7 +487,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, >> from = (loff_t)startblock << this->bbt_erase_shift; >> } >> >> - if (this->bbt_options & NAND_BBT_SCANLASTPAGE) >> + if (this->bbt_options & NAND_BBM_LASTPAGE) > > You're not checking the right field here: > > if (this->options & NAND_BBM_LASTPAGE) I will fix this. Regards, Frieder > >> from += mtd->erasesize - (mtd->writesize * numpages); >> >> for (i = startblock; i < numblocks; i++) {
diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c index 4ca4b194e7d7..d6701b8f031f 100644 --- a/drivers/mtd/nand/onenand/onenand_base.c +++ b/drivers/mtd/nand/onenand/onenand_base.c @@ -2458,7 +2458,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs) bbm->bbt[block >> 2] |= 0x01 << ((block & 0x03) << 1); /* We write two bytes, so we don't have to mess with 16-bit access */ - ofs += mtd->oobsize + (bbm->badblockpos & ~0x01); + ofs += mtd->oobsize + (this->badblockpos & ~0x01); /* FIXME : What to do when marking SLC block in partition * with MLC erasesize? For now, it is not advisable to * create partitions containing both SLC and MLC regions. diff --git a/drivers/mtd/nand/onenand/onenand_bbt.c b/drivers/mtd/nand/onenand/onenand_bbt.c index dde20487937d..880b0abd36c8 100644 --- a/drivers/mtd/nand/onenand/onenand_bbt.c +++ b/drivers/mtd/nand/onenand/onenand_bbt.c @@ -191,7 +191,7 @@ static int onenand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd) return -ENOMEM; /* Set the bad block position */ - bbm->badblockpos = ONENAND_BADBLOCK_POS; + this->badblockpos = NAND_BBM_POS_ONENAND; /* Set erase shift */ bbm->bbt_erase_shift = this->erase_shift; diff --git a/drivers/mtd/nand/raw/nand_amd.c b/drivers/mtd/nand/raw/nand_amd.c index 890c5b43e03c..6e248bdcc797 100644 --- a/drivers/mtd/nand/raw/nand_amd.c +++ b/drivers/mtd/nand/raw/nand_amd.c @@ -40,7 +40,7 @@ static void amd_nand_decode_id(struct nand_chip *chip) static int amd_nand_init(struct nand_chip *chip) { if (nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; return 0; } diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index cca4b24d2ffa..c6199cc68d2f 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -298,11 +298,11 @@ static int nand_block_bad(struct nand_chip *chip, loff_t ofs) int page, page_end, res; u8 bad; - if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) + if (chip->options & NAND_BBM_LASTPAGE) ofs += mtd->erasesize - mtd->writesize; page = (int)(ofs >> chip->page_shift) & chip->pagemask; - page_end = page + (chip->bbt_options & NAND_BBT_SCAN2NDPAGE ? 2 : 1); + page_end = page + ((chip->options & NAND_BBM_SECONDPAGE) ? 2 : 1); for (; page < page_end; page++) { res = chip->ecc.read_oob(chip, page); @@ -541,7 +541,7 @@ static int nand_default_block_markbad(struct nand_chip *chip, loff_t ofs) ops.mode = MTD_OPS_PLACE_OOB; /* Write to first/last page(s) if necessary */ - if (chip->bbt_options & NAND_BBT_SCANLASTPAGE) + if (chip->options & NAND_BBM_LASTPAGE) ofs += mtd->erasesize - mtd->writesize; do { res = nand_do_write_oob(chip, ofs, &ops); @@ -550,7 +550,7 @@ static int nand_default_block_markbad(struct nand_chip *chip, loff_t ofs) i++; ofs += mtd->writesize; - } while ((chip->bbt_options & NAND_BBT_SCAN2NDPAGE) && i < 2); + } while ((chip->options & NAND_BBM_SECONDPAGE) && i < 2); return ret; } @@ -4576,9 +4576,9 @@ static void nand_decode_bbm_options(struct nand_chip *chip) /* Set the bad block position */ if (mtd->writesize > 512 || (chip->options & NAND_BUSWIDTH_16)) - chip->badblockpos = NAND_LARGE_BADBLOCK_POS; + chip->badblockpos = NAND_BBM_POS_LARGE; else - chip->badblockpos = NAND_SMALL_BADBLOCK_POS; + chip->badblockpos = NAND_BBM_POS_SMALL; } static inline bool is_full_id_nand(struct nand_flash_dev *type) diff --git a/drivers/mtd/nand/raw/nand_bbt.c b/drivers/mtd/nand/raw/nand_bbt.c index 1b722fe9213c..862eaa3a0552 100644 --- a/drivers/mtd/nand/raw/nand_bbt.c +++ b/drivers/mtd/nand/raw/nand_bbt.c @@ -466,7 +466,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, pr_info("Scanning device for bad blocks\n"); - if (bd->options & NAND_BBT_SCAN2NDPAGE) + if (this->options & NAND_BBM_SECONDPAGE) numpages = 2; else numpages = 1; @@ -487,7 +487,7 @@ static int create_bbt(struct nand_chip *this, uint8_t *buf, from = (loff_t)startblock << this->bbt_erase_shift; } - if (this->bbt_options & NAND_BBT_SCANLASTPAGE) + if (this->bbt_options & NAND_BBM_LASTPAGE) from += mtd->erasesize - (mtd->writesize * numpages); for (i = startblock; i < numblocks; i++) { diff --git a/drivers/mtd/nand/raw/nand_esmt.c b/drivers/mtd/nand/raw/nand_esmt.c index 96f039a83bc8..80d2d3a1b5f2 100644 --- a/drivers/mtd/nand/raw/nand_esmt.c +++ b/drivers/mtd/nand/raw/nand_esmt.c @@ -36,7 +36,7 @@ static void esmt_nand_decode_id(struct nand_chip *chip) static int esmt_nand_init(struct nand_chip *chip) { if (nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; return 0; } diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c index 343f477362d1..2789bf3857c9 100644 --- a/drivers/mtd/nand/raw/nand_hynix.c +++ b/drivers/mtd/nand/raw/nand_hynix.c @@ -672,9 +672,9 @@ static int hynix_nand_init(struct nand_chip *chip) int ret; if (!nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCANLASTPAGE; + chip->options |= NAND_BBM_LASTPAGE; else - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; hynix = kzalloc(sizeof(*hynix), GFP_KERNEL); if (!hynix) diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c index 47d8cda547cf..6db7ced4b96b 100644 --- a/drivers/mtd/nand/raw/nand_macronix.c +++ b/drivers/mtd/nand/raw/nand_macronix.c @@ -62,7 +62,7 @@ static void macronix_nand_fix_broken_get_timings(struct nand_chip *chip) static int macronix_nand_init(struct nand_chip *chip) { if (nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; macronix_nand_fix_broken_get_timings(chip); diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c index b85e1c13b79e..ea6b6e80d6f8 100644 --- a/drivers/mtd/nand/raw/nand_micron.c +++ b/drivers/mtd/nand/raw/nand_micron.c @@ -448,7 +448,7 @@ static int micron_nand_init(struct nand_chip *chip) goto err_free_manuf_data; if (mtd->writesize == 2048) - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; ondie = micron_supports_on_die_ecc(chip); diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c index e46d4c492ad8..3be737139210 100644 --- a/drivers/mtd/nand/raw/nand_samsung.c +++ b/drivers/mtd/nand/raw/nand_samsung.c @@ -121,9 +121,9 @@ static int samsung_nand_init(struct nand_chip *chip) chip->options |= NAND_SAMSUNG_LP_OPTIONS; if (!nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCANLASTPAGE; + chip->options |= NAND_BBM_LASTPAGE; else - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; return 0; } diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c index d068163b64b3..222a66bf0aa6 100644 --- a/drivers/mtd/nand/raw/nand_toshiba.c +++ b/drivers/mtd/nand/raw/nand_toshiba.c @@ -147,7 +147,7 @@ static void toshiba_nand_decode_id(struct nand_chip *chip) static int toshiba_nand_init(struct nand_chip *chip) { if (nand_is_slc(chip)) - chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; + chip->options |= NAND_BBM_SECONDPAGE; /* Check that chip is BENAND and ECC mode is on-die */ if (nand_is_slc(chip) && chip->ecc.mode == NAND_ECC_ON_DIE && diff --git a/drivers/mtd/nand/raw/sh_flctl.c b/drivers/mtd/nand/raw/sh_flctl.c index cf6b1be1cf9c..2017b00fadd8 100644 --- a/drivers/mtd/nand/raw/sh_flctl.c +++ b/drivers/mtd/nand/raw/sh_flctl.c @@ -101,14 +101,12 @@ static const struct mtd_ooblayout_ops flctl_4secc_oob_largepage_ops = { static uint8_t scan_ff_pattern[] = { 0xff, 0xff }; static struct nand_bbt_descr flctl_4secc_smallpage = { - .options = NAND_BBT_SCAN2NDPAGE, .offs = 11, .len = 1, .pattern = scan_ff_pattern, }; static struct nand_bbt_descr flctl_4secc_largepage = { - .options = NAND_BBT_SCAN2NDPAGE, .offs = 0, .len = 2, .pattern = scan_ff_pattern, @@ -1178,6 +1176,8 @@ static int flctl_probe(struct platform_device *pdev) if (pdata->flcmncr_val & SEL_16BIT) nand->options |= NAND_BUSWIDTH_16; + nand->options |= NAND_BBM_SECONDPAGE; + pm_runtime_enable(&pdev->dev); pm_runtime_resume(&pdev->dev); diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h index 3102bd754d18..010bc5544c54 100644 --- a/include/linux/mtd/bbm.h +++ b/include/linux/mtd/bbm.h @@ -93,10 +93,7 @@ struct nand_bbt_descr { #define NAND_BBT_WRITE 0x00002000 /* Read and write back block contents when writing bbt */ #define NAND_BBT_SAVECONTENT 0x00004000 -/* Search good / bad pattern on the first and the second page */ -#define NAND_BBT_SCAN2NDPAGE 0x00008000 -/* Search good / bad pattern on the last page of the eraseblock */ -#define NAND_BBT_SCANLASTPAGE 0x00010000 + /* * Use a flash based bad block table. By default, OOB identifier is saved in * OOB area. This option is passed to the default bad block table function. @@ -123,13 +120,6 @@ struct nand_bbt_descr { /* The maximum number of blocks to scan for a bbt */ #define NAND_BBT_SCAN_MAXBLOCKS 4 -/* - * Constants for oob configuration - */ -#define NAND_SMALL_BADBLOCK_POS 5 -#define NAND_LARGE_BADBLOCK_POS 0 -#define ONENAND_BADBLOCK_POS 0 - /* * Bad block scanning errors */ @@ -140,7 +130,6 @@ struct nand_bbt_descr { /** * struct bbm_info - [GENERIC] Bad Block Table data structure * @bbt_erase_shift: [INTERN] number of address bits in a bbt entry - * @badblockpos: [INTERN] position of the bad block marker in the oob area * @options: options for this descriptor * @bbt: [INTERN] bad block table pointer * @isbad_bbt: function to determine if a block is bad @@ -150,7 +139,6 @@ struct nand_bbt_descr { */ struct bbm_info { int bbt_erase_shift; - int badblockpos; int options; uint8_t *bbt; diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 33e240acdc6d..da785d92e26e 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -169,6 +169,21 @@ enum nand_ecc_algo { /* Macros to identify the above */ #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ)) +/* + * There are different places where the manufacturer stores the factory bad + * block markers. + * + * Position within the block: Each of these pages needs to be checked for a + * bad block marking pattern. + */ +#define NAND_BBM_SECONDPAGE 0x02000000 +#define NAND_BBM_LASTPAGE 0x04000000 + +/* Position within the OOB data of the page */ +#define NAND_BBM_POS_SMALL 5 +#define NAND_BBM_POS_LARGE 0 +#define NAND_BBM_POS_ONENAND 0 + /* Non chip related options */ /* This option skips the bbt scan during initialization. */ #define NAND_SKIP_BBTSCAN 0x00010000