Skip to contentSkip to conversation

mtd: nand: sunxi: cleanup and improvements

PATCH New Boris Brezillon 2 participants 9 messages Sep 30, 2015 – Oct 4, 2015

Subsystems (3)
Memory technology devices (MTD) Arm/Allwinner sunXi SoC support NAND flash
lore
Share & tools
lore

Boris Brezillon

author
#0 lore raw

v2 cover mtd: nand: sunxi: cleanup and improvements

Hello,

This patch series aims at cleaning up the sunxi_nand driver by factorizing the duplicated code found in hw_ecc and hw_syndrome_ecc implementations.

It also adds support for OOB bytes protection (only on a limited amount of OOB bytes), and add code to correctly handle the 'bitflips in erased pages' case.

Best Regards,

Boris

Changes since v1:
- drop the first patch (already applied)
- split the second patch to ease the review
- add the 'fix bitflips in erased pages' patch

Boris Brezillon (7):
mtd: nand: sunxi: create sunxi_nfc_hw_ecc_enable()/disable() functions
mtd: nand: sunxi: introduce sunxi_nfc_hw_ecc_read/write_chunk()
mtd: nand: sunxi: make use of sunxi_nfc_hw_ecc_read/write_chunk()
mtd: nand: sunxi: factorize extra OOB bytes handling
mtd: nand: sunxi: retrieve corrected OOB bytes
mtd: nand: sunxi: replace the NFC_BUF_TO_USER_DATA() macro by an
inline function
mtd: nand: sunxi: fix bitflips in erased pages

 drivers/mtd/nand/sunxi_nand.c | 416 ++++++++++++++++++++++--------------------
 1 file changed, 220 insertions(+), 196 deletions(-)
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH v2 1/7 mtd: nand: sunxi: create sunxi_nfc_hw_ecc_enable()/disable() functions

The code used to enable/disable the hardware ECC engine is repeated in a lot of places. Create two functions to avoid code duplication.

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 70 ++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 38 deletions(-)
drivers/mtd/nand/sunxi_nand.c +32 -38
index dc44435..471fc2b 100644
@@ -543,6 +543,30 @@ static void sunxi_nfc_cmd_ctrl(struct mtd_info *mtd, int dat,
 	sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
 }
 
+static void sunxi_nfc_hw_ecc_enable(struct mtd_info *mtd)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	struct sunxi_nand_hw_ecc *data = nand->ecc.priv;
+	u32 ecc_ctl;
+
+	ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
+	ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
+		     NFC_ECC_BLOCK_SIZE_MSK);
+	ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
+
+	writel(ecc_ctl, nfc->regs + NFC_REG_ECC_CTL);
+}
+
+static void sunxi_nfc_hw_ecc_disable(struct mtd_info *mtd)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+
+	writel(readl(nfc->regs + NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
+	       nfc->regs + NFC_REG_ECC_CTL);
+}
+
 static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 				      struct nand_chip *chip, uint8_t *buf,
 				      int oob_required, int page)

@@ -550,7 +574,6 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	struct nand_ecclayout *layout = ecc->layout;
-	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	unsigned int max_bitflips = 0;
 	int offset;
 	int ret;

@@ -558,11 +581,7 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 	int i;
 	int cnt;
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE | NFC_ECC_BLOCK_SIZE_MSK);
-	tmp |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
 		if (i)

@@ -620,10 +639,7 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 		}
 	}
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~NFC_ECC_EN;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_disable(mtd);
 
 	return max_bitflips;
 }

@@ -635,18 +651,13 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	struct nand_ecclayout *layout = ecc->layout;
-	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	int offset;
 	int ret;
 	u32 tmp;
 	int i;
 	int cnt;
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE | NFC_ECC_BLOCK_SIZE_MSK);
-	tmp |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
 		if (i)

@@ -686,10 +697,7 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 		}
 	}
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~NFC_ECC_EN;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_disable(mtd);
 
 	return 0;
 }

@@ -701,7 +709,6 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	unsigned int max_bitflips = 0;
 	uint8_t *oob = chip->oob_poi;
 	int offset = 0;

