[PATCH -next v4 0/5] mtd: mtd_nandecctest: add more tests

[PATCH -next v4 0/5] mtd: mtd_nandecctest: add more tests

[Akinobu Mita]

This patch series adds several tests cases to mtd_nandecctest module
and tries to cover possible patterns of corruption which can be reliably
corrected or detected.

Changelog
* v4
- fix swapped two ecc arguments for __nand_correct_data()
- remove asm/byteorder include
- add comments
- update commit messages
- remove patches which are already in l2-mtd.git

* v3
- remove unnecessary include
- split into still smaller patches

* v2
- split one patch into smaller patches

Akinobu Mita (5):
  mtd: mtd_nandecctest: support injecting bit error for ecc code
  mtd: mtd_nandecctest: rewrite the test routine
  mtd: mtd_nandecctest: add no corruption test
  mtd: mtd_nandecctest: add single bit error correction test
  mtd: mtd_nandecctest: add double bit error detection tests

 drivers/mtd/tests/mtd_nandecctest.c | 249 ++++++++++++++++++++++++++++++++----
 1 file changed, 226 insertions(+), 23 deletions(-)

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
-- 
1.7.11.4

[PATCH -next v4 1/5] mtd: mtd_nandecctest: support injecting bit error for ecc code

[Akinobu Mita]

Currently inject_single_bit_error() is used to inject single bit error
into randomly selected bit position of the 256 or 512 bytes data block.

Later change will add tests which inject bit errors into the ecc code.
Unfortunately, inject_single_bit_error() doesn't work for the ecc code
which is not a multiple of sizeof(unsigned long).

Because bit fliping at random position is done by __change_bit().
For example, flipping bit position 0 by __change_bit(0, addr) modifies
3rd byte (32bit) or 7th byte (64bit) on big-endian systems.

Using little-endian version of bitops can fix this issue.  But
little-endian version of __change_bit is not yet available.
So this defines __change_bit_le() locally in a similar fashion to
asm-generic/bitops/le.h and use it.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
* v4
- remove include asm/byteorder.h
- add comment for __change_bit_le()

 drivers/mtd/tests/mtd_nandecctest.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index d3e8873..d90daf8 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -9,11 +9,25 @@
 
 #if defined(CONFIG_MTD_NAND) || defined(CONFIG_MTD_NAND_MODULE)
 
+/*
+ * The reason for this __change_bit_le() instead of __change_bit() is to inject
+ * bit error properly within the region which is not a multiple of
+ * sizeof(unsigned long) on big-endian systems
+ */
+#ifdef __LITTLE_ENDIAN
+#define __change_bit_le(nr, addr) __change_bit(nr, addr)
+#elif defined(__BIG_ENDIAN)
+#define __change_bit_le(nr, addr) \
+		__change_bit((nr) ^ ((BITS_PER_LONG - 1) & ~0x7), addr)
+#else
+#error "Unknown byte order"
+#endif
+
 static void inject_single_bit_error(void *data, size_t size)
 {
-	unsigned long offset = random32() % (size * BITS_PER_BYTE);
+	unsigned int offset = random32() % (size * BITS_PER_BYTE);
 
-	__change_bit(offset, data);
+	__change_bit_le(offset, data);
 }
 
 static void dump_data_ecc(void *error_data, void *error_ecc, void *correct_data,
-- 
1.7.11.4

[PATCH -next v4 2/5] mtd: mtd_nandecctest: rewrite the test routine

[Akinobu Mita]

This rewrites the entire test routine in order to make it easy to add more
tests by later changes and minimize duplication of each tests as much as
possible.

Now that each test is described by the members of struct nand_ecc_test:
- name: descriptive testname
- prepare: function to prepare data block and ecc with artifical corruption
- verify: function to verify the result of correcting data block

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
* v4
- fix swapped two ecc arguments for __nand_correct_data()
- add comment for this test

 drivers/mtd/tests/mtd_nandecctest.c | 93 ++++++++++++++++++++++++++++---------
 1 file changed, 70 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index d90daf8..204f796ed 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -7,8 +7,24 @@
 #include <linux/slab.h>
 #include <linux/mtd/nand_ecc.h>
 
+/*
+ * Test the implementation for software ECC
+ *
+ * No actual MTD device is needed, So we don't need to warry about losing
+ * important data by human error.
+ *
+ * This covers possible patterns of corruption which can be reliably corrected
+ * or detected.
+ */
+
 #if defined(CONFIG_MTD_NAND) || defined(CONFIG_MTD_NAND_MODULE)
 
+struct nand_ecc_test {
+	const char *name;
+	void (*prepare)(void *, void *, void *, void *, const size_t);
+	int (*verify)(void *, void *, void *, const size_t);
+};
+
 /*
  * The reason for this __change_bit_le() instead of __change_bit() is to inject
  * bit error properly within the region which is not a multiple of
@@ -23,13 +39,44 @@
 #error "Unknown byte order"
 #endif
 
-static void inject_single_bit_error(void *data, size_t size)
+static void single_bit_error_data(void *error_data, void *correct_data,
+				size_t size)
 {
 	unsigned int offset = random32() % (size * BITS_PER_BYTE);
 
-	__change_bit_le(offset, data);
+	memcpy(error_data, correct_data, size);
+	__change_bit_le(offset, error_data);
+}
+
+static void single_bit_error_in_data(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	single_bit_error_data(error_data, correct_data, size);
+	memcpy(error_ecc, correct_ecc, 3);
+}
+
+static int single_bit_error_correct(void *error_data, void *error_ecc,
+				void *correct_data, const size_t size)
+{
+	unsigned char calc_ecc[3];
+	int ret;
+
+	__nand_calculate_ecc(error_data, size, calc_ecc);
+	ret = __nand_correct_data(error_data, error_ecc, calc_ecc, size);
+	if (ret == 1 && !memcmp(correct_data, error_data, size))
+		return 0;
+
+	return -EINVAL;
 }
 
+static const struct nand_ecc_test nand_ecc_test[] = {
+	{
+		.name = "single-bit-error-in-data-correct",
+		.prepare = single_bit_error_in_data,
+		.verify = single_bit_error_correct,
+	},
+};
+
 static void dump_data_ecc(void *error_data, void *error_ecc, void *correct_data,
 			void *correct_ecc, const size_t size)
 {
@@ -46,14 +93,14 @@ static void dump_data_ecc(void *error_data, void *error_ecc, void *correct_data,
 			DUMP_PREFIX_NONE, 16, 1, correct_ecc, 3, false);
 }
 
-static int nand_ecc_test(const size_t size)
+static int nand_ecc_test_run(const size_t size)
 {
+	int i;
 	int err = 0;
 	void *error_data;
 	void *error_ecc;
 	void *correct_data;
 	void *correct_ecc;
-	char testname[30];
 
 	error_data = kmalloc(size, GFP_KERNEL);
 	error_ecc = kmalloc(3, GFP_KERNEL);
@@ -65,25 +112,25 @@ static int nand_ecc_test(const size_t size)
 		goto error;
 	}
 
-	sprintf(testname, "nand-ecc-%zu", size);
-
 	get_random_bytes(correct_data, size);
-
-	memcpy(error_data, correct_data, size);
-	inject_single_bit_error(error_data, size);
-
 	__nand_calculate_ecc(correct_data, size, correct_ecc);
-	__nand_calculate_ecc(error_data, size, error_ecc);
-	__nand_correct_data(error_data, correct_ecc, error_ecc, size);
-
-	if (memcmp(correct_data, error_data, size)) {
-		pr_err("mtd_nandecctest: not ok - %s\n", testname);
-		dump_data_ecc(error_data, error_ecc, correct_data, correct_ecc,
-				size);
-		err = -EINVAL;
-		goto error;
+
+	for (i = 0; i < ARRAY_SIZE(nand_ecc_test); i++) {
+		nand_ecc_test[i].prepare(error_data, error_ecc,
+				correct_data, correct_ecc, size);
+		err = nand_ecc_test[i].verify(error_data, error_ecc,
+						correct_data, size);
+
+		if (err) {
+			pr_err("mtd_nandecctest: not ok - %s-%zd\n",
+				nand_ecc_test[i].name, size);
+			dump_data_ecc(error_data, error_ecc,
+				correct_data, correct_ecc, size);
+			break;
+		}
+		pr_info("mtd_nandecctest: ok - %s-%zd\n",
+			nand_ecc_test[i].name, size);
 	}
-	pr_info("mtd_nandecctest: ok - %s\n", testname);
 error:
 	kfree(error_data);
 	kfree(error_ecc);
@@ -95,7 +142,7 @@ error:
 
 #else
 
-static int nand_ecc_test(const size_t size)
+static int nand_ecc_test_run(const size_t size)
 {
 	return 0;
 }
@@ -106,11 +153,11 @@ static int __init ecc_test_init(void)
 {
 	int err;
 
-	err = nand_ecc_test(256);
+	err = nand_ecc_test_run(256);
 	if (err)
 		return err;
 
-	return nand_ecc_test(512);
+	return nand_ecc_test_run(512);
 }
 
 static void __exit ecc_test_exit(void)
-- 
1.7.11.4

[PATCH -next v4 3/5] mtd: mtd_nandecctest: add no corruption test

[Akinobu Mita]

This adds no corruptin test case listed below:

Prepare data block and ECC data with no corruption, and verify that
the data block is preserved by __nand_correct_data()

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
* v4
- fix swapped two ecc arguments for __nand_correct_data()

 drivers/mtd/tests/mtd_nandecctest.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index 204f796ed..ff97b10 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -48,6 +48,27 @@ static void single_bit_error_data(void *error_data, void *correct_data,
 	__change_bit_le(offset, error_data);
 }
 
+static void no_bit_error(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	memcpy(error_data, correct_data, size);
+	memcpy(error_ecc, correct_ecc, 3);
+}
+
+static int no_bit_error_verify(void *error_data, void *error_ecc,
+				void *correct_data, const size_t size)
+{
+	unsigned char calc_ecc[3];
+	int ret;
+
+	__nand_calculate_ecc(error_data, size, calc_ecc);
+	ret = __nand_correct_data(error_data, error_ecc, calc_ecc, size);
+	if (ret == 0 && !memcmp(correct_data, error_data, size))
+		return 0;
+
+	return -EINVAL;
+}
+
 static void single_bit_error_in_data(void *error_data, void *error_ecc,
 		void *correct_data, void *correct_ecc, const size_t size)
 {
@@ -71,6 +92,11 @@ static int single_bit_error_correct(void *error_data, void *error_ecc,
 
 static const struct nand_ecc_test nand_ecc_test[] = {
 	{
+		.name = "no-bit-error",
+		.prepare = no_bit_error,
+		.verify = no_bit_error_verify,
+	},
+	{
 		.name = "single-bit-error-in-data-correct",
 		.prepare = single_bit_error_in_data,
 		.verify = single_bit_error_correct,
-- 
1.7.11.4

[PATCH -next v4 4/5] mtd: mtd_nandecctest: add single bit error correction test

[Akinobu Mita]

This adds the single bit error correction test case listed below:

Prepare data block without corruption and ECC data with single bit error,
and verify that the data block is preserved by __nand_correct_data().

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
No changes from previous version

 drivers/mtd/tests/mtd_nandecctest.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index ff97b10..caaeb64 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -48,6 +48,31 @@ static void single_bit_error_data(void *error_data, void *correct_data,
 	__change_bit_le(offset, error_data);
 }
 
+static unsigned int random_ecc_bit(size_t size)
+{
+	unsigned int offset = random32() % (3 * BITS_PER_BYTE);
+
+	if (size == 256) {
+		/*
+		 * Don't inject a bit error into the insignificant bits (16th
+		 * and 17th bit) in ECC code for 256 byte data block
+		 */
+		while (offset == 16 || offset == 17)
+			offset = random32() % (3 * BITS_PER_BYTE);
+	}
+
+	return offset;
+}
+
+static void single_bit_error_ecc(void *error_ecc, void *correct_ecc,
+				size_t size)
+{
+	unsigned int offset = random_ecc_bit(size);
+
+	memcpy(error_ecc, correct_ecc, 3);
+	__change_bit_le(offset, error_ecc);
+}
+
 static void no_bit_error(void *error_data, void *error_ecc,
 		void *correct_data, void *correct_ecc, const size_t size)
 {
@@ -76,6 +101,13 @@ static void single_bit_error_in_data(void *error_data, void *error_ecc,
 	memcpy(error_ecc, correct_ecc, 3);
 }
 
+static void single_bit_error_in_ecc(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	memcpy(error_data, correct_data, size);
+	single_bit_error_ecc(error_ecc, correct_ecc, size);
+}
+
 static int single_bit_error_correct(void *error_data, void *error_ecc,
 				void *correct_data, const size_t size)
 {
@@ -101,6 +133,11 @@ static const struct nand_ecc_test nand_ecc_test[] = {
 		.prepare = single_bit_error_in_data,
 		.verify = single_bit_error_correct,
 	},
+	{
+		.name = "single-bit-error-in-ecc-correct",
+		.prepare = single_bit_error_in_ecc,
+		.verify = single_bit_error_correct,
+	},
 };
 
 static void dump_data_ecc(void *error_data, void *error_ecc, void *correct_data,
-- 
1.7.11.4

[PATCH -next v4 5/5] mtd: mtd_nandecctest: add double bit error detection tests

[Akinobu Mita]

This adds the double bit error detection test cases listed below:

* Prepare data block with double bit error and ECC data without
corruption, and verify that the uncorrectable error is detected by
__nand_correct_data().

* Prepare data block with single bit error and ECC data with single bit
error, and verify that the uncorrectable error is detected.

* Prepare data block without corruption and ECC data with double bit
error, and verify that the uncorrectable error is detected.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-mtd@lists.infradead.org
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
* v4
- fix swapped two ecc arguments for __nand_correct_data()
- remove empty line

 drivers/mtd/tests/mtd_nandecctest.c | 79 +++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/mtd/tests/mtd_nandecctest.c b/drivers/mtd/tests/mtd_nandecctest.c
index caaeb64..b437fa4 100644
--- a/drivers/mtd/tests/mtd_nandecctest.c
+++ b/drivers/mtd/tests/mtd_nandecctest.c
@@ -48,6 +48,22 @@ static void single_bit_error_data(void *error_data, void *correct_data,
 	__change_bit_le(offset, error_data);
 }
 
+static void double_bit_error_data(void *error_data, void *correct_data,
+				size_t size)
+{
+	unsigned int offset[2];
+
+	offset[0] = random32() % (size * BITS_PER_BYTE);
+	do {
+		offset[1] = random32() % (size * BITS_PER_BYTE);
+	} while (offset[0] == offset[1]);
+
+	memcpy(error_data, correct_data, size);
+
+	__change_bit_le(offset[0], error_data);
+	__change_bit_le(offset[1], error_data);
+}
+
 static unsigned int random_ecc_bit(size_t size)
 {
 	unsigned int offset = random32() % (3 * BITS_PER_BYTE);
@@ -73,6 +89,21 @@ static void single_bit_error_ecc(void *error_ecc, void *correct_ecc,
 	__change_bit_le(offset, error_ecc);
 }
 
+static void double_bit_error_ecc(void *error_ecc, void *correct_ecc,
+				size_t size)
+{
+	unsigned int offset[2];
+
+	offset[0] = random_ecc_bit(size);
+	do {
+		offset[1] = random_ecc_bit(size);
+	} while (offset[0] == offset[1]);
+
+	memcpy(error_ecc, correct_ecc, 3);
+	__change_bit_le(offset[0], error_ecc);
+	__change_bit_le(offset[1], error_ecc);
+}
+
 static void no_bit_error(void *error_data, void *error_ecc,
 		void *correct_data, void *correct_ecc, const size_t size)
 {
@@ -122,6 +153,39 @@ static int single_bit_error_correct(void *error_data, void *error_ecc,
 	return -EINVAL;
 }
 
+static void double_bit_error_in_data(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	double_bit_error_data(error_data, correct_data, size);
+	memcpy(error_ecc, correct_ecc, 3);
+}
+
+static void single_bit_error_in_data_and_ecc(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	single_bit_error_data(error_data, correct_data, size);
+	single_bit_error_ecc(error_ecc, correct_ecc, size);
+}
+
+static void double_bit_error_in_ecc(void *error_data, void *error_ecc,
+		void *correct_data, void *correct_ecc, const size_t size)
+{
+	memcpy(error_data, correct_data, size);
+	double_bit_error_ecc(error_ecc, correct_ecc, size);
+}
+
+static int double_bit_error_detect(void *error_data, void *error_ecc,
+				void *correct_data, const size_t size)
+{
+	unsigned char calc_ecc[3];
+	int ret;
+
+	__nand_calculate_ecc(error_data, size, calc_ecc);
+	ret = __nand_correct_data(error_data, error_ecc, calc_ecc, size);
+
+	return (ret == -1) ? 0 : -EINVAL;
+}
+
 static const struct nand_ecc_test nand_ecc_test[] = {
 	{
 		.name = "no-bit-error",
@@ -138,6 +202,21 @@ static const struct nand_ecc_test nand_ecc_test[] = {
 		.prepare = single_bit_error_in_ecc,
 		.verify = single_bit_error_correct,
 	},
+	{
+		.name = "double-bit-error-in-data-detect",
+		.prepare = double_bit_error_in_data,
+		.verify = double_bit_error_detect,
+	},
+	{
+		.name = "single-bit-error-in-data-and-ecc-detect",
+		.prepare = single_bit_error_in_data_and_ecc,
+		.verify = double_bit_error_detect,
+	},
+	{
+		.name = "double-bit-error-in-ecc-detect",
+		.prepare = double_bit_error_in_ecc,
+		.verify = double_bit_error_detect,
+	},
 };
 
 static void dump_data_ecc(void *error_data, void *error_ecc, void *correct_data,
-- 
1.7.11.4

Re: [PATCH -next v4 0/5] mtd: mtd_nandecctest: add more tests

[Artem Bityutskiy]

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

On Sat, 2012-09-08 at 01:48 +0900, Akinobu Mita wrote:
> This patch series adds several tests cases to mtd_nandecctest module
> and tries to cover possible patterns of corruption which can be reliably
> corrected or detected.

Pushed all to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

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