Patchwork [U-Boot] cmd_nand: add biterror insertion command for NAND Flash

login
register
mail settings
Submitter Holger Brunck
Date Jan. 4, 2012, 3:32 p.m.
Message ID <1325691123-19565-1-git-send-email-holger.brunck@keymile.com>
Download mbox | patch
Permalink /patch/134284/
State Under Review
Delegated to: Scott Wood
Headers show

Comments

Holger Brunck - Jan. 4, 2012, 3:32 p.m.
From: Stefan Bigler <stefan.bigler@keymile.com>

Initial implementation for unsafe feature for biterror insertion on
NAND-Flash devices. The code flips single bits in the data block of the
flash to simulate single bit-errors.
Tested with Samsung K9F1G08U0D and Micron MT29F1G08ABADAWP on
km_kirkwood boards.

Signed-off-by: Stefan Bigler <stefan.bigler@keymile.com>
Cc: Holger Brunck <holger.brunck@keymile.com>
Cc: Valentin Longchamp <valentin.longchamp@keymile.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 common/cmd_nand.c |  131 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 128 insertions(+), 3 deletions(-)
Scott Wood - Jan. 4, 2012, 5:44 p.m.
On 01/04/2012 09:32 AM, Holger Brunck wrote:
> From: Stefan Bigler <stefan.bigler@keymile.com>
> 
> Initial implementation for unsafe feature for biterror insertion on
> NAND-Flash devices. The code flips single bits in the data block of the
> flash to simulate single bit-errors.
> Tested with Samsung K9F1G08U0D and Micron MT29F1G08ABADAWP on
> km_kirkwood boards.

Do we still need dedicated code for this now that we have the ability to
do raw writes?

-Scott
Holger Brunck - Jan. 5, 2012, 8:59 a.m.
Hi Scott,

On 01/04/2012 06:44 PM, Scott Wood wrote:
> On 01/04/2012 09:32 AM, Holger Brunck wrote:
>>
>> Initial implementation for unsafe feature for biterror insertion on
>> NAND-Flash devices. The code flips single bits in the data block of the
>> flash to simulate single bit-errors.
>> Tested with Samsung K9F1G08U0D and Micron MT29F1G08ABADAWP on
>> km_kirkwood boards.
> 
> Do we still need dedicated code for this now that we have the ability to
> do raw writes?
> 

to be able to do raw writes was very usefull for me to reproduce a bug in the
UBI layer in the past. Bitflips happen very rarely but they happen, and with
this code you are able to force this and see how other SW layer handle this. Due
to the fact there was an empty implementation in the u-boot code for this I
thought it might be usefull for others too.

Best regards
Holger
Scott Wood - Jan. 5, 2012, 7:11 p.m.
On 01/05/2012 02:59 AM, Holger Brunck wrote:
> Hi Scott,
> 
> On 01/04/2012 06:44 PM, Scott Wood wrote:
>> On 01/04/2012 09:32 AM, Holger Brunck wrote:
>>>
>>> Initial implementation for unsafe feature for biterror insertion on
>>> NAND-Flash devices. The code flips single bits in the data block of the
>>> flash to simulate single bit-errors.
>>> Tested with Samsung K9F1G08U0D and Micron MT29F1G08ABADAWP on
>>> km_kirkwood boards.
>>
>> Do we still need dedicated code for this now that we have the ability to
>> do raw writes?
>>
> 
> to be able to do raw writes was very usefull for me to reproduce a bug in the
> UBI layer in the past. Bitflips happen very rarely but they happen, and with
> this code you are able to force this and see how other SW layer handle this. Due
> to the fact there was an empty implementation in the u-boot code for this I
> thought it might be usefull for others too.

The biterr stub has been around since before we had raw writes.  If we
decide we don't need a separate biterr command, we should remove the stub.

Anything the biterr command does should be possible with raw writes, so
while having a dedicated command might be slightly more convenient, I
don't think it's worth the extra code size, review/fix iterations, and
maintenance.

Does anyone else have a strong opinion on the matter?

-Scott

Patch

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 3e2edb8..3873d6f 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -96,6 +96,125 @@  static int nand_dump(nand_info_t *nand, ulong off, int only_oob, int repeat)
 	return 0;
 }
 