@@ -710,11 +717,7 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 	u32 tmp;
 	int i;
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE | NFC_ECC_BLOCK_SIZE_MSK);
-	tmp |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
 		chip->read_buf(mtd, NULL, ecc->size);

@@ -755,8 +758,7 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 		}
 	}
 
-	writel(readl(nfc->regs + NFC_REG_ECC_CTL) & ~NFC_ECC_EN,
-	       nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_disable(mtd);
 
 	return max_bitflips;
 }

@@ -768,7 +770,6 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	uint8_t *oob = chip->oob_poi;
 	int offset = 0;
 	int ret;

@@ -776,11 +777,7 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 	u32 tmp;
 	int i;
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE | NFC_ECC_BLOCK_SIZE_MSK);
-	tmp |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | NFC_ECC_EXCEPTION;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
 		chip->write_buf(mtd, buf + (i * ecc->size), ecc->size);

@@ -810,10 +807,7 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 		}
 	}
 
-	tmp = readl(nfc->regs + NFC_REG_ECC_CTL);
-	tmp &= ~NFC_ECC_EN;
-
-	writel(tmp, nfc->regs + NFC_REG_ECC_CTL);
+	sunxi_nfc_hw_ecc_disable(mtd);
 
 	return 0;
 }
Signature · 1 line
2.1.4
author
#2 lore raw

PATCH v2 2/7 mtd: nand: sunxi: introduce sunxi_nfc_hw_ecc_read/write_chunk()

In reply to Boris Brezillon

The logic behind normal and syndrome ECC handling is pretty much the same, the only difference is the ECC bytes placement.
Create two functions to read/write ECC chunks. Those functions will later be used by the sunxi_nfc_hw_ecc_read/write_page() and sunxi_nfc_hw_syndrome_ecc_read/write_page() functions.

Signed-off-by Boris Brezillon <[email protected]>

---
Brian,

Introducing those two functions without making use of them is the only solution I found to make the patch more readable, but this also means that this commit generates compilation warnings (unused static functions) which are only fixed by the next commit.

Let me know if you see a more elegant solution.

Best Regards,

Boris

---
 drivers/mtd/nand/sunxi_nand.c | 92 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)
drivers/mtd/nand/sunxi_nand.c +92 -0
index 471fc2b..9d8cf06 100644
@@ -567,6 +567,98 @@ static void sunxi_nfc_hw_ecc_disable(struct mtd_info *mtd)
 	       nfc->regs + NFC_REG_ECC_CTL);
 }
 
+static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
+				       u8 *data, int data_off,
+				       u8 *oob, int oob_off,
+				       int *cur_off,
+				       unsigned int *max_bitflips)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	u32 status;
+	int ret;
+
+	if (*cur_off != data_off)
+		nand->cmdfunc(mtd, NAND_CMD_RNDOUT, data_off, -1);
+
+	sunxi_nfc_read_buf(mtd, data, ecc->size);
+
+	if (data_off + ecc->bytes != oob_off)
+		nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
+
+	ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
+	if (ret)
+		return ret;
+
+	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
+	       nfc->regs + NFC_REG_CMD);
+
+	ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+	if (ret)
+		return ret;
+
+	status = readl(nfc->regs + NFC_REG_ECC_ST);
+	ret = NFC_ECC_ERR_CNT(0, readl(nfc->regs + NFC_REG_ECC_ERR_CNT(0)));
+
+	memcpy_fromio(data, nfc->regs + NFC_RAM0_BASE, ecc->size);
+
+	nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
+	sunxi_nfc_read_buf(mtd, oob, ecc->bytes + 4);
+
+	if (status & NFC_ECC_ERR(0))
+		ret = -EIO;
+
+	if (ret < 0) {
+		mtd->ecc_stats.failed++;
+	} else {
+		mtd->ecc_stats.corrected += ret;
+		*max_bitflips = max_t(unsigned int, *max_bitflips, ret);
+	}
+
+	*cur_off = oob_off + ecc->bytes + 4;
+
+	return 0;
+}
+
+static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
+					const u8 *data, int data_off,
+					const u8 *oob, int oob_off,
+					int *cur_off)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	int ret;
+
+	if (data_off != *cur_off)
+		nand->cmdfunc(mtd, NAND_CMD_RNDIN, data_off, -1);
+
+	sunxi_nfc_write_buf(mtd, data, ecc->size);
+
+	/* Fill OOB data in */
+	writel(NFC_BUF_TO_USER_DATA(oob), nfc->regs + NFC_REG_USER_DATA(0));
+
+	if (data_off + ecc->bytes != oob_off)
+		nand->cmdfunc(mtd, NAND_CMD_RNDIN, oob_off, -1);
+
+	ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
+	if (ret)
+		return ret;
+
+	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
+	       NFC_ACCESS_DIR | NFC_ECC_OP,
+	       nfc->regs + NFC_REG_CMD);
+
+	ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+	if (ret)
+		return ret;
+
+	*cur_off = oob_off + ecc->bytes + 4;
+
+	return 0;
+}
+
 static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 				      struct nand_chip *chip, uint8_t *buf,
 				      int oob_required, int page)
