[PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Brian Norris]

Hello again,

This is v2 of my patches to change the nand_chip and nand_ecc_ctrl interfaces
so that the nand_ecc_ctrl functions have information about whether the higher
layers actually need OOB data to be read/written from/to the NAND device. Per
some suggestions I have received, I have settled on using a boolean 'use_oob'
argument that tells the callee function whether the calling function is using
chip->oob_poi for OOB data.

Please refer to previous communications for other details.

Similar notes from last time:

I could not compile all the affected drivers, since some required ARCH-specific
builds that I am not familiar with.

Artem: can you perform your regular compile tests? I compile-tested as many as
I could.

Others: if you care about your driver, please compile test and review to be
sure I'm doing things safely for you. Because most in-kernel drivers seem to be
perfecly happy using nand_chip.oob_poi for OOB data unconditionally, I have not
struggled to port most of them to take advantage of this full change. However,
some developers have noted that certain drivers could benefit from utilizing
this 'use_oob' parameter. Fell free to write/submit/review any code on top of
these patches, assuming they are eventually accepted.

Thanks for reviewing!

Brian

Brian Norris (2):
  mtd: nand: add 'use_oob' argument to NAND {read,write}_page
    interfaces
  mtd: nand: nand_base - pass proper 'use_oob' parameter

 drivers/mtd/nand/atmel_nand.c          |    3 +-
 drivers/mtd/nand/bcm_umi_bch.c         |   10 +++--
 drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
 drivers/mtd/nand/bf5xx_nand.c          |    4 +-
 drivers/mtd/nand/cafe_nand.c           |   13 ++++---
 drivers/mtd/nand/denali.c              |    8 ++--
 drivers/mtd/nand/docg4.c               |   12 +++---
 drivers/mtd/nand/fsl_elbc_nand.c       |   11 ++----
 drivers/mtd/nand/fsl_ifc_nand.c        |   10 ++---
 drivers/mtd/nand/fsmc_nand.c           |    3 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    6 ++--
 drivers/mtd/nand/nand_base.c           |   58 ++++++++++++++++++++-----------
 drivers/mtd/nand/pxa3xx_nand.c         |    4 +-
 drivers/mtd/nand/sh_flctl.c            |    4 +-
 include/linux/mtd/nand.h               |   11 +++---
 15 files changed, 89 insertions(+), 70 deletions(-)

-- 
1.7.5.4.2.g519b1

[PATCH v2 1/2] mtd: nand: add 'use_oob' argument to NAND {read, write}_page interfaces

[Brian Norris]

New NAND controllers can perform read/write via HW engines which don't expose
OOB data in their DMA mode. To reflect this, we should rework the nand_chip /
nand_ecc_ctrl interfaces that assume that drivers will always read/write OOB
data in the nand_chip.oob_poi buffer. A better interface includes a boolean
argument that explicitly tells the callee when OOB data is requested (for
reading/writing to/from nand_chip.oob_poi).

This patch adds the 'use_oob' parameter to each relevant {read,write}_page
interface; all 'use_oob' parameters are left unused for now. The next patch
will set the parameter properly in the nand_base.c callers, and follow-up
patches may make use of 'use_oob' in the callee functions.

Note that currently, there is no harm in ignoring the 'use_oob' parameter and
*always* utilizing nand_chip.oob_poi, but there can be performance/complexity/
design benefits from avoiding filling oob_poi in the common case.

Note: I couldn't compile-test all of these easily, as some had ARCH
dependencies.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/atmel_nand.c          |    3 +-
 drivers/mtd/nand/bcm_umi_bch.c         |   10 +++--
 drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
 drivers/mtd/nand/bf5xx_nand.c          |    4 +-
 drivers/mtd/nand/cafe_nand.c           |   13 +++++---
 drivers/mtd/nand/denali.c              |    8 ++--
 drivers/mtd/nand/docg4.c               |   12 +++---
 drivers/mtd/nand/fsl_elbc_nand.c       |   11 ++----
 drivers/mtd/nand/fsl_ifc_nand.c        |   10 ++---
 drivers/mtd/nand/fsmc_nand.c           |    3 +-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    6 ++--
 drivers/mtd/nand/nand_base.c           |   54 ++++++++++++++++++++------------
 drivers/mtd/nand/pxa3xx_nand.c         |    4 +-
 drivers/mtd/nand/sh_flctl.c            |    4 +-
 include/linux/mtd/nand.h               |   11 +++---
 15 files changed, 86 insertions(+), 69 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 2165576..fa6a889 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -324,9 +324,10 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
  * mtd:        mtd info structure
  * chip:       nand chip info structure
  * buf:        buffer to store read data
+ * use_oob:    caller expects OOB data read to chip->oob_poi
  */
 static int atmel_nand_read_page(struct mtd_info *mtd,
-		struct nand_chip *chip, uint8_t *buf, int page)
+		struct nand_chip *chip, uint8_t *buf, bool use_oob, int page)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
index a930666..10d610c 100644
--- a/drivers/mtd/nand/bcm_umi_bch.c
+++ b/drivers/mtd/nand/bcm_umi_bch.c
@@ -22,9 +22,9 @@
 
 /* ---- Private Function Prototypes -------------------------------------- */
 static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, uint8_t *buf, int page);
+	struct nand_chip *chip, uint8_t *buf, bool use_oob, int page);
 static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, const uint8_t *buf);
+	struct nand_chip *chip, const uint8_t *buf, bool use_oob);
 
 /* ---- Private Variables ------------------------------------------------ */
 
