[U-Boot] [PATCH v2 3/4] mtd: nand: add Freescale NFC driver

[Bill Pringlemeir]

On 14 Aug 2014, stefan at agner.ch wrote:

> This adds initial support for Freescale NFC (NAND Flash Controller)
> found in ARM Vybrid SoC's, Power Architecture MPC5125 and others.
> However, this driver is only tested on Vybrid.
> 
> Signed-off-by: Stefan Agner <stefan at agner.ch>
> ---
>  drivers/mtd/nand/Makefile    |   1 +
>  drivers/mtd/nand/vf610_nfc.c | 706
> ++++++++++++++++++++++++++++++++++++++++++> +
>  2 files changed, 707 insertions(+)
>  create mode 100644 drivers/mtd/nand/vf610_nfc.c
> 
> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
> new file mode 100644
> index 0000000..3150ac1
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,706 @@
> +/*
> + * Copyright 2009-2014 Freescale Semiconductor, Inc. and others
> + *

[snip]

> +/* Count the number of 0's in buff upto max_bits */
> +static inline int count_written_bits(uint8_t *buff, int size, int max_bits)
> +{
> +	uint32_t *buff32 = (uint32_t *)buff;
> +	int k, written_bits = 0;
> +
> +	for (k = 0; k < (size / 4); k++) {
> +		written_bits += hweight32(~buff32[k]);
> +		if (written_bits > max_bits)
> +			break;
> +	}
> +
> +	return written_bits;
> +}
 
That is a nice change. 
 
> +static inline int vf610_nfc_correct_data(struct mtd_info *mtd, u_char *dat)
> +{
> +	struct vf610_nfc *nfc = mtd_to_nfc(mtd);
> +	u8 ecc_status;
> +	u8 ecc_count;
> +	int flip;
> +
> +	ecc_status = __raw_readb(nfc->regs + ECC_SRAM_ADDR * 8 + ECC_OFFSET);
> +	ecc_count = ecc_status & ECC_ERR_COUNT;
> +	if (!(ecc_status & ECC_STATUS_MASK))
> +		return ecc_count;
> +
> +	/* If 'ecc_count' zero or less then buffer is all 0xff or erased. */
> +	flip = count_written_bits(dat, nfc->chip.ecc.size, ecc_count);
> +
> +	/* ECC failed. */
> +	if (flip > ecc_count)
> +		return -1;

Sorry, I missed this in version one of the patch.  The original had,

<       if (flip > ecc_count) {
<               nfc->page = -1;
---
>       if (flip > ecc_count)
522d508
<       }

I can see why you removed this (nfc->page = -1).  However, I think that
higher layers may want to re-read on an error in case of un-stable bits?
It is very little code to ensure a re-read in case of ECC failure.  The
2nd physical read may pass whereas the first failed.  This path is rare,
but maybe important?  A higher layer may migrate the data in this case;
just as with a corrected bits.  But maybe U-Boot will never do this?

> +
> +	/* Erased page. */
> +	memset(dat, 0xff, nfc->chip.ecc.size);
> +	return 0;
> +}

Regards,
Bill Pringlemeir.