Hello Charles,
On Wed, 20 Sep 2006 10:06:27 +1200
Charles Manning <
manningc2@actrix.gen.nz> 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
(text/plain)