+#define MAX_WRITE_BLOCKS 64
+
+static int nand_biterror(nand_info_t *nand, ulong off, int bit)
+{
+	int ret = 0;
+	int block = 0;
+	ulong  block_off;
+	u_char * datbuf[MAX_WRITE_BLOCKS]; /* contains data and obb */
+	u_char * oobbuf[MAX_WRITE_BLOCKS]; /* not used */
+	u_char data;
+	int nbr_write_blocks = nand->erasesize / nand->writesize;
+	struct erase_info einfo;
+
+	if (nbr_write_blocks > MAX_WRITE_BLOCKS) {
+		puts("to many write blocks in one erase block\n");
+		return 1;
+	}
+
+	if ((bit < 0) || (bit > 7)) {
+		puts("bit position 0 to 7 is allowed\n");
+		return 1;
+	}
+
+	/* allocate memory */
+	memset(datbuf, 0, sizeof(datbuf));
+	memset(oobbuf, 0, sizeof(oobbuf));
+
+	for (block = 0; block < nbr_write_blocks; block++) {
+		datbuf[block] = malloc(nand->writesize + nand->oobsize);
+		oobbuf[block] = malloc(nand->oobsize);
+		if (!datbuf[block] || !oobbuf[block]) {
+			puts("No memory for page buffer\n");
+			ret = 1;
+			goto free_memory;
+		}
+	}
+
+	/* read out memory as first step */
+	block_off = off & (~(nand->erasesize - 1));
+	/* allign to eraseable block boundary */
+	for (block = 0; block < nbr_write_blocks; block++) {
+		struct mtd_oob_ops ops;
+
+		loff_t addr = (loff_t) block_off;
+		memset(&ops, 0, sizeof(ops));
+		ops.datbuf = datbuf[block];
+		ops.oobbuf = oobbuf[block];
+		/* must exist, but oob data will be appended to ops.datbuf */
+		ops.len = nand->writesize;
+		ops.ooblen = nand->oobsize;
+		ops.mode = MTD_OOB_RAW;
+		ret = nand->read_oob(nand, addr, &ops);
+		if (ret < 0) {
+			printf("Error (%d) reading page %08lx\n",
+				ret, block_off);
+			ret = 1;
+			goto free_memory;
+		}
+		block_off += nand->writesize;
+	}
+
+	/* second step erase the block */
+	memset(&einfo, 0, sizeof(einfo));
+	einfo.mtd = nand;
+	/* allign to eraseable block boundry */
+	einfo.addr = (loff_t) (off & (~(nand->erasesize - 1)));
+	einfo.len = nand->erasesize;
+	ret = nand_erase_nand(nand, &einfo, 1);
+
+	if (ret < 0) {
+		printf("Error (%d) nand_erase_nand page %08llx\n",
+			ret, einfo.addr);
+		ret = 1;
+		goto free_memory;
+	}
+
+	/* third step twist a bit in data part */
+	/* offset in the erasable block */
+	block_off = off & (nand->erasesize - 1);
+	data = datbuf[block_off/nand->writesize][block_off%nand->writesize];
+	data ^= (1 << bit);
+	datbuf[block_off/nand->writesize][block_off%nand->writesize] = data;
+
+	printf("Flip data at 0x%lx with xor 0x%02x (bit=%d) to value=0x%02x\n",
+		off, (1 << bit), bit, data);
+
+	/* write back twisted data and unmodified obb */
+	/* allign to eraseable block boundry */
+	block_off = off & (~(nand->erasesize - 1));
+	for (block = 0; block < nbr_write_blocks; block++) {
+		struct mtd_oob_ops ops;
+
+		loff_t addr = (loff_t) block_off;
+		memset(&ops, 0, sizeof(ops));
+		ops.datbuf = datbuf[block];
+		ops.oobbuf = &datbuf[block][nand->writesize];
+		ops.len = nand->writesize;
+		ops.ooblen = nand->oobsize;
+		ops.mode = MTD_OOB_RAW;
+		ret = nand->write_oob(nand, addr, &ops);
+
+		if (ret < 0) {
+			printf("Error (%d) write page %08lx\n", ret, block_off);
+			ret = 1;
+			goto free_memory;
+		}
+		block_off += nand->writesize;
+	}
+
+free_memory:
+	for (block = 0; block < nbr_write_blocks; block++) {
+		if (datbuf[block])
+			free(datbuf[block]);
+		if (oobbuf[block])
+			free(oobbuf[block]);
+	}
+	return ret;
+}
+
 /* ------------------------------------------------------------------------- */
 
 static int set_dev(int dev)
@@ -676,8 +795,14 @@  int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 	}
 
 	if (strcmp(cmd, "biterr") == 0) {
-		/* todo */
-		return 1;
+		int bit;
+		if (argc == 4) {
+			off = (int)simple_strtoul(argv[2], NULL, 16);
+			bit = (int)simple_strtoul(argv[3], NULL, 10);
+			ret = nand_biterror(nand, off, bit);
+			return ret;
+		} else
+			goto usage;
 	}
 
 #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK
@@ -756,7 +881,7 @@  U_BOOT_CMD(
 	"nand scrub [-y] off size | scrub.part partition | scrub.chip\n"
 	"    really clean NAND erasing bad blocks (UNSAFE)\n"
 	"nand markbad off [...] - mark bad block(s) at offset (UNSAFE)\n"
-	"nand biterr off - make a bit error at offset (UNSAFE)"
+	"nand biterr off bit - make a bit error at offset and bit position (UNSAFE)"
 #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK
 	"\n"
 	"nand lock [tight] [status]\n"