[Yaffs] Fixes for working with MTD

[Artis Kugevics]

As Yaffs2 does not specify its own nand_oobinfo structure, MTD uses default 
one. That structure has field oobfree, which specifies, at which offset to 
put oob info from yaffs. For example, following structure

static struct nand_oobinfo nand_oob_64 = {
	.useecc = MTD_NANDECC_AUTOPLACE,
	.eccbytes = 24,
	.eccpos = {
		40, 41, 42, 43, 44, 45, 46, 47, 
		48, 49, 50, 51, 52, 53, 54, 55, 
		56, 57, 58, 59, 60, 61, 62, 63},
	.oobfree = { {2, 38} }
};

which is by default in MTD, leaves first 2 bytes unused (for marking block as 
bad) and places oob info from yaffs starting at offset 2. mtd->write_ecc() 
and mtd->read_ecc() does that correctly.
But mtd->read_oob() and mtd->write_oob() reads and writes whole oob space, 
without use of nand_oobinfo structure. It works as expected in yaffs_mtdif.c, 
where such behaviour is needed. But it fails in yaffs_mtdif2.c, where 
mtd->read_oob() expects to read only yaffs oob info, and not whole oob area 
of nand. So, this oobfree offset has to be applied somewhere, to read yaffs 
oob info from correct offset. Fixing this in yaffs may not seem the right 
solution, but it is required, to don't break yaffs1 support at the same time. 
Here is my patch:

=== Cut ===
--- orig/yaffs_mtdif2.c	2005-07-27 12:41:59.000000000 +0300
+++ yaffs_mtdif2.c	2005-07-26 12:04:37.000000000 +0300
@@ -100,20 +100,29 @@
 		{
 			retval = 
mtd->read_ecc(mtd,addr,dev->nBytesPerChunk,&dummy,data,dev->spareBuffer,NULL);
 		}
+		memcpy(&pt,dev->spareBuffer,sizeof(pt));
 	}
 	else
 	{
 #endif
 	if(data)
 		retval = mtd->read(mtd,addr,dev->nBytesPerChunk,&dummy,data);
-	if(tags)
-		retval = mtd->read_oob(mtd,addr,mtd->oobsize,&dummy,dev->spareBuffer);
+	if(tags) {
+		int i, j;
+		retval = mtd->read_oob(mtd,addr,mtd->oobsize,&dummy,
+				       dev->spareBuffer);
+		for (i = 0, j = 0; mtd->oobinfo.oobfree[i][1] ; i++) {
+			int from = mtd->oobinfo.oobfree[i][0];
+			int num = mtd->oobinfo.oobfree[i][1];
+			memcpy(&((char *) &pt)[j],
+			       &((char *) dev->spareBuffer)[from], num);
+			j+= num;
+		}
+	}
 #ifndef	CONFIG_YAFFS_USE_OLD_MTD
 	}
 #endif
 
-    memcpy(&pt,dev->spareBuffer,sizeof(pt));
-    
     if(tags)
 	yaffs_UnpackTags2(tags,&pt);
     
=== Cut ===

Another problem - yaffs_mtdif.c does not use return value from 
mtd->write_ecc():

=== Cut ===
--- orig/yaffs_mtdif.c	2005-07-27 12:41:59.000000000 +0300
+++ yaffs_mtdif.c	2005-07-26 17:26:04.000000000 +0300
@@ -53,9 +54,9 @@
 	if(data && spare)
 	{
 		if(dev->useNANDECC)
-			
mtd->write_ecc(mtd,addr,dev->nBytesPerChunk,&dummy,data,spareAsBytes,&yaffs_oobinfo);
+			retval = 
mtd->write_ecc(mtd,addr,dev->nBytesPerChunk,&dummy,data,spareAsBytes,&yaffs_oobinfo);
 		else
-			
mtd->write_ecc(mtd,addr,dev->nBytesPerChunk,&dummy,data,spareAsBytes,&yaffs_noeccinfo);
+			retval = 
mtd->write_ecc(mtd,addr,dev->nBytesPerChunk,&dummy,data,spareAsBytes,&yaffs_noeccinfo);
 	}
 	else
 	{
=== Cut ===

And maybe it is better to initialize unused oob tag bits to something, before 
writing them to nand? Those ar 2 bits in yaffs1 fs:

=== Cut ===
--- orig/yaffs_tagscompat.c	2005-07-27 12:41:59.000000000 +0300
+++ yaffs_tagscompat.c	2005-07-26 12:39:01.000000000 +0300
@@ -586,6 +586,7 @@
 		tags.chunkId = eTags->chunkId;
 		tags.byteCount = eTags->byteCount;
 		tags.serialNumber = eTags->serialNumber;
+		tags.unusedStuff = 0xffffffff;
 		
 // NCB
 		if (!dev->useNANDECC && data)
=== Cut ===

Artis


On Wednesday 27 July 2005 11:36, Chris Williams wrote:
> Hello,
>
> I wrote earlier trying to figure out why I was losing data with YAFFS2,
> thinking I had the settings wrong. Instead it appears that there are a
> couple bugs in the code that interfaces to MTD (assuming that I
> understand everything correctly of course ;) .)
>
> Anyhow, the two issues I was experiencing were that data would be lost
> because all blocks written to would become bad blocks if I reset the
> system (i.e. updated the bad block table), and after solving that, my
> data would be minorly corrupted. Number one was caused by the code in
> nandmtd2_WriteChunkWithTagsToNAND() and
> nandmtd2_ReadChunkWithTagsToNAND() which write and read the tags struct
> from position 0 in the OOB--thus overwriting the bad block marker. The
> second issue (data corruption) was being caused by these same two
> functions which when writing out the tag, write it out as oobsize, so it
> overwrites the MTD ECC with random data that's past the end of the tag
> in memory.
>
> In nandmtd2_WriteChunkWithTagsToNAND()
> -    if(tags)
> -        retval = mtd->write_oob(mtd,addr, mtd->oobsize,&dummy,(__u8
> *)&pt); +    if(tags)
> +        retval = mtd->write_oob(mtd,addr + 2, sizeof(pt),&dummy,(__u8
> *)&pt);   /* skip two bytes so we don't have to worry about the bad
> block marker */
>
> In nandmtd2_ReadChunkWithTagsToNAND()
> -    if(tags)
> -        retval =
> mtd->read_oob(mtd,addr,mtd->oobsize,&dummy,dev->spareBuffer);
> +    if(tags)
> +        retval = mtd->read_oob(mtd,addr +
> 2,sizeof(pt),&dummy,dev->spareBuffer);   /* tag data begins two bytes
> into the OOB */
>
> Also, I noticed that the tag ecc is, according to the reference
> material, only supposed to be three bytes, however the structure is
> defined as:
>
> typedef struct
> {
>     unsigned char colParity;
>     unsigned lineParity;           //int
>     unsigned lineParityPrime;   //int
> } yaffs_ECCOther;
>
> So it is actually being written out as 9 bytes in the OOB, I believe.
>
> -Chris
>
> _______________________________________________
> yaffs mailing list
> yaffs at stoneboat.aleph1.co.uk
> http://stoneboat.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs