[PATCH 0/3] mtd: nand: sunxi: fix SLC NAND support

Hello,

Those 3 patches aims a fixing support for SLC NANDs with rather small
NAND pages (<= 4k).

The first patch promote 512 bytes ECC steps to 1k steps, which should
both save a few OOB bytes and provide stronger ECC.
The second patch checks the ECC step value, and rejects any NAND that
requires ECC steps != 512 | 1024.
The last patch implements ecc->write_subpage(), thus fixing a NULL
pointer dereference when the NAND chip supports subpage write.

Best Regards,

Boris

Boris Brezillon (3):
  mtd: nand: sunxi: prefer 1k ECC blocks when applicable
  mtd: nand: sunxi: check ecc->size values
  mtd: nand: sunxi: fix subpage write

 drivers/mtd/nand/sunxi_nand.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

-- 
2.7.4

Switching to 1k ECC blocks when possible provides better resistance against
concentrated bitflips. Say you have those two configurations:

1/ 16bits/512bytes
2/ 32bits/1024bytes

Both of them require the same amount of ECC bytes (only true for this
specific engine), but the second config allows you to correct the case
where most of your bitflips are concentrated in a single 512bytes portion.

This fact makes the 1k ECC block size more advantageous than the 512bytes
one.

Signed-off-by: Boris Brezillon <redacted>
---
 drivers/mtd/nand/sunxi_nand.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 5299ab0..4f5b4ad 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c

@@ -1784,6 +1784,12 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
 	if (!data)
 		return -ENOMEM;
 
+	/* Prefer 1k ECC chunk over 512 ones */
+	if (ecc->size == 512 && mtd->writesize > 512) {
+		ecc->size = 1024;
+		ecc->strength *= 2;
+	}
+
 	/* Add ECC info retrieval from DT */
 	for (i = 0; i < ARRAY_SIZE(strengths); i++) {
 		if (ecc->strength <= strengths[i])

-- 
2.7.4

Verify that the ecc->size value is either 512 or 1024 bytes.
This should always be the case if this field was assigned to the
nand->ecc_step_size_ds value, but can be wrong when the user overloaded
this value with the nand-ecc-step-size DT property.

Signed-off-by: Boris Brezillon <redacted>
---
 drivers/mtd/nand/sunxi_nand.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 4f5b4ad..252cbbf 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c

@@ -1784,6 +1784,9 @@ static int sunxi_nand_hw_common_ecc_ctrl_init(struct mtd_info *mtd,
 	if (!data)
 		return -ENOMEM;
 
+	if (ecc->size != 512 && ecc->size != 1024)
+		return -EINVAL;
+
 	/* Prefer 1k ECC chunk over 512 ones */
 	if (ecc->size == 512 && mtd->writesize > 512) {
 		ecc->size = 1024;

-- 
2.7.4

Signed-off-by: Boris Brezillon <redacted>
---
 drivers/mtd/nand/sunxi_nand.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 252cbbf..409acbe 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c

@@ -1350,6 +1350,36 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 	return 0;
 }
 
+static int sunxi_nfc_hw_ecc_write_subpage(struct mtd_info *mtd,
+					  struct nand_chip *chip,
+					  u32 data_offs, u32 data_len,
+					  const u8 *buf, int oob_required,
+					  int page)
+{
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	int ret, i, cur_off = 0;
+
+	sunxi_nfc_hw_ecc_enable(mtd);
+
+	for (i = data_offs / ecc->size;
+	     i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
+		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, !i, page);
+		if (ret)
+			return ret;
+	}
+
+	sunxi_nfc_hw_ecc_disable(mtd);
+
+	return 0;
+}
+
 static int sunxi_nfc_hw_ecc_write_page_dma(struct mtd_info *mtd,
 					   struct nand_chip *chip,
 					   const u8 *buf,

@@ -1862,7 +1892,8 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct mtd_info *mtd,
 		ecc->write_page = sunxi_nfc_hw_ecc_write_page;
 	}
 
-	/* TODO: support DMA for raw accesses */
+	/* TODO: support DMA for raw accesses and subpage write */
+	ecc->write_subpage = sunxi_nfc_hw_ecc_write_subpage;
 	ecc->read_oob_raw = nand_read_oob_std;
 	ecc->write_oob_raw = nand_write_oob_std;
 	ecc->read_subpage = sunxi_nfc_hw_ecc_read_subpage;

-- 
2.7.4

On Mon,  6 Jun 2016 13:59:14 +0200
Boris Brezillon [off-list ref] wrote:

Oops, I forgot to add this commit message:

"
Implement ecc->write_subpage() to prevent core code from assigning
this hook to nand_write_subpage_hwecc(). This default implementation
tries to call ecc->hwctl() which in our case is NULL, thus leading
to a NULL pointer dereference.
"

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

On Mon,  6 Jun 2016 13:59:11 +0200
Boris Brezillon [off-list ref] wrote:

Hello,

Those 3 patches aims a fixing support for SLC NANDs with rather small
NAND pages (<= 4k).

The first patch promote 512 bytes ECC steps to 1k steps, which should
both save a few OOB bytes and provide stronger ECC.
The second patch checks the ECC step value, and rejects any NAND that
requires ECC steps != 512 | 1024.
The last patch implements ecc->write_subpage(), thus fixing a NULL
pointer dereference when the NAND chip supports subpage write.

Applied.

Best Regards,

Boris

Boris Brezillon (3):
  mtd: nand: sunxi: prefer 1k ECC blocks when applicable
  mtd: nand: sunxi: check ecc->size values
  mtd: nand: sunxi: fix subpage write

 drivers/mtd/nand/sunxi_nand.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)