Re: [PATCH v3 3/3] mtd: nand: mxc_nand: support software ECC

[Sascha Hauer]

On Tue, May 14, 2024 at 04:25:19PM +0200, Sascha Hauer wrote:
> With these changes the driver can be used with software BCH ECC which
> is useful for NAND chips that require a stronger ECC than the i.MX
> hardware supports.
> 
> The controller normally interleaves user data with OOB data when
> accessing the NAND chip. With Software BCH ECC we write the data
> to the NAND in a way that the raw data on the NAND chip matches the
> way the NAND layer sees it. This way commands like NAND_CMD_RNDOUT
> work as expected.
> 
> This was tested on i.MX27 but should work on the other SoCs supported
> by this driver as well.
> 
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/mxc_nand.c | 73 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 68 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index 6f8b8f4b118ec..ae4cff2584a2d 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -1404,10 +1404,10 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
>  	chip->ecc.bytes = host->devtype_data->eccbytes;
>  	host->eccsize = host->devtype_data->eccsize;
>  	chip->ecc.size = 512;
> -	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
>  
>  	switch (chip->ecc.engine_type) {
>  	case NAND_ECC_ENGINE_TYPE_ON_HOST:
> +		mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
>  		chip->ecc.read_page = mxc_nand_read_page;
>  		chip->ecc.read_page_raw = mxc_nand_read_page_raw;
>  		chip->ecc.read_oob = mxc_nand_read_oob;
> @@ -1417,6 +1417,8 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
>  		break;
>  
>  	case NAND_ECC_ENGINE_TYPE_SOFT:
> +		chip->ecc.write_page_raw = nand_monolithic_write_page_raw;
> +		chip->ecc.read_page_raw = nand_monolithic_read_page_raw;
>  		break;
>  
>  	default:
> @@ -1472,6 +1474,59 @@ static int mxcnd_setup_interface(struct nand_chip *chip, int chipnr,
>  	return host->devtype_data->setup_interface(chip, chipnr, conf);
>  }
>  
> +static void copy_page_to_sram(struct mtd_info *mtd, const void *buf, int buf_len)
> +{
> +	struct nand_chip *this = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(this);
> +	unsigned int no_subpages = mtd->writesize / 512;
> +	int oob_per_subpage, i;
> +
> +	oob_per_subpage = (mtd->oobsize / no_subpages) & ~1;
> +
> +	/*
> +	 * During a page write the i.MX NAND controller will read 512b from
> +	 * main_area0 SRAM, then oob_per_subpage bytes from spare0 SRAM, then
> +	 * 512b from main_area1 SRAM and so on until the full page is written.
> +	 * For software ECC we want to have a 1:1 mapping between the raw page
> +	 * data on the NAND chip and the view of the NAND core. This is
> +	 * necessary to make the NAND_CMD_RNDOUT read the data it expects.
> +	 * To accomplish this we have to write the data in the order the controller
> +	 * reads it. This is reversed in copy_page_from_sram() below.
> +	 */
> +	for (i = 0; i < no_subpages; i++) {
> +		memcpy16_toio(host->main_area0 + i * 512, buf, 512);
> +		buf += 512;
> +
> +		memcpy16_toio(host->spare0 + i * host->devtype_data->spare_len, buf,
> +			      oob_per_subpage);
> +		buf += oob_per_subpage;
> +	}
> +}

I noticed the nandbiterr test won't work with this. It needs the following
fixup. The problem is that the core wants to write only user data
without OOB, so we have to make sure the remaining SRAM is filled up
with 0xff.

I'll wait some time for other comments before sending a v4 with this
included.

Sascha

-----------------------------8<--------------------------------