@@ -103,11 +103,12 @@ static struct nand_ecclayout nand_hw_eccoob_4096 = {
 *  @mtd:	mtd info structure
 *  @chip:	nand chip info structure
 *  @buf:	buffer to store read data
+*  @use_oob:	caller expects OOB data read to chip->oob_poi
 *
 ***************************************************************************/
 static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 				       struct nand_chip *chip, uint8_t * buf,
-						 int page)
+				       bool use_oob, int page)
 {
 	int sectorIdx = 0;
 	int eccsize = chip->ecc.size;
@@ -188,10 +189,11 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 *  @mtd:	mtd info structure
 *  @chip:	nand chip info structure
 *  @buf:	data buffer
+*  @use_oob:	caller placed non-0xFF data in chip->oob_poi
 *
 ***************************************************************************/
 static void bcm_umi_bch_write_page_hwecc(struct mtd_info *mtd,
-	struct nand_chip *chip, const uint8_t *buf)
+	struct nand_chip *chip, const uint8_t *buf, bool use_oob)
 {
 	int sectorIdx = 0;
 	int eccsize = chip->ecc.size;
diff --git a/drivers/mtd/nand/bcm_umi_nand.c b/drivers/mtd/nand/bcm_umi_nand.c
index 6908cdd..27d11e8 100644
--- a/drivers/mtd/nand/bcm_umi_nand.c
+++ b/drivers/mtd/nand/bcm_umi_nand.c
@@ -341,7 +341,7 @@ static int bcm_umi_nand_verify_buf(struct mtd_info *mtd, const u_char * buf,
 	 * for MLC parts which may have permanently stuck bits.
 	 */
 	struct nand_chip *chip = mtd->priv;
-	int ret = chip->ecc.read_page(mtd, chip, readbackbuf, 0);
+	int ret = chip->ecc.read_page(mtd, chip, readbackbuf, false, 0);
 	if (ret < 0)
 		return -EFAULT;
 	else {
diff --git a/drivers/mtd/nand/bf5xx_nand.c b/drivers/mtd/nand/bf5xx_nand.c
index d7b86b9..8d5279a 100644
--- a/drivers/mtd/nand/bf5xx_nand.c
+++ b/drivers/mtd/nand/bf5xx_nand.c
@@ -558,7 +558,7 @@ static void bf5xx_nand_dma_write_buf(struct mtd_info *mtd,
 }
 
 static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-		uint8_t *buf, int page)
+		uint8_t *buf, bool use_oob, int page)
 {
 	bf5xx_nand_read_buf(mtd, buf, mtd->writesize);
 	bf5xx_nand_read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -567,7 +567,7 @@ static int bf5xx_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip
 }
 
 static void bf5xx_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-		const uint8_t *buf)
+		const uint8_t *buf, bool use_oob)
 {
 	bf5xx_nand_write_buf(mtd, buf, mtd->writesize);
 	bf5xx_nand_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index 2973d97..3ec6add 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -375,12 +375,13 @@ static int cafe_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	buffer to store read data
+ * @oob:	buffer to store OOB data
  *
  * The hw generator calculates the error syndrome automatically. Therefor
  * we need a special oob layout and handling.
  */
 static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			       uint8_t *buf, int page)
+			       uint8_t *buf, bool use_oob, int page)
 {
 	struct cafe_priv *cafe = mtd->priv;
 
@@ -518,7 +519,8 @@ static struct nand_bbt_descr cafe_bbt_mirror_descr_512 = {
 
 
 static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
-					  struct nand_chip *chip, const uint8_t *buf)
+					  struct nand_chip *chip,
+					  const uint8_t *buf, bool use_oob)
 {
 	struct cafe_priv *cafe = mtd->priv;
 
@@ -530,16 +532,17 @@ static void cafe_nand_write_page_lowlevel(struct mtd_info *mtd,
 }
 
 static int cafe_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf, int page, int cached, int raw)
+				const uint8_t *buf, bool use_oob, int page,
+				int cached, int raw)
 {
 	int status;
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		chip->ecc.write_page_raw(mtd, chip, buf);
+		chip->ecc.write_page_raw(mtd, chip, buf, use_oob);
 	else
-		chip->ecc.write_page(mtd, chip, buf);
+		chip->ecc.write_page(mtd, chip, buf, use_oob);
 
 	/*
 	 * 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 a1048c7..95ee020 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1084,7 +1084,7 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * by write_page above.
  * */
 static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf)
+				const uint8_t *buf, bool use_oob)
 {
 	/* for regular page writes, we let HW handle all the ECC
 	 * data written to the device. */
@@ -1096,7 +1096,7 @@ static void denali_write_page(struct mtd_info *mtd, struct nand_chip *chip,
  * write_page() function above.
  */
 static void denali_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-					const uint8_t *buf)
+					const uint8_t *buf, bool use_oob)
 {
 	/* for raw page writes, we want to disable ECC and simply write
 	   whatever data is in the buffer. */
@@ -1119,7 +1119,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-			    uint8_t *buf, int page)
+			    uint8_t *buf, bool use_oob, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
@@ -1171,7 +1171,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, bool use_oob, int page)
 {
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index b082026..1caf68a 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -786,13 +786,13 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 
 
 static int docg4_read_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
-			       uint8_t *buf, int page)
+			       uint8_t *buf, bool use_oob, int page)
 {
 	return read_page(mtd, nand, buf, page, false);
 }
 
 static int docg4_read_page(struct mtd_info *mtd, struct nand_chip *nand,
-			   uint8_t *buf, int page)
+			   uint8_t *buf, bool use_oob, int page)
 {
 	return read_page(mtd, nand, buf, page, true);
 }
@@ -952,13 +952,13 @@ static void write_page(struct mtd_info *mtd, struct nand_chip *nand,
 }
 
 static void docg4_write_page_raw(struct mtd_info *mtd, struct nand_chip *nand,
-				 const uint8_t *buf)
+				 const uint8_t *buf, bool use_oob)
 {
 	return write_page(mtd, nand, buf, false);
 }
 
 static void docg4_write_page(struct mtd_info *mtd, struct nand_chip *nand,
-			     const uint8_t *buf)
+			     const uint8_t *buf, bool use_oob)
 {
 	return write_page(mtd, nand, buf, true);
 }
@@ -1002,7 +1002,7 @@ static int __init read_factory_bbt(struct mtd_info *mtd)
 		return -ENOMEM;
 
 	read_page_prologue(mtd, g4_addr);
-	status = docg4_read_page(mtd, nand, buf, DOCG4_FACTORY_BBT_PAGE);
+	status = docg4_read_page(mtd, nand, buf, false, DOCG4_FACTORY_BBT_PAGE);
 	if (status)
 		goto exit;
 
@@ -1079,7 +1079,7 @@ static int docg4_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 	/* write first page of block */
 	write_page_prologue(mtd, g4_addr);
