Re: [PATCH RESEND] mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

[Marek Vasut <marex@denx.de>]

Dear Matthieu CASTET,

> Hi,
> 
> for ONFI flash (like this micron one) the information should be extracted
> form the ONFI table (programs_per_page IIRC)
> 
> This should be better than relying on the SOC driver for setting this
> flags.
> 
> Does the gpmi driver set this flag because it do not support partial write
> ?

Yes

> In this case why it doesn't set  chip->ecc.steps to 1 ?

Can you elabore how exactly will that help please?

> Matthieu
> 
> Brian Norris a écrit :
> > The NAND_CHIPOPTIONS_MSK has limited utility and is causing real bugs. It
> > silently masks off at least one flag that might be set by the driver
> > (NAND_NO_SUBPAGE_WRITE). This breaks the GPMI NAND driver and possibly
> > others.
> > 
> > Really, as long as driver writers exercise a small amount of care with
> > NAND_* options, this mask is not necessary at all. Thus, kill it.
> > 
> >>From Huang Shijie:
> > "I tested this patch on imx6q-arm2 board with Micron MT29F32G08QAA.
> > it works fine, thanks."
> > 
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> > Tested-by: Huang Shijie <shijie8@gmail.com>
> > Cc: Marek Vasut <marex@denx.de>
> > ---
> > Hello Artem/David,
> > 
> > GPMI NAND has needed this patch for some time, and I think it was agreed
> > on by a few. I'm resending to get acknowledgment from a maintainer.
> > 
> >  drivers/mtd/nand/nand_base.c | 7 ++-----
> >  include/linux/mtd/nand.h     | 3 ---
> >  2 files changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index ead301a..996cc48 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2909,8 +2909,6 @@ static int nand_flash_detect_onfi(struct mtd_info
> > *mtd, struct nand_chip *chip,
> > 
> >  	if (le16_to_cpu(p->features) & 1)
> >  	
> >  		*busw = NAND_BUSWIDTH_16;
> > 
> > -	chip->options &= ~NAND_CHIPOPTIONS_MSK;
> > -
> > 
> >  	pr_info("ONFI flash detected\n");
> >  	return 1;
> >  
> >  }
> > 
> > @@ -3074,9 +3072,8 @@ static struct nand_flash_dev
> > *nand_get_flash_type(struct mtd_info *mtd,
> > 
> >  			mtd->erasesize <<= ((id_data[3] & 0x03) << 1);
> >  		
> >  		}
> >  	
> >  	}
> > 
> > -	/* Get chip options, preserve non chip based options */
> > -	chip->options &= ~NAND_CHIPOPTIONS_MSK;
> > -	chip->options |= type->options & NAND_CHIPOPTIONS_MSK;
> > +	/* Get chip options */
> > +	chip->options |= type->options;
> > 
> >  	/*
> >  	
> >  	 * Check if chip is not a Samsung device. Do not clear the
> > 
> > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> > index 6dce5a7..eeb7015 100644
> > --- a/include/linux/mtd/nand.h
> > +++ b/include/linux/mtd/nand.h
> > @@ -206,9 +206,6 @@ typedef enum {
> > 
> >  #define NAND_SUBPAGE_READ(chip) ((chip->ecc.mode == NAND_ECC_SOFT) \
> >  
> >  					&& (chip->page_shift > 9))
> > 
> > -/* Mask to zero out the chip options, which come from the id table */
> > -#define NAND_CHIPOPTIONS_MSK	0x0000ffff
> > -
> > 
> >  /* Non chip related options */
> >  /* This option skips the bbt scan during initialization. */
> >  #define NAND_SKIP_BBTSCAN	0x00010000

Best regards,
Marek Vasut