Signature · 1 line
2.1.4
author
#3 lore raw

PATCH v2 3/7 mtd: nand: sunxi: make use of sunxi_nfc_hw_ecc_read/write_chunk()

In reply to Boris Brezillon

The sunxi_nfc_hw_ecc_read/write_chunk() functions have been created to factorize the code in the normal and syndrome ECC implementation. Make use of them where appropriate.

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 168 ++++++++++--------------------------------
 1 file changed, 40 insertions(+), 128 deletions(-)
drivers/mtd/nand/sunxi_nand.c +40 -128
index 9d8cf06..176d0f0 100644
@@ -663,61 +663,25 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 				      struct nand_chip *chip, uint8_t *buf,
 				      int oob_required, int page)
 {
-	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct nand_ecclayout *layout = ecc->layout;
 	unsigned int max_bitflips = 0;
+	int ret, i, cur_off = 0;
 	int offset;
-	int ret;
-	u32 tmp;
-	int i;
 	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
-		if (i)
-			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, i * ecc->size, -1);
-
-		offset = mtd->writesize + layout->eccpos[i * ecc->bytes] - 4;
-
-		chip->read_buf(mtd, NULL, ecc->size);
-
-		chip->cmdfunc(mtd, NAND_CMD_RNDOUT, offset, -1);
-
-		ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
+		int data_off = i * ecc->size;
+		int oob_off = i * (ecc->bytes + 4);
+		u8 *data = buf + data_off;
+		u8 *oob = chip->oob_poi + oob_off;
+
+		ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
+						  oob_off + mtd->writesize,
+						  &cur_off, &max_bitflips);
 		if (ret)
 			return ret;
-
-		tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP;
-		writel(tmp, nfc->regs + NFC_REG_CMD);
-
-		ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
-		if (ret)
-			return ret;
-
-		memcpy_fromio(buf + (i * ecc->size),
-			      nfc->regs + NFC_RAM0_BASE, ecc->size);
-
-		if (readl(nfc->regs + NFC_REG_ECC_ST) & NFC_ECC_ERR(0)) {
-			mtd->ecc_stats.failed++;
-		} else {
-			tmp = readl(nfc->regs + NFC_REG_ECC_ERR_CNT(0));
-			mtd->ecc_stats.corrected += NFC_ECC_ERR_CNT(0, tmp);
-			max_bitflips = max_t(unsigned int, max_bitflips, tmp);
-		}
-
-		if (oob_required) {
-			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, offset, -1);
-
-			ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
-			if (ret)
-				return ret;
-
-			offset -= mtd->writesize;
-			chip->read_buf(mtd, chip->oob_poi + offset,
-				      ecc->bytes + 4);
-		}
 	}
 
 	if (oob_required) {

@@ -740,40 +704,22 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 				       struct nand_chip *chip,
 				       const uint8_t *buf, int oob_required)
 {
-	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct nand_ecclayout *layout = ecc->layout;
+	int ret, i, cur_off = 0;
 	int offset;
-	int ret;
-	u32 tmp;
-	int i;
 	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
-		if (i)
-			chip->cmdfunc(mtd, NAND_CMD_RNDIN, i * ecc->size, -1);
-
-		chip->write_buf(mtd, buf + (i * ecc->size), ecc->size);
-
-		offset = layout->eccpos[i * ecc->bytes] - 4 + mtd->writesize;
-
-		/* Fill OOB data in */
-		writel(NFC_BUF_TO_USER_DATA(chip->oob_poi +
-					    layout->oobfree[i].offset),
-		       nfc->regs + NFC_REG_USER_DATA(0));
-
-		chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
-
-		ret = sunxi_nfc_wait_cmd_fifo_empty(nfc);
-		if (ret)
-			return ret;
-
-		tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ACCESS_DIR |
-		      NFC_ECC_OP;
-		writel(tmp, nfc->regs + NFC_REG_CMD);
-		ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+		int data_off = i * ecc->size;
+		int oob_off = i * (ecc->bytes + 4);
+		const u8 *data = buf + data_off;
+		const u8 *oob = chip->oob_poi + oob_off;
+
+		ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off, oob,
+						   oob_off + mtd->writesize,
+						   &cur_off);
 		if (ret)
 			return ret;
 	}

