lkml.org 
[lkml]   [2018]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 1/3] mtd: rawnand: Support bad block markers in first, second or last page
Hi Frieder,

On Thu, 20 Dec 2018 14:35:05 +0000
Schrempf Frieder <frieder.schrempf@kontron.de> wrote:

> On 20.12.18 14:59, Boris Brezillon wrote:
> > On Mon, 17 Dec 2018 15:49:07 +0000
> > Schrempf Frieder <frieder.schrempf@kontron.de> wrote:
> >
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> Currently supported bad block marker positions within the block are:
> >> * in first page only
> >> * in last page only
> >> * in first or second page
> >>
> >> Some ESMT NANDs are known to have been shipped by the manufacturer
> >> with bad block markers in the first or last page, instead of the
> >> first or second page.
> >>
> >> Also the datasheets for Cypress/Spansion/AMD NANDs claim that the
> >> first, second *and* last page needs to be checked.
> >>
> >> Therefore we make it possible to set NAND_BBT_SCAN2NDPAGE and
> >> NAND_BBT_SCANLASTPAGE at the same time to scan/set all three pages.
> >>
> >> To simplify the code, the logic to evaluate the flags is moved to a
> >> a new function nand_bbm_page_offset().
> >>
> >> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> >> ---
> >> drivers/mtd/nand/raw/internals.h | 1 +
> >> drivers/mtd/nand/raw/nand_base.c | 72 ++++++++++++++++++++++++++---------
> >> drivers/mtd/nand/raw/nand_bbt.c | 30 +++++++--------
> >> 3 files changed, 68 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> >> index 04c2cf7..8e4b168 100644
> >> --- a/drivers/mtd/nand/raw/internals.h
> >> +++ b/drivers/mtd/nand/raw/internals.h
> >> @@ -76,6 +76,7 @@ extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
> >>
> >> /* Core functions */
> >> const struct nand_manufacturer *nand_get_manufacturer(u8 id);
> >> +int nand_bbm_page_offset(struct nand_chip *chip, int index);
> >> int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
> >> int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
> >> int allowbbt);
> >> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> >> index 71050a0..388d9ed 100644
> >> --- a/drivers/mtd/nand/raw/nand_base.c
> >> +++ b/drivers/mtd/nand/raw/nand_base.c
> >> @@ -253,6 +253,45 @@ static void nand_release_device(struct mtd_info *mtd)
> >> }
> >>
> >> /**
> >> + * nand_bbm_page_offset - Get the page offsets for bad block markers
> >> + * @chip: NAND chip object
> >> + * @index: Index for the page offset
> >
> > Hm, the meaning of index is far from obvious. How about passing the
> > current page instead (and return 1 if there are more pages to scan 0
> > otherwise)?
>
> Good idea.
>
> >
> > Something like:
> >
> > static int nand_bbm_get_next_page(struct nand_chip *chip, int page)
> > {
> > struct mtd_info *mtd = nand_to_mtd(chip);
> > int last_page = ((mtd->erasesize - mtd->writesize) >>
> > chip->page_shift) & chip->pagemask;
> >
> > if (page < 0 && chip->bbt_options & NAND_BBT_SCANFIRSTPAGE)
> > return 0;
> > else if (page < 1 && chip->bbt_options & NAND_BBT_SCAN2NDPAGE)
> > return 1;
> > else if (page < last_page &&
> > chip->bbt_options & NAND_BBT_SCANLASTPAGE)
> > return last_page;
> >
> > return -1;
> > }
> >
> > And yes, that means defining NAND_BBT_SCANFIRSTPAGE and setting it when
> > appropriate.
>
> I tried to keep the existing flags and their current meanings, but you
> are right. If we redefine the flags and add NAND_BBT_SCANFIRSTPAGE and
> NAND_BBT_SCANFIRST2PAGES this will be much easier to read.
>
> Also maybe renaming the flags to NAND_BBM_XXX would be even cleaner, as
> we use them not only for scanning, but also for writing markers and they
> are not directly related to the bad block table (BBT)?

Yep, and maybe move them to chip->options too.

>
> By the way, what are your plans for using the common NAND layer (that is
> used by the SPI NAND layer) for raw NAND?

I'd still like to have this done at some point, just don't have the
time to do it myself ;-). I started working on that a few weeks back
[1], but didn't have time to finish it.

> I'm thinking of SPI NANDs that might require things like this, too.

Yes, probably.

> Currently they seem to have the markers in the first page only, but that
> could change easily and in that case it would be nice to share the code.
Yes. Actually, that's the whole BBT + BBM scanning logic we should make
generic. But I'd like to take this as an opportunity to
cleanup/simplify the bbt code instead of simply porting it to the
generic NAND layer.

If you have some time, feel free to finish what I started.

Regards,

Boris

[1]https://github.com/bbrezillon/linux/commits/nand/cleanup

\
 
 \ /
  Last update: 2018-12-20 16:42    [W:0.064 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site