Re: [PATCH v3 32/37] mtd: nand: denali: support hardware-assisted erased page detection

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Thu, 30 Mar 2017 17:15:03 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:

> Recent versions of this IP support automatic erased page detection.
> If an erased page is detected on reads, the controller does not set
> INTR__ECC_UNCOR_ERR, but INTR__ERASED_PAGE.  If this feature is
> supported, the driver can use this information instead of calling
> nand_check_erased_ecc_chunk().
> 
> The detection of erased page is based on the number of zeros in the
> page; if the number of zeros is less than the value in the field
> ERASED_THRESHOLD, the page is assumed as erased.
> 
> Set the ERASED_THRESHOLD to (chip->ecc.strength + 1).  This is the
> worst case where all the bitflips come from the same ECC sector.
> This field is Reserved for older IP versions, so this commit has
> no impact on them.

Do you have a way to know the actual number of bitflips in an erased
ECC block? BTW, is the threshold a per-page information or a per ECC
block information.

If you can't know the real number of bitflips I don't think it's safe
to set the threshold to chip->ecc.strength + 1.

You can still use the feature to detect erased pages without any
bitflips (set ERASED_THRESHOLD to 1), which should be the case most of
the time, but for cases where you have bitflips I'd recommend using
nand_check_erased_ecc_chunk() if you can't know the actual number of
bitflips in the page.

> 
> One thing worth mentioning is the driver still needs to fill the
> buffer with 0xff in the case because the ECC correction engine has
> already manipulated the data in the buffer.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> Changes in v3: None
> Changes in v2:
>   - Newly added
> 
>  drivers/mtd/nand/denali.c | 10 +++++++++-
>  drivers/mtd/nand/denali.h |  5 +++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 51e8a9a..9ee0f30 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -604,6 +604,9 @@ static int denali_pio_read(struct denali_nand_info *denali, void *buf,
>  	if (!(irq_status & INTR__PAGE_XFER_INC))
>  		return -EIO;
>  
> +	if (irq_status & INTR__ERASED_PAGE)
> +		memset(buf, 0xff, size);
> +
>  	return irq_status & ecc_err_mask ? -EBADMSG : 0;
>  }
>  
> @@ -676,6 +679,9 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
>  	denali_enable_dma(denali, false);
>  	dma_sync_single_for_cpu(denali->dev, dma_addr, size, dir);
>  
> +	if (irq_status & INTR__ERASED_PAGE)
> +		memset(buf, 0xff, size);
> +
>  	return ret;
>  }
>  
> @@ -1475,7 +1481,9 @@ int denali_init(struct denali_nand_info *denali)
>  	chip->ecc.bytes = denali_calc_ecc_bytes(chip->ecc.size,
>  						chip->ecc.strength);
>  
> -	iowrite32(chip->ecc.strength, denali->flash_reg + ECC_CORRECTION);
> +	iowrite32(MAKE_ECC_CORRECTION(chip->ecc.strength,
> +				      chip->ecc.strength + 1),
> +		  denali->flash_reg + ECC_CORRECTION);
>  	iowrite32(mtd->erasesize / mtd->writesize,
>  		  denali->flash_reg + PAGES_PER_BLOCK);
>  	iowrite32(denali->nand.options & NAND_BUSWIDTH_16 ? 1 : 0,
> diff --git a/drivers/mtd/nand/denali.h b/drivers/mtd/nand/denali.h
> index f92b9db..b5ea8d7 100644
> --- a/drivers/mtd/nand/denali.h
> +++ b/drivers/mtd/nand/denali.h
> @@ -110,6 +110,10 @@
>  
>  #define ECC_CORRECTION				0x1b0
>  #define     ECC_CORRECTION__VALUE			GENMASK(4, 0)
> +#define     ECC_CORRECTION__ERASE_THRESHOLD		GENMASK(31, 16)
> +#define     MAKE_ECC_CORRECTION(val, thresh)		\
> +			(((val) & (ECC_CORRECTION__VALUE)) | \
> +			(((thresh) << 16) & (ECC_CORRECTION__ERASE_THRESHOLD)))
>  
>  #define READ_MODE				0x1c0
>  #define     READ_MODE__VALUE				GENMASK(3, 0)
> @@ -233,6 +237,7 @@
>  #define     INTR__RST_COMP				BIT(13)
>  #define     INTR__PIPE_CMD_ERR				BIT(14)
>  #define     INTR__PAGE_XFER_INC				BIT(15)
> +#define     INTR__ERASED_PAGE				BIT(16)
>  
>  #define PAGE_CNT(bank)				(0x430 + (bank) * 0x50)
>  #define ERR_PAGE_ADDR(bank)			(0x440 + (bank) * 0x50)