@@ -799,53 +745,31 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 					       uint8_t *buf, int oob_required,
 					       int page)
 {
-	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	unsigned int max_bitflips = 0;
-	uint8_t *oob = chip->oob_poi;
-	int offset = 0;
-	int ret;
+	int ret, i, cur_off = 0;
 	int cnt;
-	u32 tmp;
-	int i;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
-		chip->read_buf(mtd, NULL, ecc->size);
-
-		tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP;
-		writel(tmp, nfc->regs + NFC_REG_CMD);
-
-		ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+		int data_off = i * (ecc->size + ecc->bytes + 4);
+		int oob_off = data_off + ecc->size;
+		u8 *data = buf + (i * ecc->size);
+		u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
+
+		ret = sunxi_nfc_hw_ecc_read_chunk(mtd, data, data_off, oob,
+						  oob_off, &cur_off,
+						  &max_bitflips);
 		if (ret)
 			return ret;
-
-		memcpy_fromio(buf, nfc->regs + NFC_RAM0_BASE, ecc->size);
-		buf += ecc->size;
-		offset += ecc->size;
-
-		if (readl(nfc->regs + NFC_REG_ECC_ST) & NFC_ECC_ERR(0)) {
-			mtd->ecc_stats.failed++;
-		} else {
-			tmp = readl(nfc->regs + NFC_REG_ECC_ERR_CNT(0));
-			mtd->ecc_stats.corrected += NFC_ECC_ERR_CNT(0, tmp);
-			max_bitflips = max_t(unsigned int, max_bitflips, tmp);
-		}
-
-		if (oob_required) {
-			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, offset, -1);
-			chip->read_buf(mtd, oob, ecc->bytes + ecc->prepad);
-			oob += ecc->bytes + ecc->prepad;
-		}
-
-		offset += ecc->bytes + ecc->prepad;
 	}
 
 	if (oob_required) {
-		cnt = mtd->oobsize - (oob - chip->oob_poi);
+		cnt = mtd->writesize + mtd->oobsize - cur_off;
 		if (cnt > 0) {
-			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, offset, -1);
+			u8 *oob = chip->oob_poi + mtd->oobsize - cnt;
+
 			chip->read_buf(mtd, oob, cnt);
 		}
 	}

