Skip to contentSkip to conversation

mtd: nand: Support controllers with custom page accessors

PATCH Superseded Marc Gonzalez 2 participants 10 messages Nov 14, 2016 – Nov 19, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mtd: nand: Support controllers with custom page accessors

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

---
 drivers/mtd/nand/nand_base.c  | 31 ++++++++++++++++++++++++++++---
 drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
 include/linux/mtd/nand.h      | 12 ++++++++++++
 3 files changed, 52 insertions(+), 12 deletions(-)
drivers/mtd/nand/nand_base.c +28 -3
index 50cdf37cb8e4..b8ea6b397c16 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_default_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_default_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_default_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_default_page_accessors(ecc))
+		return false;
+
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (WARN_ON(invalid_ecc_page_accessors(chip)))
+		return -EINVAL;
+
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
drivers/mtd/nand/tango_nand.c +12 -9
index fd8cf414cb2b..7ed35348993e 100644
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
  * buf = |      PKT_0      | ... |      PKT_N      |
  *       +-----------------+-----+-----------------+
  */
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 {
 	u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_read(chip, &buf, pkt_size - rem, &pos);
 	aux_read(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 {
 	const u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_write(chip, &buf, pkt_size - rem, &pos);
 	aux_write(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
-	return raw_read(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	raw_read(chip, buf, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			  int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, NULL, chip->oob_poi);
+	raw_read(chip, NULL, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	ecc->write_page = tango_write_page;
 	ecc->read_oob = tango_read_oob;
 	ecc->write_oob = tango_write_oob;
+	ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
 
 	err = nand_scan_tail(mtd);
 	if (err)
include/linux/mtd/nand.h +12 -0
index 06d0c9d740f7..a95a27a30005 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !(chip->options & NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_default_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
Signature · 1 line
2.9.0
#1 lore raw

Hi Marc,

Quoted 86 lines · On Mon, 14 Nov 2016 10:47:38 +0100 Marc Gonzalez…

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

---
 drivers/mtd/nand/nand_base.c  | 31 ++++++++++++++++++++++++++++---
 drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
 include/linux/mtd/nand.h      | 12 ++++++++++++
 3 files changed, 52 insertions(+), 12 deletions(-)
drivers/mtd/nand/nand_base.c +28 -3
index 50cdf37cb8e4..b8ea6b397c16 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_default_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_default_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_default_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_default_page_accessors(ecc))
+		return false;
+
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (WARN_ON(invalid_ecc_page_accessors(chip)))
+		return -EINVAL;
+
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
drivers/mtd/nand/tango_nand.c +0 -0
index fd8cf414cb2b..7ed35348993e 100644

drivers/mtd/nand/tango_nand.c static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos) · line 346

Quoted 5 lines
drivers/mtd/nand/tango_nand.c +1 -1
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
  * buf = |      PKT_0      | ... |      PKT_N      |
  *       +-----------------+-----+-----------------+
  */
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)

This change is completely unrelated to the custom page accessors patch. Please separate the function prototype changes in a separate patch.

Quoted 79 lines
drivers/mtd/nand/tango_nand.c +11 -8
 {
 	u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_read(chip, &buf, pkt_size - rem, &pos);
 	aux_read(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *
 {
 	const u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_write(chip, &buf, pkt_size - rem, &pos);
 	aux_write(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
-	return raw_read(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	raw_read(chip, buf, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			  int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, NULL, chip->oob_poi);
+	raw_read(chip, NULL, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	ecc->write_page = tango_write_page;
 	ecc->read_oob = tango_read_oob;
 	ecc->write_oob = tango_write_oob;
+	ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
 
 	err = nand_scan_tail(mtd);
 	if (err)
include/linux/mtd/nand.h +6 -0
index 06d0c9d740f7..a95a27a30005 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80

include/linux/mtd/nand.h enum nand_ecc_algo { · line 195

Quoted 4 lines
include/linux/mtd/nand.h +1 -0
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !(chip->options & NAND_NO_SUBPAGE_WRITE)
					  (chip)->options

Just in case the chip parameter is something like that &x->chip.

include/linux/mtd/nand.h struct nand_ecc_ctrl { · line 578

Quoted 8 lines
include/linux/mtd/nand.h +1 -0
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_default_page_accessors(struct nand_ecc_ctrl *ecc)

Not sure the term default is correct here: some drivers implement their own ->read/write_page() methods, but expect the core to send the READ/PROG commands.
How about renaming it nand_standard_page_accessors()?

Quoted 7 lines
include/linux/mtd/nand.h +4 -0
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
#2 lore raw

PATCH v2 1/2 mtd: nand: Support controllers with custom page

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

---
v2:
(chip) in NAND_HAS_SUBPAGE_WRITE macro s/nand_default_page_accessors/nand_standard_page_accessors/ scripts/checkpatch.pl --strict v2-0001-mtd-nand-Support-controllers-with-custom-page-acc.patch
total: 0 errors, 0 warnings, 0 checks, 91 lines checked

---
 drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
 include/linux/mtd/nand.h     | 12 ++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)
drivers/mtd/nand/nand_base.c +28 -3
index 50cdf37cb8e4..db5f27db3748 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_standard_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_standard_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_standard_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_standard_page_accessors(ecc))
+		return false;
+
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (WARN_ON(invalid_ecc_page_accessors(chip)))
+		return -EINVAL;
+
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
include/linux/mtd/nand.h +12 -0
index 06d0c9d740f7..c5f3a012ae62 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
Signature · 1 line
2.9.0
#3 lore raw

Re: [PATCH v2 1/2] mtd: nand: Support controllers with custom page

Hi Marc,

Quoted 61 lines · On Mon, 14 Nov 2016 18:01:27 +0100 Marc Gonzalez…

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

---
v2:
(chip) in NAND_HAS_SUBPAGE_WRITE macro s/nand_default_page_accessors/nand_standard_page_accessors/ scripts/checkpatch.pl --strict v2-0001-mtd-nand-Support-controllers-with-custom-page-acc.patch
total: 0 errors, 0 warnings, 0 checks, 91 lines checked

---
 drivers/mtd/nand/nand_base.c | 31 ++++++++++++++++++++++++++++---
 include/linux/mtd/nand.h     | 12 ++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)
drivers/mtd/nand/nand_base.c +13 -3
index 50cdf37cb8e4..db5f27db3748 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_standard_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_standard_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_standard_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,25 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_standard_page_accessors(ecc))
+		return false;
+

drivers/mtd/nand/nand_base.c static bool nand_ecc_strength_good(struct mtd_info *mtd) · line 4561

Quoted 10 lines
drivers/mtd/nand/nand_base.c +10 -0
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage));

Just had a closer look at the nand_write_page() function, and it seems that !NAND_NO_SUBPAGE_WRITE && !ecc->write_subpage is valid use case. In this situation, nand_write_page() uses ecc->write_page().

I know it's a real mess, and each time I look at this subpage support detection, I have a hard time remembering how it works, and why it's done this way. The idea is that, even if the controller does not explicitly implement ->write_subpage(), the core will fill the page buffer with 0xff, and since programming NANDs is just about moving bits from 1 to zero, when you leave bits to one, they are just and-ed with the previous content.

The correct test is:

	return (!ecc->read_page || !ecc->write_page ||
		!ecc->read_page_raw || !ecc->write_page_raw ||
		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
		 ecc->hwctl && ecc->calculate))

drivers/mtd/nand/nand_base.c int nand_scan_tail(struct mtd_info *mtd) · line 4586

Quoted 12 lines
drivers/mtd/nand/nand_base.c +5 -0
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4581,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (WARN_ON(invalid_ecc_page_accessors(chip)))
+		return -EINVAL;
+

Sorry, I didn't notice that one in my previous review. We're currently trying to get rid of all BUG() and WARN_ON() calls in the NAND framework. Can you replace this WARN_ON() by a pr_err() message?

Quoted 40 lines
drivers/mtd/nand/nand_base.c +0 -0
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
include/linux/mtd/nand.h +12 -0
index 06d0c9d740f7..c5f3a012ae62 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
#4 lore raw

PATCH v3 1/3 mtd: nand: Support controllers with custom page

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

---
v3:
ecc->write_subpage handling
don't use WARN_ON

---
 drivers/mtd/nand/nand_base.c | 34 +++++++++++++++++++++++++++++++---
 include/linux/mtd/nand.h     | 12 ++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)
drivers/mtd/nand/nand_base.c +31 -3
index 50cdf37cb8e4..e7034f3bcace 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_standard_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_standard_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_standard_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,26 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_standard_page_accessors(ecc))
+		return false;
+
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
+		 ecc->hwctl && ecc->calculate));
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4582,11 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (invalid_ecc_page_accessors(chip)) {
+		pr_err("Invalid ECC page accessors setup\n");
+		return -EINVAL;
+	}
+
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
include/linux/mtd/nand.h +12 -0
index 06d0c9d740f7..c5f3a012ae62 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
Signature · 1 line
2.9.0
#5 lore raw

Re: [PATCH v3 1/3] mtd: nand: Support controllers with custom page

Hi Mark,

Quoted 5 lines · On Tue, 15 Nov 2016 10:56:20 +0100 Marc Gonzalez…

If your controller already sends the required NAND commands when reading or writing a page, then the framework is not supposed to send READ0 and SEQIN/PAGEPROG respectively.

Applied the whole series.

Just a small request for your future contributions, can you please group the patches of a specific version in the same thread instead of making them answers to my previous reviews?

Thanks,

Boris

Quoted 119 lines

---
v3:
ecc->write_subpage handling
don't use WARN_ON

---
 drivers/mtd/nand/nand_base.c | 34 +++++++++++++++++++++++++++++++---
 include/linux/mtd/nand.h     | 12 ++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)
