Re: [PATCH 01/10] mtd: set the cell information for ONFI nand

[Brian Norris <computersforpeace@gmail.com>]

On Mon, Aug 12, 2013 at 07:22:38AM +0000, Gupta, Pekon wrote:
> > The current code does not set the SLC/MLC information for onfi nand.
> > (This makes that the kernel treats all the onfi nand as SLC nand.)
> > 
> > This patch fills the chip->cellinfo when the onfi nand is a MLC(or TLC) nand
> > (p->bits_per_cell > 1).
> > 
> > The macro NAND_CI_CELLTYPE_SHIFT is added to avoid the hardcode.
> > 
> > Signed-off-by: Huang Shijie <b32955@freescale.com>
> > ---
> >  drivers/mtd/nand/nand_base.c |    3 +++
> >  include/linux/mtd/nand.h     |    1 +
> >  2 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index ff605c8..ee1aa52 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2988,6 +2988,9 @@ static int nand_flash_detect_onfi(struct mtd_info
> > *mtd, struct nand_chip *chip,
> >  	chip->chipsize = le32_to_cpu(p->blocks_per_lun);
> >  	chip->chipsize *= (uint64_t)mtd->erasesize * p->lun_count;
> > 
> > +	/* @bits_per_cell equals 1 means this is a SLC nand. */
> > +	chip->cellinfo = (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;
> > +
> [Pekon]: For future scalability, good to update only MLC related bit-fields
> So ORing instead of assigning..
> chip->cellinfo |= (p->bits_per_cell - 1) << NAND_CI_CELLTYPE_SHIFT;

I was thinking of an alternate approach: since nand_chip.cellinfo is
only used for checking SLC vs. MLC (and it is admittedly bad at that,
currently), we should modify it so that is a reliable source of *only* 1
piece of information -- the number of bits per cell. Currently, it
contains unused (and potentially unmaintainable) information for some
chips about number of simultaneously-programmed pages, write caching,
internal chip numbering, etc.

So personally, I would rename cellinfo to bits_per_cell and make sure
it is set properly. That is, for the legacy chips, make sure we
initialize it 1 (SLC); for extended-ID chips, make sure the third ID
byte has the correct info (you can refer to [1]) and set it with
something like this:

  chip->bits_per_cell = id_data[2] & NAND_CI_CELLTYPE_MSK
  chip->bits_per_cell >>= NAND_CI_CELLTYPE_SHIFT;
  chip->bits_per_cell += 1;

for chips listed by full-ID, add an appropriate flag/field; and for ONFI
chips, just use p->bits_per_cell.

If you really need the other cellinfo fields in the future, we can add
more fields to nand_chip.

> 
> >  	if (onfi_feature(chip) & ONFI_FEATURE_16_BIT_BUS)
> >  		*busw = NAND_BUSWIDTH_16;
> >  	else

Thanks,
Brian

[1] An incomplete NAND ID table:

  http://www.linux-mtd.infradead.org/nand-data/nanddata.html