[PATCH 2/3] mtd: spi-nor: Implement die erase command logic

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen at atmel.com]
> Sent: Tuesday, November 29, 2016 5:47 PM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski at nokia.com>; linux-mtd at lists.infradead.org
> Cc: rfsw-patches at mlist.nokia.com; dwmw2 at infradead.org;
> computersforpeace at gmail.com; marek.vasut at gmail.com
> Subject: Re: [PATCH 2/3] mtd: spi-nor: Implement die erase command logic
> 
> Hi Marcin,
> 
> I know you have already answered some of my questions of IRC but I ask
> them again here on the mailing list so everybody can benefit from your
> answers.
> 
> Le 24/10/2016 à 15:03, marcin.krzeminski at nokia.com a écrit :
> > From: Marcin Krzeminski <marcin.krzeminski at nokia.com>
> >
> > This commit implements die erase logic.
> > Sector at a time procedure is moved to function, then erase algorithm
> > will try to use die erase cmd if size and address cover one or more
> > dies.
> >
> 
> I'm reading your v2 series but indeed I try to understand the purpose of the
> series simply because I don't know what does die erase do as compared to
> chip erase of sector erase.
> Is you series a bug fix or an optmization, maybe both?

In current spi-nor framework when user want to erase whole flash
(eg. mtd_debug erase /dev/mtdX 0 0x8000000) framework issue chip erase cmd.
N25Q00 does not support chip erase, instead there was introduced
die erase command that can erase whole die (N25Q00 is a mulit-die NOR chip).
So for this case this patch is a bug fix.
Since I have die erase command implemented it was tempting to use die erase
also in case where erase area cover one or more dies to speed up erase process,
especially when user has enabled 4KiB sector. That is a small optimization.

I will rewrite commit message and add those informations.

Thanks,
Marcin

> 
> Best regards,
> 
> Cyrille
> 
> > Signed-off-by: Marcin Krzeminski <marcin.krzeminski at nokia.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 90
> > +++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 78 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > b/drivers/mtd/spi-nor/spi-nor.c index 3afe207..17bbec0 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -360,6 +360,29 @@ static int spi_nor_erase_sector(struct spi_nor
> *nor, u32 addr)
> >  	return nor->write_reg(nor, nor->erase_opcode, buf, nor-
> >addr_width);
> > }
> >
> > +static inline int spi_nor_sector_at_time_erase(struct mtd_info *mtd,
> > +u32 addr, u32 len) {
> > +	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> > +	int ret = 0;
> > +
> > +	while (len) {
> > +		write_enable(nor);
> > +
> > +		ret = spi_nor_erase_sector(nor, addr);
> > +		if (ret)
> > +			return ret;
> > +
> > +		addr += mtd->erasesize;
> > +		len -= mtd->erasesize;
> > +
> > +		ret = spi_nor_wait_till_ready(nor);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  /*
> >   * Erase an address range on the nor chip.  The address range may extend
> >   * one or more erase sectors.  Return an error is there is a problem erasing.
> > @@ -367,9 +390,11 @@ static int spi_nor_erase_sector(struct spi_nor
> > *nor, u32 addr)  static int spi_nor_erase(struct mtd_info *mtd, struct
> > erase_info *instr)  {
> >  	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> > -	u32 addr, len;
> > +	u32 addr, len, die_no, die_size;
> >  	uint32_t rem;
> >  	int ret;
> > +	unsigned long timeout;
> > +	u8 die_erase = nor->die_cnt && SNOR_F_DIE_ERASE;
> >
> >  	dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
> >  			(long long)instr->len);
> > @@ -386,7 +411,7 @@ static int spi_nor_erase(struct mtd_info *mtd,
> struct erase_info *instr)
> >  		return ret;
> >
> >  	/* whole-chip erase? */
> > -	if (len == mtd->size) {
> > +	if (len == mtd->size && !die_erase) {
> >  		unsigned long timeout;
> >
> >  		write_enable(nor);
> > @@ -416,17 +441,58 @@ static int spi_nor_erase(struct mtd_info *mtd,
> > struct erase_info *instr)
> >
> >  	/* "sector"-at-a-time erase */
> >  	} else {
> > -		while (len) {
> > -			write_enable(nor);
> > -
> > -			ret = spi_nor_erase_sector(nor, addr);
> > -			if (ret)
> > +		if (die_erase) {
> > +			die_size = div_u64_rem(mtd->size, nor->die_cnt,
> &rem);
> > +			if (rem) {
> > +				ret = -EINVAL;
> >  				goto erase_err;
> > -
> > -			addr += mtd->erasesize;
> > -			len -= mtd->erasesize;
> > -
> > -			ret = spi_nor_wait_till_ready(nor);
> > +			}
> > +			while (len) {
> > +				die_no = div_u64_rem(addr, die_size,
> &rem);
> > +
> > +				/* Check if address is aligned to die begin*/
> > +				if (!rem) {
> > +					/* die erase? */
> > +					if (len >= die_size) {
> > +						ret = spi_nor_die_erase(nor,
> addr);
> > +						if (ret)
> > +							goto erase_err;
> > +
> > +						len -= die_size;
> > +						addr += die_size;
> > +
> > +						timeout =
> max(CHIP_ERASE_2MB_READY_WAIT_JIFFIES,
> > +
> 	CHIP_ERASE_2MB_READY_WAIT_JIFFIES *
> > +								(unsigned
> long)(die_size / SZ_2M));
> > +						ret =
> spi_nor_wait_till_ready_with_timeout(nor, timeout);
> > +						if (ret)
> > +							goto erase_err;
> > +					} else {
> > +						ret =
> spi_nor_sector_at_time_erase(mtd, addr, len);
> > +						if (ret)
> > +							goto erase_err;
> > +						len = 0;
> > +					}
> > +				} else {
> > +					/* check if end address cover at least
> one die */
> > +					if (div64_ul(addr + len, die_size) >
> ++die_no) {
> > +						/* align to next die */
> > +						rem = die_size - rem;
> > +						ret =
> spi_nor_sector_at_time_erase(mtd, addr, rem);
> > +						if (ret)
> > +							goto erase_err;
> > +						len -= rem;
> > +						addr += rem;
> > +					} else {
> > +						ret =
> spi_nor_sector_at_time_erase(mtd, addr, len);
> > +						if (ret)
> > +							goto erase_err;
> > +						len = 0;
> > +					}
> > +				}
> > +			}
> > +		} else {
> > +			ret = spi_nor_sector_at_time_erase(mtd, addr, len);
> >  			if (ret)
> >  				goto erase_err;
> >  		}
> >