Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flashafter erasure interrupted

[Artem Bityutskiy]

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@xxxxxxxxxx>
> 
> 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.

-- 
Best Regards,
Artem Bityutskiy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/