Re: [PATCH 2/3] mtd: rawnand: Export nand_read_page_hwecc_oob_first()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Miquel,

Le ven., oct. 15 2021 at 08:13:13 +0200, Miquel Raynal <miquel.raynal@xxxxxxxxxxx> a écrit :
Hi Paul,

paul@xxxxxxxxxxxxxxx wrote on Sat,  9 Oct 2021 20:49:51 +0200:

 Move the function nand_read_page_hwecc_oob_first() (previously
nand_davinci_read_page_hwecc_oob_first()) to nand_base.c, and export it
 as a GPL symbol, so that it can be used by more modules.

 Cc: <stable@xxxxxxxxxxxxxxx> # v5.2
Fixes: a0ac778eb82c ("mtd: rawnand: ingenic: Add support for the JZ4740")
 Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
 ---
drivers/mtd/nand/raw/davinci_nand.c | 70 +---------------------------- drivers/mtd/nand/raw/nand_base.c | 69 ++++++++++++++++++++++++++++
  include/linux/mtd/rawnand.h         |  2 +
  3 files changed, 72 insertions(+), 69 deletions(-)

diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
 index 89de24d3bb7a..45fec8c192ab 100644
 --- a/drivers/mtd/nand/raw/davinci_nand.c
 +++ b/drivers/mtd/nand/raw/davinci_nand.c
@@ -371,74 +371,6 @@ static int nand_davinci_correct_4bit(struct nand_chip *chip, u_char *data,
  	return corrected;
  }

 -/**
 - * nand_read_page_hwecc_oob_first - hw ecc, read oob first
 - * @chip: nand chip info structure
 - * @buf: buffer to store read data
 - * @oob_required: caller requires OOB data read to chip->oob_poi
 - * @page: page number to read
 - *
- * Hardware ECC for large page chips, require OOB to be read first. For this - * ECC mode, the write_page method is re-used from ECC_HW. These methods - * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with - * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from - * the data area, by overwriting the NAND manufacturer bad block markings.
 - */
-static int nand_davinci_read_page_hwecc_oob_first(struct nand_chip *chip,
 -						  uint8_t *buf,
 -						  int oob_required, int page)
 -{
 -	struct mtd_info *mtd = nand_to_mtd(chip);
 -	int i, eccsize = chip->ecc.size, ret;
 -	int eccbytes = chip->ecc.bytes;
 -	int eccsteps = chip->ecc.steps;
 -	uint8_t *p = buf;
 -	uint8_t *ecc_code = chip->ecc.code_buf;
 -	unsigned int max_bitflips = 0;
 -
 -	/* Read the OOB area first */
- ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
 -	if (ret)
 -		return ret;
 -
 -	ret = nand_read_page_op(chip, page, 0, NULL, 0);
 -	if (ret)
 -		return ret;
 -
 -	ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
 -					 chip->ecc.total);
 -	if (ret)
 -		return ret;
 -
 -	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 -		int stat;
 -
 -		chip->ecc.hwctl(chip, NAND_ECC_READ);
 -
 -		ret = nand_read_data_op(chip, p, eccsize, false, false);
 -		if (ret)
 -			return ret;
 -
 -		stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
 -		if (stat == -EBADMSG &&
 -		    (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
 -			/* check for empty pages with bitflips */
 -			stat = nand_check_erased_ecc_chunk(p, eccsize,
 -							   &ecc_code[i],
 -							   eccbytes, NULL, 0,
 -							   chip->ecc.strength);
 -		}
 -
 -		if (stat < 0) {
 -			mtd->ecc_stats.failed++;
 -		} else {
 -			mtd->ecc_stats.corrected += stat;
 -			max_bitflips = max_t(unsigned int, max_bitflips, stat);
 -		}
 -	}
 -	return max_bitflips;
 -}
 -
