Hello Charles, On Wed, 20 Sep 2006 10:06:27 +1200 Charles Manning wrote: > I know you don't have a 512-byte board, so maybe someone with a 512-byte board > could chase the yaffs1 stuff. Well, the latest patch has been tested on a 'nandsim' NAND simulator w/ 512b page size and default nand_base's OOB layout. > I don't have a 2.6.x board at present (and am a bit short of time) so have > not tried out the changes. I thus can't submit a back-patch. However, I'll > make a few comments that you might be able to consider: > > In yaffs_mtdif2.c: > 1. For yaffs2 (ie what yaffs_mtdif2.c is there for), useNANDECC is always set. > Thus there is no need for the MTD_OOB_RAW case. YAFFS2 assumes that all the > byte placement ugliness is in the driver. I know the original code has > references to useNANDECC, but both paths are the same. So am I getting right that there should remain only one path (useNANDECC)? > 2. The code says that it is OK to write data with no tags, but it is a bug to > write tags with no data. For yaffs2, there should be both tags and data. > Anything else is a bug (and certainly writing anything without tags is). The > only occasion when I expect tags to be written with no data is bad block > marking, but that is also hidden within the driver. It sounds like I should consider data && !tags as a BUG(), right? > 3. In the reading path, yaffs2 can read any combination of data only, tags > only or both tags and data. I assume it is OK for ops.datbuff to be NULL. Ok. > in yaffs_mtdif.c: > 1) The tags packing seems tyo no longer to ECC on the tags area. Is this done > elsewhere (in the driver). If not, some robustness is lost. I admit that the > original ECC was not very strong, but it was something... As I've implemented [ugly] translation routines, YAFFS1 ECC can also be used. The default which I was testing against is ECC performed by MTD/NAND layer. > 2) The code uses single bits for both the page status and block status. This > is a loss of robustness vs the previous (SmartMedia) layout. In the original > layout, the block and page status used a whole byte with bit counting as a > voting mechanism. Moving to a single bit is more likely to invite tags > corruption. As I stated in http://aleph1.co.uk/lurker/message/20060916.105018.3b42635a.en.html, we're facing a *big* problem here. YAFFS1 uses SmartMedia OOB layout which presumes there're 10 spare OOB bytes, which will almost never be true for a common NAND chip. The default MTD/NAND OOB layout is 8 spare OOB bytes so we have to be able to pack 10 bytes into 8. The way I did it was: - leave 6 bits for tagByte[5] - leave 1 bit for pageStatus - leave 1 bit for blockStatus I assume there's a better way to do it, but I don't know it. The _real_ way out would be to have the tags fit in not more than 8 bytes, better in 6. > 3) Page status and block status values seem to have been swapped. Block status > is the one that gets set to 'Y'. Oh thanks, I'll re-check that. > 4) The code always does seperate writes for data and spare. Can these be > combined into a single write to make the writing faster? I think so, will redo. And thanks a lot for the review! Best regards, Vitaly