@@ -860,41 +784,29 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 						const uint8_t *buf,
 						int oob_required)
 {
-	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	uint8_t *oob = chip->oob_poi;
-	int offset = 0;
-	int ret;
+	int ret, i, cur_off = 0;
 	int cnt;
-	u32 tmp;
-	int i;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 
 	for (i = 0; i < ecc->steps; i++) {
-		chip->write_buf(mtd, buf + (i * ecc->size), ecc->size);
-		offset += ecc->size;
-
-		/* Fill OOB data in */
-		writel(NFC_BUF_TO_USER_DATA(oob),
-		       nfc->regs + NFC_REG_USER_DATA(0));
-
-		tmp = NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ACCESS_DIR |
-		      NFC_ECC_OP;
-		writel(tmp, nfc->regs + NFC_REG_CMD);
+		int data_off = i * (ecc->size + ecc->bytes + 4);
+		int oob_off = data_off + ecc->size;
+		const u8 *data = buf + (i * ecc->size);
+		const u8 *oob = chip->oob_poi + (i * (ecc->bytes + 4));
 
-		ret = sunxi_nfc_wait_int(nfc, NFC_CMD_INT_FLAG, 0);
+		ret = sunxi_nfc_hw_ecc_write_chunk(mtd, data, data_off,
+						   oob, oob_off, &cur_off);
 		if (ret)
 			return ret;
-
-		offset += ecc->bytes + ecc->prepad;
-		oob += ecc->bytes + ecc->prepad;
 	}
 
 	if (oob_required) {
-		cnt = mtd->oobsize - (oob - chip->oob_poi);
+		cnt = mtd->writesize + mtd->oobsize - cur_off;
 		if (cnt > 0) {
-			chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
+			u8 *oob = chip->oob_poi + mtd->oobsize - cnt;
+
 			chip->write_buf(mtd, oob, cnt);
 		}
 	}
Signature · 1 line
2.1.4
author
#4 lore raw

PATCH v2 4/7 mtd: nand: sunxi: factorize extra OOB bytes handling

In reply to Boris Brezillon

Add helper functions to factorize the code dealing extra OOB bytes in the normal and syndrome ECC implementations.

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 90 +++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 42 deletions(-)
drivers/mtd/nand/sunxi_nand.c +48 -42
index 176d0f0..45b74fc 100644
@@ -621,6 +621,26 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
 	return 0;
 }
 
+static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
+					    u8 *oob, int *cur_off)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	int offset = ((ecc->bytes + 4) * ecc->steps);
+	int len = mtd->oobsize - offset;
+
+	if (len <= 0)
+		return;
+
+	if (*cur_off != offset)
+		nand->cmdfunc(mtd, NAND_CMD_RNDOUT,
+			      offset + mtd->writesize, -1);
+
+	sunxi_nfc_read_buf(mtd, oob + offset, len);
+
+	*cur_off = mtd->oobsize + mtd->writesize;
+}
+
 static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
 					const u8 *data, int data_off,
 					const u8 *oob, int oob_off,

@@ -659,6 +679,26 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
 	return 0;
 }
 
+static void sunxi_nfc_hw_ecc_write_extra_oob(struct mtd_info *mtd,
+					     u8 *oob, int *cur_off)
+{
+	struct nand_chip *nand = mtd->priv;
+	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	int offset = ((ecc->bytes + 4) * ecc->steps);
+	int len = mtd->oobsize - offset;
+
+	if (len <= 0)
+		return;
+
+	if (*cur_off != offset)
+		nand->cmdfunc(mtd, NAND_CMD_RNDIN,
+			      offset + mtd->writesize, -1);
+
+	sunxi_nfc_write_buf(mtd, oob + offset, len);
+
+	*cur_off = mtd->oobsize + mtd->writesize;
+}
+
 static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 				      struct nand_chip *chip, uint8_t *buf,
 				      int oob_required, int page)

@@ -666,8 +706,6 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	unsigned int max_bitflips = 0;
 	int ret, i, cur_off = 0;
-	int offset;
-	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 

@@ -684,16 +722,8 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 			return ret;
 	}
 
-	if (oob_required) {
-		cnt = ecc->layout->oobfree[ecc->steps].length;
-		if (cnt > 0) {
-			offset = mtd->writesize +
-				 ecc->layout->oobfree[ecc->steps].offset;
-			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, offset, -1);
-			offset -= mtd->writesize;
-			chip->read_buf(mtd, chip->oob_poi + offset, cnt);
-		}
-	}
+	if (oob_required)
+		sunxi_nfc_hw_ecc_read_extra_oob(mtd, chip->oob_poi, &cur_off);
 
 	sunxi_nfc_hw_ecc_disable(mtd);
 

@@ -706,8 +736,6 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 {
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int ret, i, cur_off = 0;
-	int offset;
-	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 

@@ -724,16 +752,8 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 			return ret;
 	}
 
