On Wednesday 20 September 2006 20:43, Vitaly Wool wrote: > 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)? Yes. > > > 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? Yes > > > 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 The standard oob allowance, from way back in the 256 byte page days has always been 8 oob per 256 of data. So, 512+16 is the standard 512-byte page layout and 2048 + 64 for 2k. The oob layout I used for yaffs1 fits in with the SmartMedia spec (in terms of placement of ECC, block & page state bytes etc). SM uses 1 byte for block state. YAFFS1 used to use 0x00 to mark bad blocks, but I changed this to 'Y' to make it easier to tell factory marked bad blocks from yaffs-marked. This breaks down with some of the more modern non-SM-compatable parts which have internal controllers etc and don't give you full access to the 16 bytes. > > 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. The real way, I think, is to start doing tags-in-data for 512 then it can run yaffs2 on 512-byte pages. Watch this space...... > > > 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. I think this is what is breaking Jonathan. > > > 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! Yea, sorry it should have been sooner :-). > > Best regards, > Vitaly