-	docg4_write_page(mtd, nand, buf);
+	docg4_write_page(mtd, nand, buf, true);
 	ret = pageprog(mtd);
 	if (!ret)
 		mtd->ecc_stats.badblocks++;
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 80b5264..12b3d4b 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -738,10 +738,8 @@ static int fsl_elbc_chip_init_tail(struct mtd_info *mtd)
 	return 0;
 }
 
-static int fsl_elbc_read_page(struct mtd_info *mtd,
-                              struct nand_chip *chip,
-			      uint8_t *buf,
-			      int page)
+static int fsl_elbc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			      uint8_t *buf, bool use_oob, int page)
 {
 	fsl_elbc_read_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -755,9 +753,8 @@ static int fsl_elbc_read_page(struct mtd_info *mtd,
 /* 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,
-                                const uint8_t *buf)
+static void fsl_elbc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+				const uint8_t *buf, bool use_oob)
 {
 	fsl_elbc_write_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 5387cec..401a772 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -692,9 +692,8 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip)
 	return nand_fsr | NAND_STATUS_WP;
 }
 
-static int fsl_ifc_read_page(struct mtd_info *mtd,
-			      struct nand_chip *chip,
-			      uint8_t *buf, int page)
+static int fsl_ifc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			     uint8_t *buf, bool use_oob, int page)
 {
 	struct fsl_ifc_mtd *priv = chip->priv;
 	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
@@ -714,9 +713,8 @@ static int fsl_ifc_read_page(struct mtd_info *mtd,
 /* 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,
-				const uint8_t *buf)
+static void fsl_ifc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+			       const uint8_t *buf, bool use_oob)
 {
 	fsl_ifc_write_buf(mtd, buf, mtd->writesize);
 	fsl_ifc_write_buf(mtd, chip->oob_poi, mtd->oobsize);
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 1b8330e..6f2b56b 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -692,6 +692,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	buffer to store read data
+ * @use_oob:	caller expects OOB data read to chip->oob_poi
  * @page:	page number to read
  *
  * This routine is needed for fsmc version 8 as reading from NAND chip has to be
@@ -701,7 +702,7 @@ static void fsmc_write_buf_dma(struct mtd_info *mtd, const uint8_t *buf,
  * max of 8 bits)
  */
 static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				 uint8_t *buf, int page)
+				 uint8_t *buf, bool use_oob, int page)
 {
 	struct fsmc_nand_data *host = container_of(mtd,
 					struct fsmc_nand_data, mtd);
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 75b1dde..dcc923a 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -841,7 +841,7 @@ static void block_mark_swapping(struct gpmi_nand_data *this,
 }
 
 static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, bool use_oob, int page)
 {
 	struct gpmi_nand_data *this = chip->priv;
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
@@ -927,8 +927,8 @@ exit_nfc:
 	return ret;
 }
 
-static void gpmi_ecc_write_page(struct mtd_info *mtd,
-				struct nand_chip *chip, const uint8_t *buf)
+static void gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+				const uint8_t *buf, bool use_oob)
 {
 	struct gpmi_nand_data *this = chip->priv;
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index eb88d8b..c4989b5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1066,12 +1066,13 @@ EXPORT_SYMBOL(nand_lock);
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects OOB data read to chip->oob_poi
  * @page: page number to read
  *
  * Not for syndrome calculating ECC controllers, which use a special oob layout.
  */
 static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-			      uint8_t *buf, int page)
+			      uint8_t *buf, bool use_oob, int page)
 {
 	chip->read_buf(mtd, buf, mtd->writesize);
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -1083,13 +1084,14 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects OOB data read to chip->oob_poi
  * @page: page number to read
  *
  * We need a special oob layout and handling even when OOB isn't used.
  */
 static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
 					struct nand_chip *chip,
-					uint8_t *buf, int page)
+					uint8_t *buf, bool use_oob, int page)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1126,10 +1128,11 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects OOB data read to chip->oob_poi
  * @page: page number to read
  */
 static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, bool use_oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1139,7 +1142,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 
-	chip->ecc.read_page_raw(mtd, chip, buf, page);
+	chip->ecc.read_page_raw(mtd, chip, buf, use_oob, page);
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
@@ -1257,12 +1260,13 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects OOB data read to chip->oob_poi
  * @page: page number to read
  *
  * Not for syndrome calculating ECC controllers which need a special oob layout.
  */
 static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, bool use_oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1302,6 +1306,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects 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
@@ -1311,7 +1316,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * the data area, by overwriting the NAND manufacturer bad block markings.
  */
 static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
-	struct nand_chip *chip, uint8_t *buf, int page)
+	struct nand_chip *chip, uint8_t *buf, bool use_oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1350,13 +1355,14 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: buffer to store read data
+ * @use_oob: caller expects OOB data read to chip->oob_poi
  * @page: page number to read
  *
  * The hw generator calculates the error syndrome automatically. Therefore we
  * need a special oob layout and handling.
  */
 static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
-				   uint8_t *buf, int page)
+				   uint8_t *buf, bool use_oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1500,14 +1506,14 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 
 			/* Now read the page into the buffer */
 			if (unlikely(ops->mode == MTD_OPS_RAW))
-				ret = chip->ecc.read_page_raw(mtd, chip,
-							      bufpoi, page);
+				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
+							      true, page);
 			else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
 				ret = chip->ecc.read_subpage(mtd, chip,
 							col, bytes, bufpoi);
 			else
 				ret = chip->ecc.read_page(mtd, chip, bufpoi,
-							  page);
+							  true, page);
 			if (ret < 0) {
 				if (!aligned)
 					/* Invalidate page cache */
@@ -1919,11 +1925,12 @@ out:
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  *
  * 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,
-				const uint8_t *buf)
+				const uint8_t *buf, bool use_oob)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -1934,12 +1941,13 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  *
  * 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,
 					struct nand_chip *chip,
-					const uint8_t *buf)
+					const uint8_t *buf, bool use_oob)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1973,9 +1981,10 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  */
 static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, bool use_oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1991,7 +2000,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);
+	chip->ecc.write_page_raw(mtd, chip, buf, use_oob);
 }
 
 /**
@@ -1999,9 +2008,10 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  */
 static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, bool use_oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -2027,12 +2037,14 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd: mtd info structure
  * @chip: nand chip info structure
  * @buf: data buffer
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  *
  * 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,