/*----------------------------------------------------------------------*/

  /* An ECC layout for using 4-bit ECC with small-page flash, storing
@@ -648,7 +580,7 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
  			} else if (chunks == 4 || chunks == 8) {
  				mtd_set_ooblayout(mtd,
  						  nand_get_large_page_ooblayout());
 -				chip->ecc.read_page = nand_davinci_read_page_hwecc_oob_first;
 +				chip->ecc.read_page = nand_read_page_hwecc_oob_first;
  			} else {
  				return -EIO;
  			}
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
 index 3d6c6e880520..cb5f343b9fa2 100644
 --- a/drivers/mtd/nand/raw/nand_base.c
 +++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3160,6 +3160,75 @@ static int nand_read_page_hwecc(struct nand_chip *chip, uint8_t *buf,
  	return max_bitflips;
  }

 +/**
 + * nand_read_page_hwecc_oob_first - Hardware ECC page read with ECC
 + *                                  data read from OOB area
 + * @chip: nand chip info structure
 + * @buf: buffer to store read data
 + * @oob_required: caller requires OOB data read to chip->oob_poi
 + * @page: page number to read
 + *
+ * Hardware ECC for large page chips, require OOB to be read first. For this

requires

With this ECC configuration?

+ * ECC mode, the write_page method is re-used from ECC_HW. These methods

I do not understand this sentence nor the next one about syndrome. I
believe it is related to your engine and should not leak into the core.

+ * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with + * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from + * the data area, by overwriting the NAND manufacturer bad block markings.

That's a sentence I don't like. What do you mean exactly?

What "Infix ECC" scheme is?

Do you mean that unlike the syndrome  mode it *does not* overwrite the
BBM ?

I don't mean anything. I did not write that comment. I just moved the function verbatim with no changes. If something needs to be fixed, then it needs to be fixed before/after this patch.

 + */
+int nand_read_page_hwecc_oob_first(struct nand_chip *chip, uint8_t *buf,
 +				   int oob_required, int page)
 +{
 +	struct mtd_info *mtd = nand_to_mtd(chip);
 +	int i, eccsize = chip->ecc.size, ret;
 +	int eccbytes = chip->ecc.bytes;
 +	int eccsteps = chip->ecc.steps;
 +	uint8_t *p = buf;
 +	uint8_t *ecc_code = chip->ecc.code_buf;
 +	unsigned int max_bitflips = 0;
 +
 +	/* Read the OOB area first */
+ ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
 +	if (ret)
 +		return ret;
 +
 +	ret = nand_read_page_op(chip, page, 0, NULL, 0);

Definitely not, your are requesting the chip to do the read_page
operation twice. You only need a nand_change_read_column I believe.

Again, this code is just being moved around - don't shoot the messenger :)

 +	if (ret)
 +		return ret;
 +
 +	ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
 +					 chip->ecc.total);
 +	if (ret)
 +		return ret;
 +
 +	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 +		int stat;
 +
 +		chip->ecc.hwctl(chip, NAND_ECC_READ);
 +
 +		ret = nand_read_data_op(chip, p, eccsize, false, false);
 +		if (ret)
 +			return ret;
 +
 +		stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
 +		if (stat == -EBADMSG &&
 +		    (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
 +			/* check for empty pages with bitflips */
 +			stat = nand_check_erased_ecc_chunk(p, eccsize,
 +							   &ecc_code[i],
 +							   eccbytes, NULL, 0,
 +							   chip->ecc.strength);
 +		}
 +
 +		if (stat < 0) {
 +			mtd->ecc_stats.failed++;
 +		} else {
 +			mtd->ecc_stats.corrected += stat;
 +			max_bitflips = max_t(unsigned int, max_bitflips, stat);
 +		}
 +	}
 +	return max_bitflips;
 +}
 +EXPORT_SYMBOL_GPL(nand_read_page_hwecc_oob_first);
 +
  /**
* nand_read_page_syndrome - [REPLACEABLE] hardware ECC syndrome based page read
   * @chip: nand chip info structure
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
 index b2f9dd3cbd69..5b88cd51fadb 100644
 --- a/include/linux/mtd/rawnand.h
 +++ b/include/linux/mtd/rawnand.h
@@ -1539,6 +1539,8 @@ int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
  		      bool force_8bit, bool check_only);
  int nand_write_data_op(struct nand_chip *chip, const void *buf,
  		       unsigned int len, bool force_8bit);
+int nand_read_page_hwecc_oob_first(struct nand_chip *chip, uint8_t *buf,
 +				   int oob_required, int page);

You certainly want to add this symbol closer to the other read/write
page helpers?

Where would that be? The other read/write page helpers are all "static" so they don't appear in any header.

Cheers,
-Paul


  /* Scan and identify a NAND device */
int nand_scan_with_ids(struct nand_chip *chip, unsigned int max_chips,


Thanks,
Miquèl





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux