Thanx for this effort Christian. I will wait a day or three for any feedback, then CVS it. -- Charles -----Original Message----- From: Christian Gan [mailto:cgan@iders.ca] Sent: Thursday, 6 March 2003 8:01 a.m. To: Nick Bane; manningc2@actrix.gen.nz; yaffs list Subject: RE: yaffs: serious error in yaffs guts Hello folks, Okay made the fixes so that hw ecc should be working for MTD and YAFFS. Here's the route I chose: I stuck with just getting rid of tmpSpare in mtdif.c mainly because the change is the easiest right now in that we don't have to risk changing the interface for other people (WINCE and direct). So, we still have to keep an eye out for those who have nandECC set and call nandmtd_ReadChunkFromNand because the buffer given will require space for the extra 2 ints. I found that CheckChunkErased already violated that. Thomas' patch and some other minor fixes are included in this patch. Okay with that said, Nick (or anyone else with a HW_ECC and new LINUX MTD configuration) can you test this out to see if the patch works and to see if I broke anything? Thanks! Christian > -----Original Message----- > From: Christian Gan [mailto:cgan@iders.ca] > Sent: Thursday, February 27, 2003 5:11 PM > To: Nick Bane; manningc2@actrix.gen.nz; yaffs list > Subject: RE: yaffs: serious error in yaffs guts > > > Hey guys, > > I think if you want to reserve the functionality to use the eccstatus in > readChunkFromNAND then we can go to the other solution of copying (spare + > eccres1 + eccres2) in nandmtd_ReadChunkFromNand and do away with tmpSpare. > But we have to be very careful for all functions calling > nandmtd_ReadChunkFromNand to ensure that it passes a yaffs_Spare > buffer that > is large enough to accomodate the extra ints. Somebody in the future may > forget this fact since usually yaffs_Spare is defined as > YAFFS_BYTES_PER_SPARE bytes. > > > -----Original Message----- > > From: Nick Bane [mailto:nick@cecomputing.co.uk] > > Sent: Thursday, February 27, 2003 4:42 PM > > To: manningc2@actrix.gen.nz; Christian Gan; yaffs list > > Subject: Re: yaffs: serious error in yaffs guts > > > > > > Charles/Christian > > > > One problem with Christian's proposal is that it fails to distinguish > > between a totally failed read and an error corrected read. If the return > > values from readChunkFromNAND can differentiate the two states > that may be > > helpful in the future. > > > > Nick Bane > > > > ----- Original Message ----- > > From: "Charles Manning" > > To: "Christian Gan" ; "Nick Bane" > ; > > "yaffs list" > > Sent: Thursday, February 27, 2003 9:01 PM > > Subject: Re: yaffs: serious error in yaffs guts > > > > > > > Hi Christian > > > > > > It is easiest for me if someone using the Linux MTD with > > NANDECC can test > > > this. Otherewise I will have to write a software emulator. > > > > > > Could you roll this together with Thomas' recent patchs for > > NANDECC, with > > > your proposed ammendments, and I can apply them as a consistent > > patch set. > > > > > > Thanx > > > > > > -- Charles > > > > > > > > > On Fri, 28 Feb 2003 06:27, Christian Gan wrote: > > > > Also, > > > > > > > > In yaffs_guts there is no real need to check eccres1 and eccres2 you > > should > > > > only need to check the return value of > nandmtd_ReadChunkFromNand. The > > two > > > > extra ints from the NAND MTD are only useful when you are performing > > > > multiple page reads when calling nand_read_ecc, since they > > will tell you > > > > which of the pages you requested has failed. But YAFFs only a reads > > single > > > > page at a time, looking at the return value of > nand_read_ecc is enough > > to > > > > know if the write was successful or if an ecc error has occurred for > > that > > > > page. This value is checked in nandmtd_ReadChunkFromNand and > > it passes > > on > > > > a YAFFS_OK or YAFFS_FAIL (for any ecc errors) appropriately. > > > > > > > > Christian > > > > > > > > > -----Original Message----- > > > > > From: Nick Bane [mailto:nick@cecomputing.co.uk] > > > > > Sent: Thursday, February 27, 2003 10:31 AM > > > > > To: yaffs list > > > > > Subject: yaffs: serious error in yaffs guts > > > > > > > > > > > > > > > Charles > > > > > > > > > > A couple of bugs. One serious, one minor. > > > > > > > > > > in yaffs_ReadChunkFromNAND when dev->useNANDECC is true, it is > > > > > assumed that > > > > > readChunkFromNAND fills in the two ecc result integers on the end > > > > > of the oob > > > > > data. > > > > > These are then tested to see if there has been and ecc read > > > > > fail/correction > > > > > by the mtd layer. > > > > > > > > > > The problem with this is that readChunkFromNAND never > > actually passes > > the > > > > > ecc correction data back but leaves it in a local variable > > > > > tmpSpare and only > > > > > copies YAFFS_BYTES_PER_SPARE from it. > > > > > > > > > > The serious consequence of this is that all chunk reads are then > > > > > flagged as > > > > > failures and their blocks are marked as needing retiring > > once the data > > is > > > > > erased so yaffs nand becomes effectively write-once. The really > > serious > > > > > consequence of this is that one cannot now distinguish between > > > > > yaffs_retired_blocks and nand_factory_bad_blocks so one has to > > > > > speculatively > > > > > erase the entire device losing the factory erase data > > (which seems to > > be > > > > > zero on devices I have seen). Shouldn't we use a value > other than 0 > > for > > > > > yaffs to mark the block as bad (with more than two zero bits) so > > > > > they can be > > > > > distinguished? > > > > > > > > > > The other bug is in testing the ecc correction results in > > > > > yaffs_ReadChunkFromNAND. > > > > > if(nspare.eccres2 || nspare.eccres2) > > > > > Clearly one of them should be eccres1 > > > > > > > > > > Nick > > > > > > > > > > ------------------------------------------- > > > > > Nick Bane > > > > > Cambridge, UK. > > > > > +44(0)1954 719270 > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------ > > > > > --------------------- > > > > > This mailing list is hosted by Toby Churchill open software > > > > > > > > (www.toby-churchill.org). > > > > If mailing list membership is no longer wanted you can > remove yourself > > from > > > > the list by > > > > sending an email to yaffs-request@toby-churchill.org with the text > > > > "unsubscribe" > > > > (without the quotes) as the subject. > > > > > > > > > > > > > > > > > -------------------------------------------------------------------------- > > - > > > >------------ This mailing list is hosted by Toby Churchill > > open software > > > > (www.toby-churchill.org). If mailing list membership is no > > longer wanted > > > > you can remove yourself from the list by sending an email to > > > > yaffs-request@toby-churchill.org with the text > "unsubscribe" (without > > the > > > > quotes) as the subject. > > > > > > > > > > > > > > ------------------------------------------------------------------ > > --------------------- > > This mailing list is hosted by Toby Churchill open software > (www.toby-churchill.org). > If mailing list membership is no longer wanted you can remove > yourself from > the list by > sending an email to yaffs-request@toby-churchill.org with the text > "unsubscribe" > (without the quotes) as the subject. > > > ------------------------------------------------------------------ > --------------------- > This mailing list is hosted by Toby Churchill open software (www.toby-churchill.org). If mailing list membership is no longer wanted you can remove yourself from the list by sending an email to yaffs-request@toby-churchill.org with the text "unsubscribe" (without the quotes) as the subject. --------------------------------------------------------------------------------------- This mailing list is hosted by Toby Churchill open software (www.toby-churchill.org). If mailing list membership is no longer wanted you can remove yourself from the list by sending an email to yaffs-request@toby-churchill.org with the text "unsubscribe" (without the quotes) as the subject.