Subject: [RFC PATCH v2] MTD: nand: add return value for write_page()/write_page_raw() functions in structure of nand_ecc_ctrl.

From: [email-masked] (Josh Wu)
Date: Wed, 20 Jun 2012 11:00:02 +0800

Hi, Artem

Ping? Do you have any comments for this patch?

Best Regards,
Josh Wu

On 6/13/2012 2:06 PM, Josh Wu wrote:
> There is an implemention of hardware ECC write page function which may return an error indication. But now the definition of 'write_page' function in struct nand_ecc_ctrl is 'void'.
> This patch change the definition of 'write_page' fuction to return an 'int' value. It adds code to test the return value, and if negative, indicate an error happend when write page with ECC.
>
> To be consistent, it also changes the 'write_page_raw' function's return type from 'void' to 'int'.
>
> This patch also adds 'return 0' for all implementation for the ecc 'write_page' and 'write_page_raw' functions.
>
> Note: I couldn't compile-test all of these easily, as some had ARCH dependencies.
>
> Signed-off-by: Josh Wu<[email-masked]>
> ---
>   drivers/mtd/nand/bcm_umi_bch.c         |    6 ++++--
>   drivers/mtd/nand/bf5xx_nand.c          |    6 ++++--
>   drivers/mtd/nand/cafe_nand.c           |   13 ++++++++++---
>   drivers/mtd/nand/denali.c              |   12 +++++++-----
>   drivers/mtd/nand/docg4.c               |    8 +++++---
>   drivers/mtd/nand/fsl_elbc_nand.c       |    4 +++-
>   drivers/mtd/nand/fsl_ifc_nand.c        |    4 +++-
>   drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    6 ++++--
>   drivers/mtd/nand/nand_base.c           |   29 +++++++++++++++++++++--------
>   drivers/mtd/nand/pxa3xx_nand.c         |    4 +++-
>   drivers/mtd/nand/sh_flctl.c            |    4 +++-
>   include/linux/mtd/nand.h               |    4 ++--
>   12 files changed, 69 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
> index 5914bb3..c8799a0 100644
> --- a/drivers/mtd/nand/bcm_umi_bch.c
> +++ b/drivers/mtd/nand/bcm_umi_bch.c
> @@ -23,7 +23,7 @@
>   /* ---- Private Function Prototypes -------------------------------------- */
>   static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>   	struct nand_chip *chip, uint8_t *buf, int oob_required, int page);
> -static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
> +static int bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
>   	struct nand_chip *chip, const uint8_t *buf, int oob_required);
>
>   /* ---- Private Variables ------------------------------------------------ */
> @@ -194,7 +194,7 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
>   *  @oob_required:	must write chip->oob_poi to OOB
>   *
>   ***************************************************************************/
> -static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
> +static int bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
>   	struct nand_chip *chip, const uint8_t *buf, int oob_required)
>   {
>   	int sectorIdx = 0;
> @@ -214,4 +214,6 @@ static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
>   	}
>
>   	bcm_umi_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
> diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
> index 3f1c185..ab0caa7 100644
> --- a/drivers/mtd/nand/bf5xx_nand.c
> +++ b/drivers/mtd/nand/bf5xx_nand.c
> @@ -566,11 +566,13 @@ static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip
>   	return 0;
>   }
>
> -static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -		const uint8_t *buf, int oob_required)
> +static int bf5xx_nand_write_page_raw(struct mtd_info *mtd,
> +		struct nand_chip *chip,	const uint8_t *buf, int oob_required)
>   {
>   	bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
>   	bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   /*
> diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
> index 41371ba..3c92a98 100644
> --- a/drivers/mtd/nand/cafe_nand.c
> +++ b/drivers/mtd/nand/cafe_nand.c
> @@ -520,7 +520,7 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_512 = {
>   };
>
>
> -static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
> +static int cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
>   					  struct nand_chip *chip,
>   					  const uint8_t *buf, int oob_required)
>   {
> @@ -531,6 +531,8 @@ static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
>
>   	/* Set up ECC autogeneration */
>   	cafe->ctl2 |= (1<<30);
> +
> +	return 0;
>   }
>
>   static int cafe_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> @@ -542,9 +544,14 @@ static int cafe_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
>   	if (unlikely(raw))
> -		chip->ecc.write_page_raw(mtd, chip, buf, oob_required);
> +		status = chip->ecc.write_page_raw(mtd, chip, buf, oob_required);
>   	else
> -		chip->ecc.write_page(mtd, chip, buf, oob_required);
> +		status = chip->ecc.write_page(mtd, chip, buf, oob_required);
> +
> +	if (status<  0) {
> +		pr_warn("Error happened when calling cafe_nand_write_page()\n");
> +		return status;
> +	}
>
>   	/*
>   	 * Cached progamming disabled for now, Not sure if its worth the
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 0650aaf..e706a23 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1028,7 +1028,7 @@ static void denali_setup_dma(struct denali_nand_info *denali, int op)
>
>   /* writes a page. user specifies type, and this function handles the
>    * configuration details. */
> -static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +static int write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   			const uint8_t *buf, bool raw_xfer)
>   {
>   	struct denali_nand_info *denali = mtd_to_denali(mtd);
> @@ -1078,6 +1078,8 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
>
>   	denali_enable_dma(denali, false);
>   	dma_sync_single_for_cpu(denali->dev, addr, size, DMA_TO_DEVICE);
> +
> +	return 0;
>   }
>
>   /* NAND core entry points */
> @@ -1086,24 +1088,24 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
>    * writing a page with ECC or without is similar, all the work is done
>    * by write_page above.
>    * */
> -static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +static int denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   				const uint8_t *buf, int oob_required)
>   {
>   	/* for regular page writes, we let HW handle all the ECC
>   	 * data written to the device. */
> -	write_page(mtd, chip, buf, false);
> +	return write_page(mtd, chip, buf, false);
>   }
>
>   /* This is the callback that the NAND core calls to write a page without ECC.
>    * raw access is similar to ECC page writes, so all the work is done in the
>    * write_page() function above.
>    */
> -static void denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +static int denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>   					const uint8_t *buf, int oob_required)
>   {
>   	/* for raw page writes, we want to disable ECC and simply write
>   	   whatever data is in the buffer. */
> -	write_page(mtd, chip, buf, true);
> +	return write_page(mtd, chip, buf, true);
>   }
>
>   static int denali_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
> index a225e49..0f2ffd7 100644
> --- a/drivers/mtd/nand/docg4.c
> +++ b/drivers/mtd/nand/docg4.c
> @@ -898,7 +898,7 @@ static void docg4_erase_block(struct mtd_info *mtd, int page)
>   	write_nop(docptr);
>   }
>
> -static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
> +static int write_page(struct mtd_info *mtd, struct nand_chip *nand,
>   		       const uint8_t *buf, bool use_ecc)
>   {
>   	struct docg4_priv *doc = nand->priv;
> @@ -950,15 +950,17 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
>   	write_nop(docptr);
>   	writew(0, docptr + DOC_DATAEND);
>   	write_nop(docptr);
> +
> +	return 0;
>   }
>
> -static void docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
> +static int docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
>   				 const uint8_t *buf, int oob_required)
>   {
>   	return write_page(mtd, nand, buf, false);
>   }
>
> -static void docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
> +static int docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
>   			     const uint8_t *buf, int oob_required)
>   {
>   	return write_page(mtd, nand, buf, true);
> diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> index 7842938..35d731d 100644
> --- a/drivers/mtd/nand/fsl_elbc_nand.c
> +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> @@ -766,11 +766,13 @@ static int fsl_elbc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>   /* ECC will be calculated automatically, and errors will be detected in
>    * waitfunc.
>    */
> -static void fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +static int fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   				const uint8_t *buf, int oob_required)
>   {
>   	fsl_elbc_write_buf(mtd, buf, mtd->writesize);
>   	fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
> index 9602c1b..47ba534 100644
> --- a/drivers/mtd/nand/fsl_ifc_nand.c
> +++ b/drivers/mtd/nand/fsl_ifc_nand.c
> @@ -721,11 +721,13 @@ static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>   /* ECC will be calculated automatically, and errors will be detected in
>    * waitfunc.
>    */
> -static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +static int fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   			       const uint8_t *buf, int oob_required)
>   {
>   	fsl_ifc_write_buf(mtd, buf, mtd->writesize);
>   	fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   static int fsl_ifc_chip_init_tail(struct mtd_info *mtd)
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index a05b7b4..3bda330 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -930,7 +930,7 @@ exit_nfc:
>   	return ret;
>   }
>
> -static void gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   				const uint8_t *buf, int oob_required)
>   {
>   	struct gpmi_nand_data *this = chip->priv;
> @@ -972,7 +972,7 @@ static void gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   				&payload_virt,&payload_phys);
>   		if (ret) {
>   			pr_err("Inadequate payload DMA buffer\n");
> -			return;
> +			return 0;
>   		}
>
>   		ret = send_page_prepare(this,
> @@ -1002,6 +1002,8 @@ exit_auxiliary:
>   				nfc_geo->payload_size,
>   				payload_virt, payload_phys);
>   	}
> +
> +	return 0;
>   }
>
>   /*
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index d47586c..ad9e9a9 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1927,12 +1927,14 @@ out:
>    *
>    * Not for syndrome calculating ECC controllers, which use a special oob layout.
>    */
> -static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +static int nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>   				const uint8_t *buf, int oob_required)
>   {
>   	chip->write_buf(mtd, buf, mtd->writesize);
>   	if (oob_required)
>   		chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   /**
> @@ -1944,7 +1946,7 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>    *
>    * We need a special oob layout and handling even when ECC isn't checked.
>    */
> -static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
> +static int nand_write_page_raw_syndrome(struct mtd_info *mtd,
>   					struct nand_chip *chip,
>   					const uint8_t *buf, int oob_required)
>   {
> @@ -1974,6 +1976,8 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>   	size = mtd->oobsize - (oob - chip->oob_poi);
>   	if (size)
>   		chip->write_buf(mtd, oob, size);
> +
> +	return 0;
>   }
>   /**
>    * nand_write_page_swecc - [REPLACEABLE] software ECC based page write function
> @@ -1982,7 +1986,7 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
>    * @buf: data buffer
>    * @oob_required: must write chip->oob_poi to OOB
>    */
> -static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
> +static int nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>   				  const uint8_t *buf, int oob_required)
>   {
>   	int i, eccsize = chip->ecc.size;
> @@ -1999,7 +2003,7 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>   	for (i = 0; i<  chip->ecc.total; i++)
>   		chip->oob_poi[eccpos[i]] = ecc_calc[i];
>
> -	chip->ecc.write_page_raw(mtd, chip, buf, 1);
> +	return chip->ecc.write_page_raw(mtd, chip, buf, 1);
>   }
>
>   /**
> @@ -2009,7 +2013,7 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>    * @buf: data buffer
>    * @oob_required: must write chip->oob_poi to OOB
>    */
> -static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> +static int nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>   				  const uint8_t *buf, int oob_required)
>   {
>   	int i, eccsize = chip->ecc.size;
> @@ -2029,6 +2033,8 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>   		chip->oob_poi[eccpos[i]] = ecc_calc[i];
>
>   	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   /**
> @@ -2041,7 +2047,7 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>    * The hw generator calculates the error syndrome automatically. Therefore we
>    * need a special oob layout and handling.
>    */
> -static void nand_write_page_syndrome(struct mtd_info *mtd,
> +static int nand_write_page_syndrome(struct mtd_info *mtd,
>   				    struct nand_chip *chip,
>   				    const uint8_t *buf, int oob_required)
>   {
> @@ -2075,6 +2081,8 @@ static void nand_write_page_syndrome(struct mtd_info *mtd,
>   	i = mtd->oobsize - (oob - chip->oob_poi);
>   	if (i)
>   		chip->write_buf(mtd, oob, i);
> +
> +	return 0;
>   }
>
>   /**
> @@ -2096,9 +2104,14 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
>   	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
>
>   	if (unlikely(raw))
> -		chip->ecc.write_page_raw(mtd, chip, buf, oob_required);
> +		status = chip->ecc.write_page_raw(mtd, chip, buf, oob_required);
>   	else
> -		chip->ecc.write_page(mtd, chip, buf, oob_required);
> +		status = chip->ecc.write_page(mtd, chip, buf, oob_required);
> +
> +	if (status<  0) {
> +		pr_warn("Error happened when calling nand_write_page()\n");
> +		return status;
> +	}
>
>   	/*
>   	 * Cached progamming disabled for now. Not sure if it's worth the
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 252aaef..86640f7 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -681,11 +681,13 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
>   	info->state = STATE_IDLE;
>   }
>
> -static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
> +static int pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
>   		struct nand_chip *chip, const uint8_t *buf, int oob_required)
>   {
>   	chip->write_buf(mtd, buf, mtd->writesize);
>   	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
> +
> +	return 0;
>   }
>
>   static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
> diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
> index aa9b8a5..0700ad9 100644
> --- a/drivers/mtd/nand/sh_flctl.c
> +++ b/drivers/mtd/nand/sh_flctl.c
> @@ -365,7 +365,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>   	return 0;
>   }
>
> -static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
> +static int flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>   				   const uint8_t *buf, int oob_required)
>   {
>   	int i, eccsize = chip->ecc.size;
> @@ -375,6 +375,8 @@ static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>
>   	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
>   		chip->write_buf(mtd, p, eccsize);
> +
> +	return 0;
>   }
>
>   static void execmd_read_page_sector(struct mtd_info *mtd, int page_addr)
> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 57977c6..4cb6c7fe 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -361,13 +361,13 @@ struct nand_ecc_ctrl {
>   			uint8_t *calc_ecc);
>   	int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>   			uint8_t *buf, int oob_required, int page);
> -	void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
> +	int (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>   			const uint8_t *buf, int oob_required);
>   	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
>   			uint8_t *buf, int oob_required, int page);
>   	int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
>   			uint32_t offs, uint32_t len, uint8_t *buf);
> -	void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
> +	int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
>   			const uint8_t *buf, int oob_required);
>   	int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
>   			int page);