Message ID | 71CF8D7F32C5C24C9CD1D0E02D52498A770B1B97@NTXXIAMBX02.xacn.micron.com |
---|---|
State | New, archived |
Headers | show |
On Mon, 2013-10-28 at 04:54 +0000, Qi Wang 王起 (qiwang) wrote: > On Sa, 2013-10-26 at 05:19 +0000, Artem Bityutskiy wrote: > >On Thu, 2013-10-10 at 08:28 +0000, Qi Wang 王起 (qiwang) wrote: > >> But I want to say the potential risk is if low level driver program data to > >> this block, it will get “timeout error”. And the timeout period could be very > >> long(almost several minutes), during this period, any operation on NOR flash > >> cannot be accept. so program data to a erasure interrupted block isn’t a > >> sensible action. in order to avoid program a erasure interrupted block, > >> I suggest UBIFS can read this block before program data. the code changing as below: > > > >Yes, reading first sounds like a good idea. Would you please send a > >patch implementing it? > > From: Qi Wang <qiwang@micron.com> > > nor_erase_prepare() will be called before erase a NOR flash, it will program '0' > into a block to mark this block. But program data into a erasure interrupted block > can cause program timtout(several minutes at most) error, could impact other > operation on NOR flash. So UBIFS can read this block first to avoid unneeded > program operation. > > This patch try to put read operation at at head of write operation in > nor_erase_prepare(), read out the data. > If the data is already corrupt, then no need to program any data into this block, > just go to erase this block. > > Signed-off-by: Qi Wang <qiwang@qiwang@micron.com> > --- > diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c > index bf79def..be6ab56 100644 > --- a/drivers/mtd/ubi/io.c > +++ b/drivers/mtd/ubi/io.c > @@ -509,26 +509,10 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) > struct ubi_vid_hdr vid_hdr; > > /* > - * It is important to first invalidate the EC header, and then the VID > - * header. Otherwise a power cut may lead to valid EC header and > - * invalid VID header, in which case UBI will treat this PEB as > - * corrupted and will try to preserve it, and print scary warnings. > - */ > - addr = (loff_t)pnum * ubi->peb_size; > - err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); > - if (!err) { > - addr += ubi->vid_hdr_aloffset; > - err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); > - if (!err) > - return 0; > - } How about structuring the code this way: if (EC header is good) invalidate EC header() if (VID header is good) invalidate VID header() Then you'll handle the case when only one of the headers is already corrupted.
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index bf79def..be6ab56 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -509,26 +509,10 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) struct ubi_vid_hdr vid_hdr; /* - * It is important to first invalidate the EC header, and then the VID - * header. Otherwise a power cut may lead to valid EC header and - * invalid VID header, in which case UBI will treat this PEB as - * corrupted and will try to preserve it, and print scary warnings. - */ - addr = (loff_t)pnum * ubi->peb_size; - err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); - if (!err) { - addr += ubi->vid_hdr_aloffset; - err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); - if (!err) - return 0; - } - - /* - * We failed to write to the media. This was observed with Spansion - * S29GL512N NOR flash. Most probably the previously eraseblock erasure + * Most probably the previously eraseblock erasure * was interrupted at a very inappropriate moment, so it became - * unwritable. In this case we probably anyway have garbage in this - * PEB. + * unwritable. In order to avoid program data into a unwritable block, + * read this block first, and check if do need to program it. */ err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0); if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR || @@ -547,6 +531,22 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) } /* + * VID or EC is valid, need to corrupt these header before erase operation. + * It is important to first invalidate the EC header, and then the VID + * header. Otherwise a power cut may lead to valid EC header and + * invalid VID header, in which case UBI will treat this PEB as + * corrupted and will try to preserve it, and print scary warnings. + */ + addr = (loff_t)pnum * ubi->peb_size; + err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); + if (!err) { + addr += ubi->vid_hdr_aloffset; + err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data); + if (!err) + return 0; + } + + /* * The PEB contains a valid VID header, but we cannot invalidate it. * Supposedly the flash media or the driver is screwed up, so return an * error.