drivers/mtd/nand/nand_base.c +31 -3
index 50cdf37cb8e4..e7034f3bcace 100644
@@ -1970,7 +1970,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 						 __func__, buf);
 
 read_retry:
-			chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+			if (nand_standard_page_accessors(&chip->ecc))
+				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 
 			/*
 			 * Now read the page into the buffer.  Absent an error,
@@ -2658,7 +2659,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	else
 		subpage = 0;
 
-	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
+	if (nand_standard_page_accessors(&chip->ecc))
+		chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
 		status = chip->ecc.write_page_raw(mtd, chip, buf,
@@ -2681,7 +2683,8 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	if (!cached || !NAND_HAS_CACHEPROG(chip)) {
 
-		chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+		if (nand_standard_page_accessors(&chip->ecc))
+			chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
 		/*
 		 * See if operation failed and additional status checks are
@@ -4539,6 +4542,26 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
+static bool invalid_ecc_page_accessors(struct nand_chip *chip)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+
+	if (nand_standard_page_accessors(ecc))
+		return false;
+
+	/*
+	 * NAND_ECC_CUSTOM_PAGE_ACCESS flag is set, make sure the NAND
+	 * controller driver implements all the page accessors because
+	 * default helpers are not suitable when the core does not
+	 * send the READ0/PAGEPROG commands.
+	 */
+	return (!ecc->read_page || !ecc->write_page ||
+		!ecc->read_page_raw || !ecc->write_page_raw ||
+		(NAND_HAS_SUBPAGE_READ(chip) && !ecc->read_subpage) ||
+		(NAND_HAS_SUBPAGE_WRITE(chip) && !ecc->write_subpage &&
+		 ecc->hwctl && ecc->calculate));
+}
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4559,6 +4582,11 @@ int nand_scan_tail(struct mtd_info *mtd)
 		   !(chip->bbt_options & NAND_BBT_USE_FLASH)))
 		return -EINVAL;
 