-	if (oob_required) {
-		cnt = ecc->layout->oobfree[i].length;
-		if (cnt > 0) {
-			offset = mtd->writesize +
-				 ecc->layout->oobfree[i].offset;
-			chip->cmdfunc(mtd, NAND_CMD_RNDIN, offset, -1);
-			offset -= mtd->writesize;
-			chip->write_buf(mtd, chip->oob_poi + offset, cnt);
-		}
-	}
+	if (oob_required)
+		sunxi_nfc_hw_ecc_write_extra_oob(mtd, chip->oob_poi, &cur_off);
 
 	sunxi_nfc_hw_ecc_disable(mtd);
 

@@ -748,7 +768,6 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	unsigned int max_bitflips = 0;
 	int ret, i, cur_off = 0;
-	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 

@@ -765,14 +784,8 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 			return ret;
 	}
 
-	if (oob_required) {
-		cnt = mtd->writesize + mtd->oobsize - cur_off;
-		if (cnt > 0) {
-			u8 *oob = chip->oob_poi + mtd->oobsize - cnt;
-
-			chip->read_buf(mtd, oob, cnt);
-		}
-	}
+	if (oob_required)
+		sunxi_nfc_hw_ecc_read_extra_oob(mtd, chip->oob_poi, &cur_off);
 
 	sunxi_nfc_hw_ecc_disable(mtd);
 

@@ -786,7 +799,6 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 {
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	int ret, i, cur_off = 0;
-	int cnt;
 
 	sunxi_nfc_hw_ecc_enable(mtd);
 

@@ -802,14 +814,8 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 			return ret;
 	}
 
-	if (oob_required) {
-		cnt = mtd->writesize + mtd->oobsize - cur_off;
-		if (cnt > 0) {
-			u8 *oob = chip->oob_poi + mtd->oobsize - cnt;
-
-			chip->write_buf(mtd, oob, cnt);
-		}
-	}
+	if (oob_required)
+		sunxi_nfc_hw_ecc_write_extra_oob(mtd, chip->oob_poi, &cur_off);
 
 	sunxi_nfc_hw_ecc_disable(mtd);
Signature · 1 line
2.1.4
author
#5 lore raw

PATCH v2 5/7 mtd: nand: sunxi: retrieve corrected OOB bytes

In reply to Boris Brezillon

The ECC engine is protecting a few OOB bytes. Retrieve them from the USER_DATA register instead of reading them in raw mode (ie without the ECC protection).

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
drivers/mtd/nand/sunxi_nand.c +17 -1
index 45b74fc..a9061a0 100644
@@ -567,6 +567,14 @@ static void sunxi_nfc_hw_ecc_disable(struct mtd_info *mtd)
 	       nfc->regs + NFC_REG_ECC_CTL);
 }
 
+static inline void sunxi_nfc_user_data_to_buf(u32 user_data, u8 *buf)
+{
+	buf[0] = user_data;
+	buf[1] = user_data >> 8;
+	buf[2] = user_data >> 16;
+	buf[3] = user_data >> 24;
+}
+
 static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
 				       u8 *data, int data_off,
 				       u8 *oob, int oob_off,

@@ -606,8 +614,16 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
 	nand->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_off, -1);
 	sunxi_nfc_read_buf(mtd, oob, ecc->bytes + 4);
 
-	if (status & NFC_ECC_ERR(0))
+	if (status & NFC_ECC_ERR(0)) {
 		ret = -EIO;
+	} else {
+		/*
+		 * The engine protects 4 bytes of OOB data per chunk.
+		 * Retrieve the corrected OOB bytes.
+		 */
+		sunxi_nfc_user_data_to_buf(readl(nfc->regs + NFC_REG_USER_DATA(0)),
+					   oob);
+	}
 
 	if (ret < 0) {
 		mtd->ecc_stats.failed++;
Signature · 1 line
2.1.4
author
#6 lore raw

PATCH v2 6/7 mtd: nand: sunxi: replace the NFC_BUF_TO_USER_DATA() macro by an inline function

In reply to Boris Brezillon

sunxi_nfc_user_data_to_buf() is exposed as an inline function, replace the NFC_BUF_TO_USER_DATA() macro by an inline function to be consistent.

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/mtd/nand/sunxi_nand.c +7 -5
index a9061a0..a76eb51 100644
@@ -156,10 +156,6 @@
 #define NFC_ECC_PAT_FOUND(x)	BIT(x + 16)
 #define NFC_ECC_ERR_CNT(b, x)	(((x) >> ((b) * 8)) & 0xff)
 
-/* NFC_USER_DATA helper macros */
-#define NFC_BUF_TO_USER_DATA(buf)	((buf)[0] | ((buf)[1] << 8) | \
-					((buf)[2] << 16) | ((buf)[3] << 24))
-
 #define NFC_DEFAULT_TIMEOUT_MS	1000
 
 #define NFC_SRAM_SIZE		1024

