[PATCH] [MTD] [OneNAND] Do not stop reading for ECC errors

Jörn Engel joern at logfs.org
Mon Nov 5 11:23:21 EST 2007


On Mon, 5 November 2007 16:46:13 +0200, Adrian Hunter wrote:
> 
> -EBADMSG is an ECC error.
> 
> ret must be non-zero to indicate that the bufferram is invalid.  Otherwise
> if we read from the same location again, the data would be taken from 
> bufferram
> and no ECC error would be reported.  However we need to keep reading (see 
> nand_base.c)
> so ret is set to zero as though no error occured.  Finally 
> mtd->ecc_stats.failed
> is examined to determine ECC errors. i.e.

Hmm.

> static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
> 	size_t *retlen, u_char *buf)
> {
> 	...
> 	stats = mtd->ecc_stats;
> 	...
> 	while (!ret) {
> 		...
> 		ret = this->wait(mtd, FL_READING);
> 		onenand_update_bufferram(mtd, from, !ret);
> 		if (ret == -EBADMSG)
> 			ret = 0;
> 	}
> 	*retlen = read;
> 	if (mtd->ecc_stats.failed - stats.failed)
> 		return -EBADMSG;
> 	if (ret)
> 		return ret;

Those two conditionals should have reversed order, imo.

> >What about -EUCLEAN?
> 
> The wait function doesn't return -EUCLEAN.  It increments 
> mtd->ecc_stats.corrected and returns
> zero for correctable ECC errors.
> 
> File systems (like say JFFS2 for example) don't expect -EUCLEAN from 
> mtd->read_oob and would treat
> it as a fatal error, so it must not be returned in this case.

Such an interface sure looks strange.  But since read_oob is strange
anyway I don't care very much either way.

Jörn

-- 
There's nothing that will change someone's moral outlook quicker
than cash in large sums.
-- Larry Flynt



More information about the linux-mtd mailing list