+	if (invalid_ecc_page_accessors(chip)) {
+		pr_err("Invalid ECC page accessors setup\n");
+		return -EINVAL;
+	}
+
 	if (!(chip->options & NAND_OWN_BUFFERS)) {
 		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
 				+ mtd->oobsize * 3, GFP_KERNEL);
include/linux/mtd/nand.h +12 -0
index 06d0c9d740f7..c5f3a012ae62 100644
@@ -142,6 +142,12 @@ enum nand_ecc_algo {
  */
 #define NAND_ECC_GENERIC_ERASED_CHECK	BIT(0)
 #define NAND_ECC_MAXIMIZE		BIT(1)
+/*
+ * If your controller already sends the required NAND commands when
+ * reading or writing a page, then the framework is not supposed to
+ * send READ0 and SEQIN/PAGEPROG respectively.
+ */
+#define NAND_ECC_CUSTOM_PAGE_ACCESS	BIT(2)
 
 /* Bit mask for flags passed to do_nand_read_ecc */
 #define NAND_GET_DEVICE		0x80
@@ -186,6 +192,7 @@ enum nand_ecc_algo {
 /* Macros to identify the above */
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_SUBPAGE_READ(chip) ((chip->options & NAND_SUBPAGE_READ))
+#define NAND_HAS_SUBPAGE_WRITE(chip) !((chip)->options & NAND_NO_SUBPAGE_WRITE)
 
 /* Non chip related options */
 /* This option skips the bbt scan during initialization. */
@@ -568,6 +575,11 @@ struct nand_ecc_ctrl {
 			int page);
 };
 
+static inline int nand_standard_page_accessors(struct nand_ecc_ctrl *ecc)
+{
+	return !(ecc->options & NAND_ECC_CUSTOM_PAGE_ACCESS);
+}
+
 /**
  * struct nand_buffers - buffer structure for read/write
  * @ecccalc:	buffer pointer for calculated ECC, size is oobsize.
#6 lore raw

PATCH v2 2/2 mtd: nand: tango: Enable custom page accessors

In reply to Boris Brezillon

Enable NAND_ECC_CUSTOM_PAGE_ACCESS in tango NFC driver. Fixup "raw" page accessors to send proper NAND commands. Drop raw_write/raw_read return values (no longer used).

---
v2:
split from previous v1 patch

---
 drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/mtd/nand/tango_nand.c +12 -9
index fd8cf414cb2b..7ed35348993e 100644
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
  * buf = |      PKT_0      | ... |      PKT_N      |
  *       +-----------------+-----+-----------------+
  */
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 {
 	u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_read(chip, &buf, pkt_size - rem, &pos);
 	aux_read(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 {
 	const u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_write(chip, &buf, pkt_size - rem, &pos);
 	aux_write(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
-	return raw_read(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	raw_read(chip, buf, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			  int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, NULL, chip->oob_poi);
+	raw_read(chip, NULL, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	ecc->write_page = tango_write_page;
 	ecc->read_oob = tango_read_oob;
 	ecc->write_oob = tango_write_oob;
+	ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
 
 	err = nand_scan_tail(mtd);
 	if (err)
Signature · 1 line
2.9.0
#7 lore raw

Re: [PATCH v2 2/2] mtd: nand: tango: Enable custom page accessors

Quoted 3 lines · On Mon, 14 Nov 2016 18:06:45 +0100 Marc Gonzalez…

Enable NAND_ECC_CUSTOM_PAGE_ACCESS in tango NFC driver. Fixup "raw" page accessors to send proper NAND commands. Drop raw_write/raw_read return values (no longer used).

I'd still prefer to have two separate patches:

1/ change the raw_read() raw_write() prototypes
2/ switch to NAND_ECC_CUSTOM_PAGE_ACCESS

Quoted 80 lines

---
v2:
split from previous v1 patch

---
 drivers/mtd/nand/tango_nand.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/mtd/nand/tango_nand.c +12 -9
index fd8cf414cb2b..7ed35348993e 100644
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
  * buf = |      PKT_0      | ... |      PKT_N      |
  *       +-----------------+-----+-----------------+
  */
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 {
 	u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_read(chip, &buf, pkt_size - rem, &pos);
 	aux_read(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 {
 	const u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -395,27 +393,31 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_write(chip, &buf, pkt_size - rem, &pos);
 	aux_write(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
-	return raw_read(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	raw_read(chip, buf, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			  int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, NULL, chip->oob_poi);
+	raw_read(chip, NULL, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +555,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	ecc->write_page = tango_write_page;
 	ecc->read_oob = tango_read_oob;
 	ecc->write_oob = tango_write_oob;
+	ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
 
 	err = nand_scan_tail(mtd);
 	if (err)
#8 lore raw

PATCH v3 2/3 mtd: nand: tango: Enable custom page accessors

Enable NAND_ECC_CUSTOM_PAGE_ACCESS in the tango NFC driver. Fixup the "raw" page accessors to send the proper NAND commands.

---
 drivers/mtd/nand/tango_nand.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/mtd/nand/tango_nand.c +6 -1
index fd8cf414cb2b..bacc2a02b7fe 100644
@@ -402,13 +402,17 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
 	return raw_read(chip, buf, chip->oob_poi);
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 				const u8 *buf, int oob_required, int page)
 {
-	return raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0, page);
+	raw_write(chip, buf, chip->oob_poi);
+	chip->cmdfunc(mtd, NAND_CMD_PAGEPROG, -1, -1);
+	return 0;
 }
 
 static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
@@ -553,6 +557,7 @@ static int chip_init(struct device *dev, struct device_node *np)
 	ecc->write_page = tango_write_page;
 	ecc->read_oob = tango_read_oob;
 	ecc->write_oob = tango_write_oob;
+	ecc->options = NAND_ECC_CUSTOM_PAGE_ACCESS;
 
 	err = nand_scan_tail(mtd);
 	if (err)
Signature · 1 line
2.9.0
#9 lore raw

PATCH v3 3/3 mtd: nand: tango: Cleanup raw_write and raw_read

In reply to Boris Brezillon

Drop raw_write return value (no longer used).
Drop raw_read return value (for symmetry).

---
 drivers/mtd/nand/tango_nand.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/mtd/nand/tango_nand.c +6 -8
index bacc2a02b7fe..7ed35348993e 100644
@@ -343,7 +343,7 @@ static void aux_write(struct nand_chip *chip, const u8 **buf, int len, int *pos)
  * buf = |      PKT_0      | ... |      PKT_N      |
  *       +-----------------+-----+-----------------+
  */
-static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
+static void raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 {
 	u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -367,11 +367,9 @@ static int raw_read(struct nand_chip *chip, u8 *buf, u8 *oob)
 	aux_read(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_read(chip, &buf, pkt_size - rem, &pos);
 	aux_read(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
-static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
+static void raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 {
 	const u8 *oob_orig = oob;
 	const int page_size = chip->mtd.writesize;
@@ -395,15 +393,14 @@ static int raw_write(struct nand_chip *chip, const u8 *buf, const u8 *oob)
 	aux_write(chip, &oob_orig, BBM_SIZE, &pos);
 	aux_write(chip, &buf, pkt_size - rem, &pos);
 	aux_write(chip, &oob, ecc_size, &pos);
-
-	return 0;
 }
 
 static int tango_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
 			       u8 *buf, int oob_required, int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, buf, chip->oob_poi);
+	raw_read(chip, buf, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
@@ -419,7 +416,8 @@ static int tango_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 			  int page)
 {
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	return raw_read(chip, NULL, chip->oob_poi);
+	raw_read(chip, NULL, chip->oob_poi);
+	return 0;
 }
 
 static int tango_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
Signature · 1 line
2.9.0