-				    struct nand_chip *chip, const uint8_t *buf)
+				    struct nand_chip *chip,
+				    const uint8_t *buf, bool use_oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -2071,21 +2083,23 @@ static void nand_write_page_syndrome(struct mtd_info *mtd,
  * @mtd: MTD device structure
  * @chip: NAND chip descriptor
  * @buf: the data to write
+ * @use_oob: caller placed non-0xFF data in chip->oob_poi
  * @page: page number to write
  * @cached: cached programming
  * @raw: use _raw version of write_page
  */
 static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-			   const uint8_t *buf, int page, int cached, int raw)
+			   const uint8_t *buf, bool use_oob, int page,
+			   int cached, int raw)
 {
 	int status;
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		chip->ecc.write_page_raw(mtd, chip, buf);
+		chip->ecc.write_page_raw(mtd, chip, buf, use_oob);
 	else
-		chip->ecc.write_page(mtd, chip, buf);
+		chip->ecc.write_page(mtd, chip, buf, use_oob);
 
 	/*
 	 * Cached progamming disabled for now. Not sure if it's worth the
@@ -2264,7 +2278,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
 
-		ret = chip->write_page(mtd, chip, wbuf, page, cached,
+		ret = chip->write_page(mtd, chip, wbuf, true, page, cached,
 				       (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index def50ca..6e69a76 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -682,14 +682,14 @@ static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 }
 
 static void pxa3xx_nand_write_page_hwecc(struct mtd_info *mtd,
-		struct nand_chip *chip, const uint8_t *buf)
+		struct nand_chip *chip, const uint8_t *buf, bool use_oob)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
 }
 
 static int pxa3xx_nand_read_page_hwecc(struct mtd_info *mtd,
-		struct nand_chip *chip, uint8_t *buf, int page)
+		struct nand_chip *chip, uint8_t *buf, bool use_oob, int page)
 {
 	struct pxa3xx_nand_host *host = mtd->priv;
 	struct pxa3xx_nand_info *info = host->info_data;
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index e9b2b26..93e7fe9 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -344,7 +344,7 @@ static void set_cmd_regs(struct mtd_info *mtd, uint32_t cmd, uint32_t flcmcdr_va
 }
 
 static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf, int page)
+				uint8_t *buf, bool use_oob, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -366,7 +366,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 static void flctl_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				   const uint8_t *buf)
+				   const uint8_t *buf, bool use_oob)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 1482340..ddc6591 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -363,15 +363,15 @@ struct nand_ecc_ctrl {
 	int (*correct)(struct mtd_info *mtd, uint8_t *dat, uint8_t *read_ecc,
 			uint8_t *calc_ecc);
 	int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
-			uint8_t *buf, int page);
+			uint8_t *buf, bool use_oob, int page);
 	void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf);
+			const uint8_t *buf, bool use_oob);
 	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			uint8_t *buf, int page);
+			uint8_t *buf, bool use_oob, 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,
-			const uint8_t *buf);
+			const uint8_t *buf, bool use_oob);
 	int (*write_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
 			int page);
 	int (*read_oob_raw)(struct mtd_info *mtd, struct nand_chip *chip,
@@ -505,7 +505,8 @@ struct nand_chip {
 	int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state,
 			int status, int page);
 	int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf, int page, int cached, int raw);
+			const uint8_t *buf, bool use_oob, int page,
+			int cached, int raw);
 
 	int chip_delay;
 	unsigned int options;
-- 
1.7.5.4.2.g519b1

[PATCH v2 2/2] mtd: nand: nand_base - pass proper 'use_oob' parameter

[Brian Norris]

We now have an interface for notifying the nand_ecc_ctrl functions when OOB
data must be returned to the upper layers and when it is simply needed for
internal calculations. This patch fills in the 'use_oob' parameter properly
from nand_do_{read,write}_ops. When utilized properly in the lower layers, this
parameter can improve performance for NAND HW that can simply avoid
transferring the OOB data.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_base.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c4989b5..089923c 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1477,6 +1477,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 		mtd->oobavail : mtd->oobsize;
 
 	uint8_t *bufpoi, *oob, *buf;
+	bool use_oob;
 
 	stats = mtd->ecc_stats;
 
@@ -1490,6 +1491,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 
 	buf = ops->datbuf;
 	oob = ops->oobbuf;
+	use_oob = oob ? true : false;
 
 	while (1) {
 		bytes = min(mtd->writesize - col, readlen);
@@ -1507,13 +1509,13 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			/* Now read the page into the buffer */
 			if (unlikely(ops->mode == MTD_OPS_RAW))
 				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
-							      true, page);
+							      use_oob, page);
 			else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
 				ret = chip->ecc.read_subpage(mtd, chip,
 							col, bytes, bufpoi);
 			else
 				ret = chip->ecc.read_page(mtd, chip, bufpoi,
-							  true, page);
+							  use_oob, page);
 			if (ret < 0) {
 				if (!aligned)
 					/* Invalidate page cache */
@@ -1536,7 +1538,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			buf += bytes;
 
 			if (unlikely(oob)) {
-
 				int toread = min(oobreadlen, max_oobsize);
 
 				if (toread) {
@@ -2216,6 +2217,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 	uint8_t *oob = ops->oobbuf;
 	uint8_t *buf = ops->datbuf;
 	int ret, subpage;
+	bool use_oob = oob ? true : false;
 
 	ops->retlen = 0;
 	if (!writelen)
@@ -2278,7 +2280,7 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			memset(chip->oob_poi, 0xff, mtd->oobsize);
 		}
 
-		ret = chip->write_page(mtd, chip, wbuf, true, page, cached,
+		ret = chip->write_page(mtd, chip, wbuf, use_oob, page, cached,
 				       (ops->mode == MTD_OPS_RAW));
 		if (ret)
 			break;
-- 
1.7.5.4.2.g519b1

Re: [PATCH v2 1/2] mtd: nand: add 'use_oob' argument to NAND {read,write}_page interfaces

[Shmulik Ladkani]

Hi Brian,

Thanks for the v2 adaptations.

Reviewing nand_base.c and nand.h (less familiar with the others).

BTW I think you missed one API convertion in gpmi-nand.c: there's a
call to 'chip->ecc.write_page_raw' not ported.

(still not completely happy with my suggestion, as the boolean
approach is not that elegant as you previously noted...)

On Mon, 23 Apr 2012 13:14:28 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index eb88d8b..c4989b5 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1066,12 +1066,13 @@ EXPORT_SYMBOL(nand_lock);
>   * @mtd: mtd info structure
>   * @chip: nand chip info structure
>   * @buf: buffer to store read data
> + * @use_oob: caller expects OOB data read to chip->oob_poi
>   * @page: page number to read
>   *
>   * Not for syndrome calculating ECC controllers, which use a special oob layout.
>   */
>  static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> -			      uint8_t *buf, int page)
> +			      uint8_t *buf, bool use_oob, int page)
>  {
>  	chip->read_buf(mtd, buf, mtd->writesize);
>  	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);

Why don't you adapt 'nand_read_page_raw' (and others) to use the
'use_oob' hint? Do you plan to?

IMO having a parameter called 'use_oob', but not using the parameter
value is very confusing.

Maybe we need an indication that states "hey, you may skip filling the
oob_poi if you don't want to, that's okay (but if you'd like to fill
it for your own purposes, it's okay too)"...
More of a 'oob_must_be_read' or 'oob_expected' for the read case (and
'oob_must_be_written' or 'oob_was_placed' for write).

(tried to come-up with better names, no success ;-)

>  static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
> -				uint8_t *buf, int page)
> +				uint8_t *buf, bool use_oob, int page)
>  {
>  	int i, eccsize = chip->ecc.size;
>  	int eccbytes = chip->ecc.bytes;
> @@ -1139,7 +1142,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>  	uint8_t *ecc_code = chip->buffers->ecccode;
>  	uint32_t *eccpos = chip->ecc.layout->eccpos;
>  
> -	chip->ecc.read_page_raw(mtd, chip, buf, page);
> +	chip->ecc.read_page_raw(mtd, chip, buf, use_oob, page);

Should pass 'true' instead of 'use_oob'.
That's because 'nand_read_page_swecc' later uses 'chip->oob_poi' and
expects it to be read.
(relevant for few other calls, e.g. nand_write_page_swecc)

BTW you used 'bool', however convention for some other boolean parameters
around nand_base.c is 'int'.
(no preference, just like things consistent)

Regards,
Shmulik

Re: [PATCH v2 1/2] mtd: nand: add 'use_oob' argument to NAND {read,write}_page interfaces

[Brian Norris]

Hi Shmulik,

On Tue, Apr 24, 2012 at 5:28 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> BTW I think you missed one API convertion in gpmi-nand.c: there's a
> call to 'chip->ecc.write_page_raw' not ported.

You're right. Thanks.

> (still not completely happy with my suggestion, as the boolean
> approach is not that elegant as you previously noted...)

I agree, but it seems like the primary alternative (using a 'uint8_t
*oob' parameter) necessitates many hacks such that 'oob' is mostly
reduced to a boolean. So I think I'll try to make the best of a
boolean, since we need *some* solution.

> On Mon, 23 Apr 2012 13:14:28 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index eb88d8b..c4989b5 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -1066,12 +1066,13 @@ EXPORT_SYMBOL(nand_lock);
>>   * @mtd: mtd info structure
>>   * @chip: nand chip info structure
>>   * @buf: buffer to store read data
>> + * @use_oob: caller expects OOB data read to chip->oob_poi
>>   * @page: page number to read
>>   *
>>   * Not for syndrome calculating ECC controllers, which use a special oob layout.
>>   */
>>  static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
>> -                           uint8_t *buf, int page)
>> +                           uint8_t *buf, bool use_oob, int page)
>>  {
>>       chip->read_buf(mtd, buf, mtd->writesize);
>>       chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
>
> Why don't you adapt 'nand_read_page_raw' (and others) to use the
> 'use_oob' hint?

I think this was because (a) I wasn't really sure what *all* users of
nand_read_page_raw, etc., expected from ecc.read_page_raw, etc., and
(b) I mostly need this patch series for a out-of-tree driver that
doesn't use the nand_base nand_ecc_ctrl functions :)

With proper usage of the 'use_oob' parameter, I think we can be
reasonably sure for (a)...but I'm still fearing random bugs. For
instance, is there any sort of hardware that expects the whole page +
OOB to be read via chip->read_buf() for all reads (note that
nand_read_page_raw may be used for NAND_ECC_HW)?

And (b) is kind of anti-open-source; if I want the interface change
for my driver, it should work reasonably well for others!

> Do you plan to?

Maybe. I find it a little difficult to sort through all the different
functions because as we've seen so far, different users expect
different results from the ecc.read_page, etc., interfaces. I can make
an effort at adapting each of the driver functions, but I'm never 100%
sure of the exact expected behavior without knowing more of the
hardware that uses them... The nand_base.c functions at least might be
manageable.

So I especially don't want to hack at the drivers that I can't build
or test. If there are obvious changes, I might submit a patch with the
prerequisite that someone can give a Tested-by or similar.

> IMO having a parameter called 'use_oob', but not using the parameter
> value is very confusing.
>
> Maybe we need an indication that states "hey, you may skip filling the
> oob_poi if you don't want to, that's okay (but if you'd like to fill
> it for your own purposes, it's okay too)"...
> More of a 'oob_must_be_read' or 'oob_expected' for the read case (and
> 'oob_must_be_written' or 'oob_was_placed' for write).
>
> (tried to come-up with better names, no success ;-)

I'm not sure about the naming. I thought 'use_oob' was better than
'has_oob'. I also was working for name that:

(1) could be used for both read and write
(2) wasn't too long

If I ignore (1) and add in:

(3) an indication that "you may skip filling the oob_poi if you don't
want to" (only when the boolean is false)

then I might come up with something like:

for 'read' functions: oob_expected
for 'write' functions: oob_provided

Or maybe something that hits all (1)-(3): oob_required. Then, saying
"oob_required = false" doesn't mean that you *can't* use oob_poi.

>>  static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>> -                             uint8_t *buf, int page)
>> +                             uint8_t *buf, bool use_oob, int page)
>>  {
>>       int i, eccsize = chip->ecc.size;
>>       int eccbytes = chip->ecc.bytes;
>> @@ -1139,7 +1142,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
>>       uint8_t *ecc_code = chip->buffers->ecccode;
>>       uint32_t *eccpos = chip->ecc.layout->eccpos;
>>
>> -     chip->ecc.read_page_raw(mtd, chip, buf, page);
>> +     chip->ecc.read_page_raw(mtd, chip, buf, use_oob, page);
>
> Should pass 'true' instead of 'use_oob'.
> That's because 'nand_read_page_swecc' later uses 'chip->oob_poi' and
> expects it to be read.
> (relevant for few other calls, e.g. nand_write_page_swecc)

Right, thanks. By "few other calls," did you mean that only
nand_write_page_swecc and nand_read_page_swecc were relevant? I didn't
find any others I missed that were used the same way.

> BTW you used 'bool', however convention for some other boolean parameters
> around nand_base.c is 'int'.
> (no preference, just like things consistent)

For reference, are the instances of 'int sndcmd' the only cases of
int-used-as-boolean that you found? bool is clearer IMO, but for the
sake of consistency I can switch. But now I just noticed docg4.c and
denali.c use 'bool'...

Thanks for the review.

Brian

Re: [PATCH v2 1/2] mtd: nand: add 'use_oob' argument to NAND {read,write}_page interfaces

[Shmulik Ladkani]

Hi Brian,

On Tue, 24 Apr 2012 20:42:02 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
> > (still not completely happy with my suggestion, as the boolean
> > approach is not that elegant as you previously noted...)
> 
> I agree, but it seems like the primary alternative (using a 'uint8_t
> *oob' parameter) necessitates many hacks such that 'oob' is mostly
> reduced to a boolean. So I think I'll try to make the best of a
> boolean, since we need *some* solution.

Agreed, it is the primary (and reasonable) alternative.
(I was only hoping for an even better suggestion, but none so far)

> > IMO having a parameter called 'use_oob', but not using the parameter
> > value is very confusing.
> >
> > Maybe we need an indication that states "hey, you may skip filling the
> > oob_poi if you don't want to, that's okay (but if you'd like to fill
> > it for your own purposes, it's okay too)"...
> > More of a 'oob_must_be_read' or 'oob_expected' for the read case (and
> > 'oob_must_be_written' or 'oob_was_placed' for write).
> >
> > (tried to come-up with better names, no success ;-)
> 
> I'm not sure about the naming. I thought 'use_oob' was better than
> 'has_oob'. I also was working for name that:
> 
> (1) could be used for both read and write
> (2) wasn't too long
> 
> If I ignore (1) and add in:
> 
> (3) an indication that "you may skip filling the oob_poi if you don't
> want to" (only when the boolean is false)
> 
> then I might come up with something like:
> 
> for 'read' functions: oob_expected
> for 'write' functions: oob_provided
> 
> Or maybe something that hits all (1)-(3): oob_required. Then, saying
> "oob_required = false" doesn't mean that you *can't* use oob_poi.

I like all of your suggestions (oob_expected, oob_provided, and
also oob_required). Better than 'has_oob' or 'use_oob'.

These names implicitly suggest that we don't have to immediately port
those implementors that do fill/use 'oob_poi' even if it's not really
required.

> >> @@ -1139,7 +1142,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
> >>    uint8_t *ecc_code = chip->buffers->ecccode;
> >>    uint32_t *eccpos = chip->ecc.layout->eccpos;
> >>
> >> -   chip->ecc.read_page_raw(mtd, chip, buf, page);
> >> +   chip->ecc.read_page_raw(mtd, chip, buf, use_oob, page);
> >
> > Should pass 'true' instead of 'use_oob'.
> > That's because 'nand_read_page_swecc' later uses 'chip->oob_poi' and
> > expects it to be read.
> > (relevant for few other calls, e.g. nand_write_page_swecc)
> 
> Right, thanks. By "few other calls," did you mean that only
> nand_write_page_swecc and nand_read_page_swecc were relevant? I didn't
> find any others I missed that were used the same way.

Took another look at nand_base.c, only these two (for some reason I
initially thought, mistakenly, that the pattern is more widespread).

> > BTW you used 'bool', however convention for some other boolean parameters
> > around nand_base.c is 'int'.
> > (no preference, just like things consistent)
> 
> For reference, are the instances of 'int sndcmd' the only cases of
> int-used-as-boolean that you found? bool is clearer IMO, but for the
> sake of consistency I can switch. But now I just noticed docg4.c and
> denali.c use 'bool'...

I only looked at nand_base.c. Seems like 'int' more used: int getchip,
int allowbbt, int cached, int raw.

Also, grepping 'bool' within include/linux/mtd/* yielded just one
result; obviously there are interfaces that get or return a boolean.

I'm not familiar with current coding preference (kernel global or mtd
local), just prefer to be consistent.

Regards,
Shmulik

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 7717 bytes --]

On Mon, 2012-04-23 at 13:14 -0700, Brian Norris wrote:
> Artem: can you perform your regular compile tests? I compile-tested as many as
> I could.

Here is what Aiaiai says when I test your patch-set. Note, the mackrell
failure is not your fault - is does not compile anymore and I did not
have time to look at it. But you broke the mxs build.

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_i386_defconfig,i386"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_i386_defconfig,i386", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_generic_defconfig,ia64,ia64-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_generic_defconfig,ia64,ia64-linux-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_ppc6xx_defconfig,powerpc,powerpc-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_ppc6xx_defconfig,powerpc,powerpc-linux-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_omap2plus_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_omap2plus_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_imx_v6_v7_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_imx_v6_v7_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------
Failed to build the following commit for configuration "l2_mxs_defconfig" (architecture arm)":

d8d1be5 mtd: nand: nand_base - pass proper 'use_oob' parameter

drivers/mtd/lpddr/lpddr_cmds.c:751:5: warning: no previous prototype for 'word_program' [-Wmissing-prototypes]
In file included from include/linux/kernel.h:19:0,
                 from include/linux/clk.h:15,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.c:21:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
In file included from include/linux/dma-mapping.h:73:0,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.h:22,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.c:27:
arch/arm/include/asm/dma-mapping.h: In function 'dma_mapping_error':
arch/arm/include/asm/dma-mapping.h:126:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'release_bch_irq':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:381:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'gpmi_dma_filter':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:400:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:400:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'mx23_check_transcription_stamp':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1194:9: warning: variable 'byte' set but not used [-Wunused-but-set-variable]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'mx23_write_transcription_stamp':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1311:3: error: too few arguments to function 'chip->ecc.write_page_raw'
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1255:15: warning: variable 'byte' set but not used [-Wunused-but-set-variable]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: At top level:
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1456:6: warning: no previous prototype for 'gpmi_nfc_exit' [-Wmissing-prototypes]
make[5]: *** [drivers/mtd/nand/gpmi-nand/gpmi-nand.o] Error 1

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_lantiq_defconfig,mips,mips-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_lantiq_defconfig,mips,mips-linux-", no issues

--------------------------------------------------------------------------------
Failed to build the following commit for configuration "l2_mackerel_defconfig" (architecture arm)":

54043ff MIPS: Kbuild: remove -Werror

In file included from include/linux/kernel.h:19:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from drivers/mtd/nand/sh_flctl.c:24:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c: In function 'set_addr':
drivers/mtd/nand/sh_flctl.c:117:25: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c: In function 'flctl_cmdfunc':
drivers/mtd/nand/sh_flctl.c:611:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c:638:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
In file included from include/linux/kernel.h:19:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from init/version.c:10:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
arch/arm/mach-shmobile/built-in.o: In function `mackerel_sdhi0_gpio_cd':
pfc-sh7372.c:(.text+0x33ec): undefined reference to `mmc_detect_change'
make[1]: *** [.tmp_vmlinux1] Error 1

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_u300_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_u300_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 7719 bytes --]

On Mon, 2012-04-23 at 13:14 -0700, Brian Norris wrote:
> Artem: can you perform your regular compile tests? I compile-tested as many as
> I could.

Here is what Aiaiai says when I test your patch-set. Note, the mackrell
failure is not your fault - is does not compile anymore and I did not
have time to look at it. But you broke the mxs build.

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_i386_defconfig,i386"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_i386_defconfig,i386", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_generic_defconfig,ia64,ia64-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_generic_defconfig,ia64,ia64-linux-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_ppc6xx_defconfig,powerpc,powerpc-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_ppc6xx_defconfig,powerpc,powerpc-linux-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_omap2plus_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_omap2plus_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_at91cap9_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_imx_v6_v7_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_imx_v6_v7_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------
Failed to build the following commit for configuration "l2_mxs_defconfig" (architecture arm)":

d8d1be5 mtd: nand: nand_base - pass proper 'use_oob' parameter

drivers/mtd/lpddr/lpddr_cmds.c:751:5: warning: no previous prototype for 'word_program' [-Wmissing-prototypes]
In file included from include/linux/kernel.h:19:0,
                 from include/linux/clk.h:15,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.c:21:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
In file included from include/linux/dma-mapping.h:73:0,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.h:22,
                 from drivers/mtd/nand/gpmi-nand/gpmi-nand.c:27:
arch/arm/include/asm/dma-mapping.h: In function 'dma_mapping_error':
arch/arm/include/asm/dma-mapping.h:126:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'release_bch_irq':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:381:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'gpmi_dma_filter':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:400:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:400:49: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'mx23_check_transcription_stamp':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1194:9: warning: variable 'byte' set but not used [-Wunused-but-set-variable]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'mx23_write_transcription_stamp':
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1311:3: error: too few arguments to function 'chip->ecc.write_page_raw'
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1255:15: warning: variable 'byte' set but not used [-Wunused-but-set-variable]
drivers/mtd/nand/gpmi-nand/gpmi-nand.c: At top level:
drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1456:6: warning: no previous prototype for 'gpmi_nfc_exit' [-Wmissing-prototypes]
make[5]: *** [drivers/mtd/nand/gpmi-nand/gpmi-nand.o] Error 1

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_lantiq_defconfig,mips,mips-linux-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_lantiq_defconfig,mips,mips-linux-", no issues

--------------------------------------------------------------------------------
Failed to build the following commit for configuration "l2_mackerel_defconfig" (architecture arm)":

54043ff MIPS: Kbuild: remove -Werror

In file included from include/linux/kernel.h:19:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from drivers/mtd/nand/sh_flctl.c:24:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c: In function 'set_addr':
drivers/mtd/nand/sh_flctl.c:117:25: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c: In function 'flctl_cmdfunc':
drivers/mtd/nand/sh_flctl.c:611:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
drivers/mtd/nand/sh_flctl.c:638:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
In file included from include/linux/kernel.h:19:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/stat.h:60,
                 from include/linux/module.h:10,
                 from init/version.c:10:
include/linux/bitops.h: In function 'hweight_long':
include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
arch/arm/mach-shmobile/built-in.o: In function `mackerel_sdhi0_gpio_cd':
pfc-sh7372.c:(.text+0x33ec): undefined reference to `mmc_detect_change'
make[1]: *** [.tmp_vmlinux1] Error 1

--------------------------------------------------------------------------------

Bisectability test results for configuration "l2_u300_defconfig,arm,arm-unknown-linux-gnueabi-"

Bisecability test passed

--------------------------------------------------------------------------------

Successfully built configuration "l2_u300_defconfig,arm,arm-unknown-linux-gnueabi-", no issues

--------------------------------------------------------------------------------

-- 
Best Regards,
Artem Bityutskiy


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Bastian Hecht]

Hello,

this should be an sh_flctl unrelated problem. Guennadi, have you seen
this error somewhere before?

thanks,

 Bastian Hecht

> --------------------------------------------------------------------------------
> Failed to build the following commit for configuration "l2_mackerel_defconfig" (architecture arm)":
>
> 54043ff MIPS: Kbuild: remove -Werror
>
> In file included from include/linux/kernel.h:19:0,
>                 from include/linux/cache.h:4,
>                 from include/linux/time.h:7,
>                 from include/linux/stat.h:60,
>                 from include/linux/module.h:10,
>                 from drivers/mtd/nand/sh_flctl.c:24:
> include/linux/bitops.h: In function 'hweight_long':
> include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> drivers/mtd/nand/sh_flctl.c: In function 'set_addr':
> drivers/mtd/nand/sh_flctl.c:117:25: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> drivers/mtd/nand/sh_flctl.c: In function 'flctl_cmdfunc':
> drivers/mtd/nand/sh_flctl.c:611:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> drivers/mtd/nand/sh_flctl.c:638:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> In file included from include/linux/kernel.h:19:0,
>                 from include/linux/cache.h:4,
>                 from include/linux/time.h:7,
>                 from include/linux/stat.h:60,
>                 from include/linux/module.h:10,
>                 from init/version.c:10:
> include/linux/bitops.h: In function 'hweight_long':
> include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> arch/arm/mach-shmobile/built-in.o: In function `mackerel_sdhi0_gpio_cd':
> pfc-sh7372.c:(.text+0x33ec): undefined reference to `mmc_detect_change'
> make[1]: *** [.tmp_vmlinux1] Error 1
>

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Guennadi Liakhovetski]

Hi Bastian

On Wed, 25 Apr 2012, Bastian Hecht wrote:

> Hello,
> 
> this should be an sh_flctl unrelated problem. Guennadi, have you seen
> this error somewhere before?

You're #111:

https://lkml.org/lkml/2012/4/19/621

Thanks
Guennadi

> 
> thanks,
> 
>  Bastian Hecht
> 
> > --------------------------------------------------------------------------------
> > Failed to build the following commit for configuration "l2_mackerel_defconfig" (architecture arm)":
> >
> > 54043ff MIPS: Kbuild: remove -Werror
> >
> > In file included from include/linux/kernel.h:19:0,
> >                 from include/linux/cache.h:4,
> >                 from include/linux/time.h:7,
> >                 from include/linux/stat.h:60,
> >                 from include/linux/module.h:10,
> >                 from drivers/mtd/nand/sh_flctl.c:24:
> > include/linux/bitops.h: In function 'hweight_long':
> > include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> > drivers/mtd/nand/sh_flctl.c: In function 'set_addr':
> > drivers/mtd/nand/sh_flctl.c:117:25: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > drivers/mtd/nand/sh_flctl.c: In function 'flctl_cmdfunc':
> > drivers/mtd/nand/sh_flctl.c:611:15: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > drivers/mtd/nand/sh_flctl.c:638:28: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > In file included from include/linux/kernel.h:19:0,
> >                 from include/linux/cache.h:4,
> >                 from include/linux/time.h:7,
> >                 from include/linux/stat.h:60,
> >                 from include/linux/module.h:10,
> >                 from init/version.c:10:
> > include/linux/bitops.h: In function 'hweight_long':
> > include/linux/bitops.h:66:26: warning: signed and unsigned type in conditional expression [-Wsign-compare]
> > arch/arm/mach-shmobile/built-in.o: In function `mackerel_sdhi0_gpio_cd':
> > pfc-sh7372.c:(.text+0x33ec): undefined reference to `mmc_detect_change'
> > make[1]: *** [.tmp_vmlinux1] Error 1
> >
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Brian Norris]

Hi Artem,

On Wed, Apr 25, 2012 at 8:38 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Mon, 2012-04-23 at 13:14 -0700, Brian Norris wrote:
>> Artem: can you perform your regular compile tests? I compile-tested as many as
>> I could.
>
> Here is what Aiaiai says when I test your patch-set. Note, the mackrell
> failure is not your fault - is does not compile anymore and I did not
> have time to look at it. But you broke the mxs build.

Thanks for the compile test!

> --------------------------------------------------------------------------------
> Failed to build the following commit for configuration "l2_mxs_defconfig" (architecture arm)":
>
> d8d1be5 mtd: nand: nand_base - pass proper 'use_oob' parameter
>
...
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c: In function 'mx23_write_transcription_stamp':
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c:1311:3: error: too few arguments to function 'chip->ecc.write_page_raw'

Shmulik caught this one for me. It's now fixed in my tree and will be
included in my next - and hopefully last - revision, v3.

Thanks again.

Brian

P.S. It seems like you have sent out 2 almost identical copies of this
email. I don't believe this is the first time, so I thought I'd let
you know.

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Jiandong Zheng]

On 4/23/2012 1:14 PM, Brian Norris wrote:
> Brian Norris (2):
>    mtd: nand: add 'use_oob' argument to NAND {read,write}_page
>      interfaces
>    mtd: nand: nand_base - pass proper 'use_oob' parameter
>
>   drivers/mtd/nand/bcm_umi_bch.c         |   10 +++--
>   drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
>   drivers/mtd/nand/nand_base.c           |   58 ++++++++++++++++++++-----------
>   include/linux/mtd/nand.h               |   11 +++---
>   15 files changed, 89 insertions(+), 70 deletions(-)
>
I've built and run these successfully on bcmring.

Acked-by: Jiandong Zheng <jdzheng@broadcom.com>

Re: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[Brian Norris]

On Tue, May 1, 2012 at 12:02 PM, Jiandong Zheng <jdzheng@broadcom.com> wrote:
> On 4/23/2012 1:14 PM, Brian Norris wrote:
>>
>> Brian Norris (2):
>>   mtd: nand: add 'use_oob' argument to NAND {read,write}_page
>>     interfaces
>>   mtd: nand: nand_base - pass proper 'use_oob' parameter
>>
>>  drivers/mtd/nand/bcm_umi_bch.c         |   10 +++--
>>  drivers/mtd/nand/bcm_umi_nand.c        |    2 +-
>>  drivers/mtd/nand/nand_base.c           |   58
>> ++++++++++++++++++++-----------
>>
>>  include/linux/mtd/nand.h               |   11 +++---
>>  15 files changed, 89 insertions(+), 70 deletions(-)
>>
> I've built and run these successfully on bcmring.
>
> Acked-by: Jiandong Zheng <jdzheng@broadcom.com>

Great, thanks! I've sent a v3 (might not have CC'd you that time) and
will be sending v4 soon. It may be worth testing v3 or v4. Let me know
if you want your Ack's pasted onto any particular patch.

Thanks,
Brian

RE: [PATCH v2 0/2] mtd: nand: rework nand_ecc_ctrl interface for OOB

[JD (Jiandong) Zheng]

> Great, thanks! I've sent a v3 (might not have CC'd you that time) and
> will be sending v4 soon. It may be worth testing v3 or v4. Let me know
> if you want your Ack's pasted onto any particular patch.

I can test new version. I'd like my ack to add to patches for generic
 code and Bcm_umi code.

> Thanks,
> Brian