@@ -657,6 +653,11 @@ static void sunxi_nfc_hw_ecc_read_extra_oob(struct mtd_info *mtd,
 	*cur_off = mtd->oobsize + mtd->writesize;
 }
 
+static inline u32 sunxi_nfc_buf_to_user_data(const u8 *buf)
+{
+	return buf[0] | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24);
+}
+
 static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
 					const u8 *data, int data_off,
 					const u8 *oob, int oob_off,

@@ -673,7 +674,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct mtd_info *mtd,
 	sunxi_nfc_write_buf(mtd, data, ecc->size);
 
 	/* Fill OOB data in */
-	writel(NFC_BUF_TO_USER_DATA(oob), nfc->regs + NFC_REG_USER_DATA(0));
+	writel(sunxi_nfc_buf_to_user_data(oob),
+	       nfc->regs + NFC_REG_USER_DATA(0));
 
 	if (data_off + ecc->bytes != oob_off)
 		nand->cmdfunc(mtd, NAND_CMD_RNDIN, oob_off, -1);
Signature · 1 line
2.1.4
author
#7 lore raw

PATCH v2 7/7 mtd: nand: sunxi: fix bitflips in erased pages

In reply to Boris Brezillon

Use the nand_check_erased_ecc_chunk() function to test if the ECC error was triggered by an erased page containing a few bitflips.

Signed-off-by Boris Brezillon <[email protected]>
---
 drivers/mtd/nand/sunxi_nand.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/mtd/nand/sunxi_nand.c +3 -1
index a76eb51..92245a3 100644
@@ -611,7 +611,9 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct mtd_info *mtd,
 	sunxi_nfc_read_buf(mtd, oob, ecc->bytes + 4);
 
 	if (status & NFC_ECC_ERR(0)) {
-		ret = -EIO;
+		ret = nand_check_erased_ecc_chunk(data,	ecc->size,
+						  oob, ecc->bytes + 4,
+						  NULL, 0, ecc->strength);
 	} else {
 		/*
 		 * The engine protects 4 bytes of OOB data per chunk.
Signature · 1 line
2.1.4
#8 lore raw

v2 cover mtd: nand: sunxi: cleanup and improvements

In reply to Boris Brezillon
Quoted 18 lines · Boris Brezillon

Hello,

This patch series aims at cleaning up the sunxi_nand driver by factorizing the duplicated code found in hw_ecc and hw_syndrome_ecc implementations.

It also adds support for OOB bytes protection (only on a limited amount of OOB bytes), and add code to correctly handle the 'bitflips in erased pages' case.

Best Regards,

Boris

Changes since v1:
- drop the first patch (already applied)
- split the second patch to ease the review
- add the 'fix bitflips in erased pages' patch

Quoted 9 lines

Boris Brezillon (7):
mtd: nand: sunxi: create sunxi_nfc_hw_ecc_enable()/disable() functions
mtd: nand: sunxi: introduce sunxi_nfc_hw_ecc_read/write_chunk()
mtd: nand: sunxi: make use of sunxi_nfc_hw_ecc_read/write_chunk()
mtd: nand: sunxi: factorize extra OOB bytes handling
mtd: nand: sunxi: retrieve corrected OOB bytes
mtd: nand: sunxi: replace the NFC_BUF_TO_USER_DATA() macro by an
inline function
mtd: nand: sunxi: fix bitflips in erased pages

All look good to me. Pushed to l2-mtd.git. Thanks.