RE: [PATCH 0/3] An alternative to SPI NAND

[qiwang]

On 01/20/2015 6:36 PM, Ezequiel Garcia wrote:
>
>On 01/12/2015 12:10 PM, Qi Wang 王起 (qiwang) wrote:
>> Hi Ezequiel,
>>
>> On 01/08/2015 11:27 AM, Ezequiel Garcia wrote:
>>>
>>> Hi Qi Wang,
>>>
>>> On 01/07/2015 11:45 PM, Qi Wang 王起 (qiwang) wrote:
>>>> Hi Brian,
>>>>
>>>> On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:
>>>>>
>>>>> On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
>>>>> wrote:
>>>>>> Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
>>>>>> drivers/mtd/Kconfig|2 +
>>>>>> drivers/mtd/Makefile   |1 +
>>>>>> drivers/mtd/spi-nand/Kconfig   |7 +
>>>>>> drivers/mtd/spi-nand/Makefile  |3 +
>>>>>> drivers/mtd/spi-nand/spi-nand-base.c   | 2034
>>>>> 
>>>>>> drivers/mtd/spi-nand/spi-nand-bbt.c| 1279
>
>>>>>
>>>>> I can already tell by the diffstat that I don't like this. We probably
>>>>> don't need 3000 new lines of code for this, but we especially don't
>want
>>>>> to duplicate nand_bbt.c. It won't take a lot of work to augment
>>>>> nand_bbt.c to make it shareable. (I can whip that patch up if needed.)
>>>>
>>>> Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
>>>> SPI NAND. Actually, we are working at this now. Will send patches to
>you
>>>> Once we finished it.
>>>>
>>>
>>> Thanks for the quick submission!
>>>
>>> However, Brian is right, this code duplication is a no go.
>>>
>>> Perhaps a more valid approach would be to first identify the code that
>>> needs to be shared in nand_bbt.c and nand_base.c, and export those
>>> symbols (or maybe do the required refactor).
>>
>> Yes, I agree Brian's suggestion in another mail.
>>
>> " The BBT code is something we definitely want to share, but it's
>actually
>> not very closely tied to nand_base.c, and it looks pretty easy to adapt
>> to any MTD that implements mtd_read_oob()/mtd_write_oob(). We'd just
>> need to parameterize a few relevant device details into a new nand_bbt
>> struct, rather than using struct nand_chip directly."
>>
>> To abstract a new nand_bbt struct instead of nand_chip to make SPI NAND
>> and parallel NAND can share nand_bbt.c file, I already begin to work on
>> this.
>>
>> For code shared in nand_base.c, I agree it would be better if we can find
>> a good method to share nand_base.c code between spi nand and parallel
>nand.
>> But frankly speaking, I'm not satisfied for the remap command method.
>This
>> method make code difficult to maintain when SPI NAND and Parallel NAND
>> evolve much differently in the future.
>>
>> Take some example,
>> If one new command (cache operation, multiple plane operation)
>implemented
>> in parallel NAND code, and is used in nand_read or nand_write, that will
>> cause maintainer to modify SPI NAND code to remap this new command,
>though
>> this modification probably could be slight. That means modification on
>> Parallel NAND flash need to consider SPI NAND as well.
>>
>> How do you think about this?
>>
>> For Peter Pan's patchset, if we do some modification to make nand_bbt.c
>to
>> make it shareable for Parallel and SPI NAND. The code line should be 2000.
>> I believe I can review this spi-nand-base.c to remove some redundant code
>> that may hundreds line. Is 1700 or 1800 code line is more acceptable?
>>
>> Let me know your opinions.
>>
>
>Sounds good.
>
>Do you still plan to maintain the spi-nand-base.c and spi-nand-device.c
>separation?

Yes, still plan to maintain the spi-nand-base.c and spi-nand-device.c
separation. Abstract common code to spi-nand-base.c, and spi-nand-device.c is
used for realize the different function for different SPI NAND, such as ecc
layout, read ID etc.

Thanks
--
Qi Wang

RE: [PATCH 0/3] An alternative to SPI NAND

[qiwang]

On 01/20/2015 6:36 PM, Ezequiel Garcia wrote:

On 01/12/2015 12:10 PM, Qi Wang 王起 (qiwang) wrote:
 Hi Ezequiel,

 On 01/08/2015 11:27 AM, Ezequiel Garcia wrote:

 Hi Qi Wang,

 On 01/07/2015 11:45 PM, Qi Wang 王起 (qiwang) wrote:
 Hi Brian,

 On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:

 On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
 wrote:
 Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
 drivers/mtd/Kconfig|2 +
 drivers/mtd/Makefile   |1 +
 drivers/mtd/spi-nand/Kconfig   |7 +
 drivers/mtd/spi-nand/Makefile  |3 +
 drivers/mtd/spi-nand/spi-nand-base.c   | 2034
 
 drivers/mtd/spi-nand/spi-nand-bbt.c| 1279


 I can already tell by the diffstat that I don't like this. We probably
 don't need 3000 new lines of code for this, but we especially don't
want
 to duplicate nand_bbt.c. It won't take a lot of work to augment
 nand_bbt.c to make it shareable. (I can whip that patch up if needed.)

 Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
 SPI NAND. Actually, we are working at this now. Will send patches to
you
 Once we finished it.


 Thanks for the quick submission!

 However, Brian is right, this code duplication is a no go.

 Perhaps a more valid approach would be to first identify the code that
 needs to be shared in nand_bbt.c and nand_base.c, and export those
 symbols (or maybe do the required refactor).

 Yes, I agree Brian's suggestion in another mail.

  The BBT code is something we definitely want to share, but it's
actually
 not very closely tied to nand_base.c, and it looks pretty easy to adapt
 to any MTD that implements mtd_read_oob()/mtd_write_oob(). We'd just
 need to parameterize a few relevant device details into a new nand_bbt
 struct, rather than using struct nand_chip directly.

 To abstract a new nand_bbt struct instead of nand_chip to make SPI NAND
 and parallel NAND can share nand_bbt.c file, I already begin to work on
 this.

 For code shared in nand_base.c, I agree it would be better if we can find
 a good method to share nand_base.c code between spi nand and parallel
nand.
 But frankly speaking, I'm not satisfied for the remap command method.
This
 method make code difficult to maintain when SPI NAND and Parallel NAND
 evolve much differently in the future.

 Take some example,
 If one new command (cache operation, multiple plane operation)
implemented
 in parallel NAND code, and is used in nand_read or nand_write, that will
 cause maintainer to modify SPI NAND code to remap this new command,
though
 this modification probably could be slight. That means modification on
 Parallel NAND flash need to consider SPI NAND as well.

 How do you think about this?

 For Peter Pan's patchset, if we do some modification to make nand_bbt.c
to
 make it shareable for Parallel and SPI NAND. The code line should be 2000.
 I believe I can review this spi-nand-base.c to remove some redundant code
 that may hundreds line. Is 1700 or 1800 code line is more acceptable?

 Let me know your opinions.


Sounds good.

Do you still plan to maintain the spi-nand-base.c and spi-nand-device.c
separation?

Yes, still plan to maintain the spi-nand-base.c and spi-nand-device.c
separation. Abstract common code to spi-nand-base.c, and spi-nand-device.c is
used for realize the different function for different SPI NAND, such as ecc
layout, read ID etc.

Thanks
--
Qi Wang

[PATCH 0/3] An alternative to SPI NAND

[qiwang]

Hi Ezequiel,

On 01/08/2015 11:27 AM, Ezequiel Garcia wrote:
>
>Hi Qi Wang,
>
>On 01/07/2015 11:45 PM, Qi Wang 王起 (qiwang) wrote:
>> Hi Brian,
>>
>> On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:
>>>
>>> On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
>>> wrote:
>>>> Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
>>>> drivers/mtd/Kconfig|2 +
>>>> drivers/mtd/Makefile   |1 +
>>>> drivers/mtd/spi-nand/Kconfig   |7 +
>>>> drivers/mtd/spi-nand/Makefile  |3 +
>>>> drivers/mtd/spi-nand/spi-nand-base.c   | 2034
>>> 
>>>> drivers/mtd/spi-nand/spi-nand-bbt.c| 1279 
>>>
>>> I can already tell by the diffstat that I don't like this. We probably
>>> don't need 3000 new lines of code for this, but we especially don't want
>>> to duplicate nand_bbt.c. It won't take a lot of work to augment
>>> nand_bbt.c to make it shareable. (I can whip that patch up if needed.)
>>
>> Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
>> SPI NAND. Actually, we are working at this now. Will send patches to you
>> Once we finished it.
>>
>
>Thanks for the quick submission!
>
>However, Brian is right, this code duplication is a no go.
>
>Perhaps a more valid approach would be to first identify the code that
>needs to be shared in nand_bbt.c and nand_base.c, and export those
>symbols (or maybe do the required refactor).

Yes, I agree Brian's suggestion in another mail. 

" The BBT code is something we definitely want to share, but it's actually
not very closely tied to nand_base.c, and it looks pretty easy to adapt
to any MTD that implements mtd_read_oob()/mtd_write_oob(). We'd just
need to parameterize a few relevant device details into a new nand_bbt
struct, rather than using struct nand_chip directly."

To abstract a new nand_bbt struct instead of nand_chip to make SPI NAND 
and parallel NAND can share nand_bbt.c file, I already begin to work on 
this.

For code shared in nand_base.c, I agree it would be better if we can find
a good method to share nand_base.c code between spi nand and parallel nand.
But frankly speaking, I'm not satisfied for the remap command method. This
method make code difficult to maintain when SPI NAND and Parallel NAND 
evolve much differently in the future.

Take some example, 
If one new command (cache operation, multiple plane operation) implemented 
in parallel NAND code, and is used in nand_read or nand_write, that will 
cause maintainer to modify SPI NAND code to remap this new command, though 
this modification probably could be slight. That means modification on 
Parallel NAND flash need to consider SPI NAND as well. 

How do you think about this?

For Peter Pan's patchset, if we do some modification to make nand_bbt.c to
make it shareable for Parallel and SPI NAND. The code line should be 2000.
I believe I can review this spi-nand-base.c to remove some redundant code
that may hundreds line. Is 1700 or 1800 code line is more acceptable?

Let me know your opinions.

>
>Then, separate the SPI NAND upper and lower logic (in a similar to my
>proposal, which I still consider turned out to be clean).
>
>These two things would lead to a simpler and smaller patchset. I also
>suggest to cut off everything that we don't utterly need on a first
>submission, so it's easier to review.
>--
>Ezequiel


--
Qi Wang

[PATCH 0/3] An alternative to SPI NAND

[qiwang]

Hi Ezequiel,

On 01/08/2015 11:27 AM, Ezequiel Garcia wrote:

Hi Qi Wang,

On 01/07/2015 11:45 PM, Qi Wang 王起 (qiwang) wrote:
 Hi Brian,

 On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:

 On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
 wrote:
 Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
 drivers/mtd/Kconfig|2 +
 drivers/mtd/Makefile   |1 +
 drivers/mtd/spi-nand/Kconfig   |7 +
 drivers/mtd/spi-nand/Makefile  |3 +
 drivers/mtd/spi-nand/spi-nand-base.c   | 2034
 
 drivers/mtd/spi-nand/spi-nand-bbt.c| 1279 

 I can already tell by the diffstat that I don't like this. We probably
 don't need 3000 new lines of code for this, but we especially don't want
 to duplicate nand_bbt.c. It won't take a lot of work to augment
 nand_bbt.c to make it shareable. (I can whip that patch up if needed.)

 Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
 SPI NAND. Actually, we are working at this now. Will send patches to you
 Once we finished it.


Thanks for the quick submission!

However, Brian is right, this code duplication is a no go.

Perhaps a more valid approach would be to first identify the code that
needs to be shared in nand_bbt.c and nand_base.c, and export those
symbols (or maybe do the required refactor).

Yes, I agree Brian's suggestion in another mail. 

 The BBT code is something we definitely want to share, but it's actually
not very closely tied to nand_base.c, and it looks pretty easy to adapt
to any MTD that implements mtd_read_oob()/mtd_write_oob(). We'd just
need to parameterize a few relevant device details into a new nand_bbt
struct, rather than using struct nand_chip directly.

To abstract a new nand_bbt struct instead of nand_chip to make SPI NAND 
and parallel NAND can share nand_bbt.c file, I already begin to work on 
this.

For code shared in nand_base.c, I agree it would be better if we can find
a good method to share nand_base.c code between spi nand and parallel nand.
But frankly speaking, I'm not satisfied for the remap command method. This
method make code difficult to maintain when SPI NAND and Parallel NAND 
evolve much differently in the future.

Take some example, 
If one new command (cache operation, multiple plane operation) implemented 
in parallel NAND code, and is used in nand_read or nand_write, that will 
cause maintainer to modify SPI NAND code to remap this new command, though 
this modification probably could be slight. That means modification on 
Parallel NAND flash need to consider SPI NAND as well. 

How do you think about this?

For Peter Pan's patchset, if we do some modification to make nand_bbt.c to
make it shareable for Parallel and SPI NAND. The code line should be 2000.
I believe I can review this spi-nand-base.c to remove some redundant code
that may hundreds line. Is 1700 or 1800 code line is more acceptable?

Let me know your opinions.


Then, separate the SPI NAND upper and lower logic (in a similar to my
proposal, which I still consider turned out to be clean).

These two things would lead to a simpler and smaller patchset. I also
suggest to cut off everything that we don't utterly need on a first
submission, so it's easier to review.
--
Ezequiel


--
Qi Wang

RE: [PATCH 0/3] An alternative to SPI NAND

[qiwang]

Hi Brian,

On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:
>
>On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
>wrote:
>> Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
>> drivers/mtd/Kconfig|2 +
>> drivers/mtd/Makefile   |1 +
>> drivers/mtd/spi-nand/Kconfig   |7 +
>> drivers/mtd/spi-nand/Makefile  |3 +
>> drivers/mtd/spi-nand/spi-nand-base.c   | 2034
>
>> drivers/mtd/spi-nand/spi-nand-bbt.c| 1279 
>
>I can already tell by the diffstat that I don't like this. We probably
>don't need 3000 new lines of code for this, but we especially don't want
>to duplicate nand_bbt.c. It won't take a lot of work to augment
>nand_bbt.c to make it shareable. (I can whip that patch up if needed.)

Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
SPI NAND. Actually, we are working at this now. Will send patches to you
Once we finished it.

But for the 2000 new lines code in spi-nand-base.c, frankly speaking, I 
still no idea how to decrease the code lines. 

I separate code of the SPI NAND and Parallel NAND that different with 
Ezequiel Garcia patches, because I think this framework could be easier 
to maintain it, in the future, SPI NAND may have more specific features 
that will be difficult to reuse the code of Nand_base.c but just remap 
command to SPI NAND from Parallel NAND.

So we write a new spi-nand-base.c, to implement read/write/erase function,
that will cause many code lines as you see. If you have other suggestion, 
please let me know. 

>
>I'll still take a look at the rest of the code eventually, but just
>wanted to give my 2 cents up front.
>
>> drivers/mtd/spi-nand/spi-nand-device.c |  281 +++
>> include/linux/mtd/spi-nand.h   |  317 +++
>> 9 files changed, 3946 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
>> create mode 100644 drivers/mtd/spi-nand/Kconfig
>> create mode 100644 drivers/mtd/spi-nand/Makefile
>> create mode 100644 drivers/mtd/spi-nand/spi-nand-base.c
>> create mode 100644 drivers/mtd/spi-nand/spi-nand-bbt.c
>> create mode 100644 drivers/mtd/spi-nand/spi-nand-device.c
>> create mode 100644 include/linux/mtd/spi-nand.h
>

Qi Wang
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 0/3] An alternative to SPI NAND

[qiwang]

Hi Brian,

On Thu, Jan 08, 2015 at 9:03:24AM +, Brian Norris wrote:

On Thu, Jan 08, 2015 at 12:47:24AM +, Peter Pan 潘栋 (peterpandong)
wrote:
 Documentation/devicetree/bindings/mtd/spi-nand.txt |   22 +
 drivers/mtd/Kconfig|2 +
 drivers/mtd/Makefile   |1 +
 drivers/mtd/spi-nand/Kconfig   |7 +
 drivers/mtd/spi-nand/Makefile  |3 +
 drivers/mtd/spi-nand/spi-nand-base.c   | 2034

 drivers/mtd/spi-nand/spi-nand-bbt.c| 1279 

I can already tell by the diffstat that I don't like this. We probably
don't need 3000 new lines of code for this, but we especially don't want
to duplicate nand_bbt.c. It won't take a lot of work to augment
nand_bbt.c to make it shareable. (I can whip that patch up if needed.)

Yes, I agree with you, Nand_bbt.c do can be shared by Parallel NAND and
SPI NAND. Actually, we are working at this now. Will send patches to you
Once we finished it.

But for the 2000 new lines code in spi-nand-base.c, frankly speaking, I 
still no idea how to decrease the code lines. 

I separate code of the SPI NAND and Parallel NAND that different with 
Ezequiel Garcia patches, because I think this framework could be easier 
to maintain it, in the future, SPI NAND may have more specific features 
that will be difficult to reuse the code of Nand_base.c but just remap 
command to SPI NAND from Parallel NAND.

So we write a new spi-nand-base.c, to implement read/write/erase function,
that will cause many code lines as you see. If you have other suggestion, 
please let me know. 


I'll still take a look at the rest of the code eventually, but just
wanted to give my 2 cents up front.

 drivers/mtd/spi-nand/spi-nand-device.c |  281 +++
 include/linux/mtd/spi-nand.h   |  317 +++
 9 files changed, 3946 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/spi-nand.txt
 create mode 100644 drivers/mtd/spi-nand/Kconfig
 create mode 100644 drivers/mtd/spi-nand/Makefile
 create mode 100644 drivers/mtd/spi-nand/spi-nand-base.c
 create mode 100644 drivers/mtd/spi-nand/spi-nand-bbt.c
 create mode 100644 drivers/mtd/spi-nand/spi-nand-device.c
 create mode 100644 include/linux/mtd/spi-nand.h


Qi Wang
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
Do you have any suggestion for my modification?
Other code I think it is ok.
Thanks


-Original Message-
From: Qi Wang 王起 (qiwang) 
Sent: Monday, January 06, 2014 1:11 PM
To: 'dedeki...@gmail.com'
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

>Please, remember that I pushed your patch to a temporary branch, only
>for you to be able to easily pick it. I do not send it upstream.

>As I said, I need to you look at it and confirm that it is all-right.
>And if it is not all-right, correct it.

>This is because I modified it myself.

>So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+   ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+   pnum, err, err); 

But I think above code print "write err" twice. so I suggest to change to below:

+   ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
Do you have any suggestion for my modification?
Other code I think it is ok.
Thanks


-Original Message-
From: Qi Wang 王起 (qiwang) 
Sent: Monday, January 06, 2014 1:11 PM
To: 'dedeki...@gmail.com'
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

Please, remember that I pushed your patch to a temporary branch, only
for you to be able to easily pick it. I do not send it upstream.

As I said, I need to you look at it and confirm that it is all-right.
And if it is not all-right, correct it.

This is because I modified it myself.

So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+   ubi_err(cannot invalidate PEB %d, write returned %d write returned %d,
+   pnum, err, err); 

But I think above code print write err twice. so I suggest to change to below:

+   ubi_err(cannot invalidate PEB %d, write returned %d, pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Attached is my patch modified, and I use git format-patch to generate this 
file. Please check it. 
Thanks

-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Thursday, January 02, 2014 11:11 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

On Wed, 2014-01-01 at 13:06 +, Qi Wang 王起 (qiwang) wrote:
> Hi Artem:
> As we talked in mail before, please check my patch as below:

Note, because you wrote the above

> From: Qi Wang 

"git am" will put all the above to the commit message. If you want to change 
your "From:", it should be in the first line.

Few other things.

1. If I save your e-mail as "qi.mbox" and then use "git am", I get this:

$ git am ~/tmp/qi.mbox
Applying: MTD: UBI: avoid program operation on NOR flash after erasure 
interrupted
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:18: trailing whitespace.
 * If VID or EC is valid, need to corrupt it before erase operation.  
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:63: space before tab in 
indent.
addr += ubi->vid_hdr_aloffset;
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:66: trailing whitespace.
goto error;
warning: 3 lines add whitespace errors.

Note, I have this in my ~/.gitconfig to notice things like that:

[core]
whitespace = trailing-space,space-before-tab


> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c

...

> -  * The PEB contains a valid VID header, but we cannot invalidate it.
> +  * The PEB contains a valid VID or EC header, but we cannot invalidate 
> it.

82 characters :-)

> - ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
> + ubi_err("cannot invalidate PEB %d, read returned %d write returned 
> +%d",

Why? The error actually happened in mtd_write. No read errors lead to this path.

>   pnum, err, err1);

I guess it is better to kill the err1 variable and use just 'err'
everywhere.

Anyway, I did not respond for long time, so I decided to do all these 
modifications myself. Could you please review and test the below patch, which 
is also attached. I did not compile it even. If there are issues, just fix-it 
up and send the the fix or the fixed version. If it is OK, give me your 
"Tested-by:" please. Thanks!


From: =?UTF-8?q?Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20=28qiwang=29?=
 
Date: Wed, 1 Jan 2014 13:06:11 +
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure  
interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang 
---
 drivers/mtd/ubi/io.c | 55 ++--
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index bf79def..ae9e5ca 
100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)  {
-   int err, err1;
+   int err;
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
+
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header @@ 
-509,50 +511,39 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, we have to corrupt them before erasing.
 * 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_E

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

From: Qi Wang 

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

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..d361349 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-   int err, err1;
+   int err;
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
+
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, we have to corrupt them before erasing.
 * 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   if(err)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err)
+   goto error;
}
+   return 0;
 
+error:
/*
-* 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.
+* The PEB contains a valid VID or EC header, but we cannot invalidate
+* it. Supposedly the flash media or the driver is screwed up, so
+* return an error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-   pnum, err, err1);
+   ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
 }
---

Compare to your patch, the change is only below:
modify from  
+   ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+   pnum, err, err);

to 

+   ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
Thanks

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

>Please, remember that I pushed your patch to a temporary branch, only
>for you to be able to easily pick it. I do not send it upstream.

>As I said, I need to you look at it and confirm that it is all-right.
>And if it is not all-right, correct it.

>This is because I modified it myself.

>So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+   ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+   pnum, err, err); 

But I think above code print "write err" twice. so I suggest to change to below:

+   ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Please, remember that I pushed your patch to a temporary branch, only
for you to be able to easily pick it. I do not send it upstream.

As I said, I need to you look at it and confirm that it is all-right.
And if it is not all-right, correct it.

This is because I modified it myself.

So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+   ubi_err(cannot invalidate PEB %d, write returned %d write returned %d,
+   pnum, err, err); 

But I think above code print write err twice. so I suggest to change to below:

+   ubi_err(cannot invalidate PEB %d, write returned %d, pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

From: Qi Wang qiw...@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 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.

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..d361349 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-   int err, err1;
+   int err;
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
+
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, we have to corrupt them before erasing.
 * 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 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
err = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
-   if (!err)
-   return 0;
+   if(err)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err)
+   goto error;
}
+   return 0;
 
+error:
/*
-* 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.
+* The PEB contains a valid VID or EC header, but we cannot invalidate
+* it. Supposedly the flash media or the driver is screwed up, so
+* return an error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
-   pnum, err, err1);
+   ubi_err(cannot invalidate PEB %d, write returned %d, pnum, err);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
 }
---

Compare to your patch, the change is only below:
modify from  
+   ubi_err(cannot invalidate PEB %d, write returned %d write returned %d,
+   pnum, err, err);

to 

+   ubi_err(cannot invalidate PEB %d, write returned %d, pnum, err);
Thanks

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Attached is my patch modified, and I use git format-patch to generate this 
file. Please check it. 
Thanks

-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Thursday, January 02, 2014 11:11 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

On Wed, 2014-01-01 at 13:06 +, Qi Wang 王起 (qiwang) wrote:
 Hi Artem:
 As we talked in mail before, please check my patch as below:

Note, because you wrote the above

 From: Qi Wang qiw...@micron.com

git am will put all the above to the commit message. If you want to change 
your From:, it should be in the first line.

Few other things.

1. If I save your e-mail as qi.mbox and then use git am, I get this:

$ git am ~/tmp/qi.mbox
Applying: MTD: UBI: avoid program operation on NOR flash after erasure 
interrupted
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:18: trailing whitespace.
 * If VID or EC is valid, need to corrupt it before erase operation.  
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:63: space before tab in 
indent.
addr += ubi-vid_hdr_aloffset;
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:66: trailing whitespace.
goto error;
warning: 3 lines add whitespace errors.

Note, I have this in my ~/.gitconfig to notice things like that:

[core]
whitespace = trailing-space,space-before-tab


 diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c

...

 -  * The PEB contains a valid VID header, but we cannot invalidate it.
 +  * The PEB contains a valid VID or EC header, but we cannot invalidate 
 it.

82 characters :-)

 - ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
 + ubi_err(cannot invalidate PEB %d, read returned %d write returned 
 +%d,

Why? The error actually happened in mtd_write. No read errors lead to this path.

   pnum, err, err1);

I guess it is better to kill the err1 variable and use just 'err'
everywhere.

Anyway, I did not respond for long time, so I decided to do all these 
modifications myself. Could you please review and test the below patch, which 
is also attached. I did not compile it even. If there are issues, just fix-it 
up and send the the fix or the fixed version. If it is OK, give me your 
Tested-by: please. Thanks!


From: =?UTF-8?q?Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20=28qiwang=29?=
 qiw...@micron.com
Date: Wed, 1 Jan 2014 13:06:11 +
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure  
interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang qiw...@micron.com
---
 drivers/mtd/ubi/io.c | 55 ++--
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index bf79def..ae9e5ca 
100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)  {
-   int err, err1;
+   int err;
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
+
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header @@ 
-509,50 +511,39 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, we have to corrupt them before erasing.
 * 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 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
err

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

OK, thank you Artem.

I will change to git send-mail next time. And I will be very careful to push 
patch to you next time.
Sorry to bring so much trouble to you as I am first time to push patch to Linux 
main trunk.(Maybe it also is Micron Technology company first time to push patch 
to Linux main trunk, I am being Micron China Shanghai team volunteer to contact 
Linux committee to push patch). 
Anyway, Thanks for your patience to give me a good starting. 
And if you have anything need my or my team help, Please kindly let me know. 
Our team is glad to contribute to Linux community.
Wish you have a wonderful 2014.
Thank you


-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Thursday, January 02, 2014 11:23 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

On Thu, 2014-01-02 at 17:11 +0200, Artem Bityutskiy wrote:
> Anyway, I did not respond for long time, so I decided to do all these
> modifications myself. Could you please review and test the below patch,
> which is also attached. I did not compile it even. If there are issues,
> just fix-it up and send the the fix or the fixed version. If it is OK,
> give me your "Tested-by:" please. Thanks!

Actually, for convenience, I've pushed this patch to the 'qi' branch of
the 'linux-ubifs' git tree:

git://git.infradead.org/ubifs-2.6.git

-- 
Best Regards,
Artem Bityutskiy

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Richard:
Artem already has push this patch to 'linux-ubifs' git tree.  
Thanks for you remind, I will change to git send-mail next time. 
Thanks again for  your reply. I will be very careful to push patch to you next 
time. 

Thanks a lot

-Original Message-
From: Richard Weinberger [mailto:rich...@nod.at] 
Sent: Thursday, January 02, 2014 10:58 PM
To: Qi Wang 王起 (qiwang)
Cc: Richard Weinberger; dedeki...@gmail.com; linux-kernel@vger.kernel.org; 
linux-...@lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

Am Mittwoch, 1. Januar 2014, 13:11:21 schrieb Qi Wang 王起:
> If have any questions, please let me know. 

Did you send the patch again using Outlook?
Or are you behind a MS Exchange server?

The patch is still malformed. :-(
e.g. it is base64 encoded.

Can you please try using git send-email?

Thanks,
//richard

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Richard:
Artem already has push this patch to 'linux-ubifs' git tree.  
Thanks for you remind, I will change to git send-mail next time. 
Thanks again for  your reply. I will be very careful to push patch to you next 
time. 

Thanks a lot

-Original Message-
From: Richard Weinberger [mailto:rich...@nod.at] 
Sent: Thursday, January 02, 2014 10:58 PM
To: Qi Wang 王起 (qiwang)
Cc: Richard Weinberger; dedeki...@gmail.com; linux-kernel@vger.kernel.org; 
linux-...@lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

Am Mittwoch, 1. Januar 2014, 13:11:21 schrieb Qi Wang 王起:
 If have any questions, please let me know. 

Did you send the patch again using Outlook?
Or are you behind a MS Exchange server?

The patch is still malformed. :-(
e.g. it is base64 encoded.

Can you please try using git send-email?

Thanks,
//richard

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

OK, thank you Artem.

I will change to git send-mail next time. And I will be very careful to push 
patch to you next time.
Sorry to bring so much trouble to you as I am first time to push patch to Linux 
main trunk.(Maybe it also is Micron Technology company first time to push patch 
to Linux main trunk, I am being Micron China Shanghai team volunteer to contact 
Linux committee to push patch). 
Anyway, Thanks for your patience to give me a good starting. 
And if you have anything need my or my team help, Please kindly let me know. 
Our team is glad to contribute to Linux community.
Wish you have a wonderful 2014.
Thank you


-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Thursday, January 02, 2014 11:23 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-...@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after 
erasure interrupted

On Thu, 2014-01-02 at 17:11 +0200, Artem Bityutskiy wrote:
 Anyway, I did not respond for long time, so I decided to do all these
 modifications myself. Could you please review and test the below patch,
 which is also attached. I did not compile it even. If there are issues,
 just fix-it up and send the the fix or the fixed version. If it is OK,
 give me your Tested-by: please. Thanks!

Actually, for convenience, I've pushed this patch to the 'qi' branch of
the 'linux-ubifs' git tree:

git://git.infradead.org/ubifs-2.6.git

-- 
Best Regards,
Artem Bityutskiy

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Richard:

I modified the place you mentioned in previous mail, please check it as below:

From: Qi Wang 

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

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

Hi Richard:
ok, I can correct my email address and re-send you again. Please check it. 
Thanks for your great help.


-Original Message-
From: Richard Weinberger [mailto:richard.weinber...@gmail.com] 
Sent: Tuesday, December 31, 2013 8:15 PM
To: Qi Wang 王起 (qiwang)
Cc: dedeki...@gmail.com; linux-...@lists.infradead.org; Adrian Hunter; 
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after 
erasure interrupted

On Thu, Nov 7, 2013 at 9:18 AM, Qi Wang 王起 (qiwang)  wrote:
> Hi:
> As we talked in mail before, please check my patch as below:
>
> From: Qi Wang 
>
> 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 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 

This email address is malformed.

> ---
> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
> index bf79def..0a6343d 100644
> --- a/drivers/mtd/ubi/io.c
> +++ b/drivers/mtd/ubi/io.c
> @@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
> pnum)
> size_t written;
> loff_t addr;
> uint32_t data = 0;
> +   struct ubi_ec_hdr ec_hdr;
> /*
>  * Note, we cannot generally define VID header buffers on stack,
>  * because of the way we deal with these buffers (see the header
> @@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, 
> int pnum)
> struct ubi_vid_hdr vid_hdr;
>
> /*
> +* If VID or EC is valid, need to corrupt it 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, , (void *));
> -   if (!err) {
> -   addr += ubi->vid_hdr_aloffset;
> -   err = mtd_write(ubi->mtd, addr, 4, , (void *));
> -   if (!err)
> -   return 0;
> +   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
> +   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
> +   err != UBI_IO_FF){
> +   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
> +   if(err1)
> +   goto error;
> }
>
> -   /*
> -* We failed to write to the media. This was observed with Spansion
> -* S29GL512N NOR flash. 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.
> -*/
> -   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
> -   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
> -   err1 == UBI_IO_FF) {
> -   struct ubi_ec_hdr ec_hdr;
> -
> -   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
> -   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR 
> ||
> -   err1 == UBI_IO_FF)
> -   /*
> -* Both VID and EC headers are corrupted, so we can
> -* safely erase this PEB and not afraid that it will 
> be
> -* treated as a valid PEB in case of an unclean 
> reboot.
> -*/
> -   return 0;
> +   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
> +   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
> +   err != UBI_IO_FF){
> +   addr += ubi->vid_hdr_aloffset;
> +   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
> +   if (err1)
> +   goto error;
> }
> +   return 0;
>
> +error:
> /*
> -* The PEB contains a valid VID header, but we cannot invalidate it.
> +* The PEB contains a valid VID or EC header, but we cannot 
> invalidate it.
>  

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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.

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

Hi Richard:
ok, I can correct my email address and re-send you again. Please check it. 
Thanks for your great help.


-Original Message-
From: Richard Weinberger [mailto:richard.weinber...@gmail.com] 
Sent: Tuesday, December 31, 2013 8:15 PM
To: Qi Wang 王起 (qiwang)
Cc: dedeki...@gmail.com; linux-...@lists.infradead.org; Adrian Hunter; 
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after 
erasure interrupted

On Thu, Nov 7, 2013 at 9:18 AM, Qi Wang 王起 (qiwang) qiw...@micron.com wrote:
 Hi:
 As we talked in mail before, please check my patch as below:

 From: Qi Wang qiw...@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 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@qiw...@micron.com

This email address is malformed.

 ---
 diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
 index bf79def..0a6343d 100644
 --- a/drivers/mtd/ubi/io.c
 +++ b/drivers/mtd/ubi/io.c
 @@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
 pnum)
 size_t written;
 loff_t addr;
 uint32_t data = 0;
 +   struct ubi_ec_hdr ec_hdr;
 /*
  * Note, we cannot generally define VID header buffers on stack,
  * because of the way we deal with these buffers (see the header
 @@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, 
 int pnum)
 struct ubi_vid_hdr vid_hdr;

 /*
 +* If VID or EC is valid, need to corrupt it 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;
 +   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
 +   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
 +   err != UBI_IO_FF){
 +   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
 +   if(err1)
 +   goto error;
 }

 -   /*
 -* We failed to write to the media. This was observed with Spansion
 -* S29GL512N NOR flash. 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.
 -*/
 -   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
 -   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
 -   err1 == UBI_IO_FF) {
 -   struct ubi_ec_hdr ec_hdr;
 -
 -   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
 -   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR 
 ||
 -   err1 == UBI_IO_FF)
 -   /*
 -* Both VID and EC headers are corrupted, so we can
 -* safely erase this PEB and not afraid that it will 
 be
 -* treated as a valid PEB in case of an unclean 
 reboot.
 -*/
 -   return 0;
 +   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
 +   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
 +   err != UBI_IO_FF){
 +   addr += ubi-vid_hdr_aloffset;
 +   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
 +   if (err1)
 +   goto error;
 }
 +   return 0;

 +error:
 /*
 -* The PEB contains a valid VID header, but we cannot invalidate it.
 +* The PEB contains a valid VID or EC header, but we cannot 
 invalidate it.
  * Supposedly the flash media or the driver is screwed up, so return 
 an
  * error.
  */
 -   ubi_err(cannot invalidate PEB %d, write returned %d read returned 
 %d,
 +   ubi_err(cannot invalidate PEB %d, read returned %d write returned 
 %d,
 pnum, err, err1);
 ubi_dump_flash(ubi, pnum, 0, ubi-peb_size

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Richard:

I modified the place you mentioned in previous mail, please check it as below:

From: Qi Wang qiw...@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 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.

This patch is validated on Micron NOR flash, part number is：JS28F512M29EWHA

Signed-off-by: Qi Wang qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

Hi Artem:
Sorry to interrupt your busy life. 
As you said in previous mail, I send my patch separately without quoting this 
e-mail. And I have send to you, but I never get  your reply. I am very confuse, 
no sure if is there anything wrong at the patch I send to you.
Can you help explain to me?
Thanks

-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Friday, November 01, 2013 4:58 PM
To: Qi Wang 王起 (qiwang)
Cc: linux-...@lists.infradead.org; Adrian Hunter; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after 
erasure interrupted

Hi,

could you please re-send your patch separately, without quoting any
parts of this conversation, so that I could use 'git am'.

Your patch also contains trailing white-spaces, please, get rid of them
in the next submission.

Also, could you please clearly state whether you have tested this patch
on a real NOR flash or not. If yes, then could you share the chip
vendor/type information?

On Thu, 2013-10-31 at 04:07 +, Qi Wang 王起 (qiwang) wrote:

> --- a/drivers/mtd/ubi/io.c
> +++ b/drivers/mtd/ubi/io.c
> @@ -499,59 +499,44 @@ static int nor_erase_prepare(struct ubi_device *ubi, 
> int pnum)
>   size_t written;
>   loff_t addr;
>   uint32_t data = 0;
> - /*
> -  * Note, we cannot generally define VID header buffers on stack,
> -  * because of the way we deal with these buffers (see the header
> -  * comment in this file). But we know this is a NOR-specific piece of
> -  * code, so we can do this. But yes, this is error-prone and we should
> -  * (pre-)allocate VID header buffer instead.
> -  */

Please, do not remove this comment.

>   struct ubi_vid_hdr vid_hdr;
> + struct ubi_ec_hdr ec_hdr;

To make it obvious what the above big comment talks about, could you
please define 'struct ubi_ec_hdr ec_hdr' above that big comment.

Otherwise looks good to me, thank you!


> My Comments for above changing:
> 1. 
>   -   /*
>   -* Note, we cannot generally define VID header buffers on stack,
>   -* because of the way we deal with these buffers (see the header
>   -* comment in this file). But we know this is a NOR-specific 
> piece of
>   -* code, so we can do this. But yes, this is error-prone and we 
> should
>   -* (pre-)allocate VID header buffer instead.
>   -*/
>   I remove above comment, because I pre-allocate VID header and EC header 
> together. 
>   So I think no need to emphasize VID header buffers cannot be on stack.
>   (Maybe my understanding about this comment is error, if so, please 
> correct me)

The problem is that some functions in io.c can read or write _beyond_
sizeof(struct ubi_vid_hdr), but this is only relevant to NAND, not for
NOR, and the code you change is NOR-only. This is why that comment is
there, and I'd like to keep it.

> 2.
>   why use
>   "if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR && err != 
> UBI_IO_FF)"
>   but not 
>   "if (!err)" 
>   to judge if need to program '0' to invalid this block.
> 
>   In case err == UBI_IO_FF_BITFLIPS, err == UBI_IO_BITFLIPS or unexpected 
> value return
>   from read function, I think UBI still need to invalid this block for 
> above mentioned 
>   condition. So I use
>   "if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR && err != 
> UBI_IO_FF)"
>   to judge. 

In case of UBI_IO_FF (all FFs) UBI will erase the eraseblock before
using it anyway, so invalidation is not necessary.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

Hi Artem:
Sorry to interrupt your busy life. 
As you said in previous mail, I send my patch separately without quoting this 
e-mail. And I have send to you, but I never get  your reply. I am very confuse, 
no sure if is there anything wrong at the patch I send to you.
Can you help explain to me?
Thanks

-Original Message-
From: Artem Bityutskiy [mailto:dedeki...@gmail.com] 
Sent: Friday, November 01, 2013 4:58 PM
To: Qi Wang 王起 (qiwang)
Cc: linux-...@lists.infradead.org; Adrian Hunter; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after 
erasure interrupted

Hi,

could you please re-send your patch separately, without quoting any
parts of this conversation, so that I could use 'git am'.

Your patch also contains trailing white-spaces, please, get rid of them
in the next submission.

Also, could you please clearly state whether you have tested this patch
on a real NOR flash or not. If yes, then could you share the chip
vendor/type information?

On Thu, 2013-10-31 at 04:07 +, Qi Wang 王起 (qiwang) wrote:

 --- a/drivers/mtd/ubi/io.c
 +++ b/drivers/mtd/ubi/io.c
 @@ -499,59 +499,44 @@ static int nor_erase_prepare(struct ubi_device *ubi, 
 int pnum)
   size_t written;
   loff_t addr;
   uint32_t data = 0;
 - /*
 -  * Note, we cannot generally define VID header buffers on stack,
 -  * because of the way we deal with these buffers (see the header
 -  * comment in this file). But we know this is a NOR-specific piece of
 -  * code, so we can do this. But yes, this is error-prone and we should
 -  * (pre-)allocate VID header buffer instead.
 -  */

Please, do not remove this comment.

   struct ubi_vid_hdr vid_hdr;
 + struct ubi_ec_hdr ec_hdr;

To make it obvious what the above big comment talks about, could you
please define 'struct ubi_ec_hdr ec_hdr' above that big comment.

Otherwise looks good to me, thank you!


 My Comments for above changing:
 1. 
   -   /*
   -* Note, we cannot generally define VID header buffers on stack,
   -* because of the way we deal with these buffers (see the header
   -* comment in this file). But we know this is a NOR-specific 
 piece of
   -* code, so we can do this. But yes, this is error-prone and we 
 should
   -* (pre-)allocate VID header buffer instead.
   -*/
   I remove above comment, because I pre-allocate VID header and EC header 
 together. 
   So I think no need to emphasize VID header buffers cannot be on stack.
   (Maybe my understanding about this comment is error, if so, please 
 correct me)

The problem is that some functions in io.c can read or write _beyond_
sizeof(struct ubi_vid_hdr), but this is only relevant to NAND, not for
NOR, and the code you change is NOR-only. This is why that comment is
there, and I'd like to keep it.

 2.
   why use
   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR  err != 
 UBI_IO_FF)
   but not 
   if (!err) 
   to judge if need to program '0' to invalid this block.
 
   In case err == UBI_IO_FF_BITFLIPS, err == UBI_IO_BITFLIPS or unexpected 
 value return
   from read function, I think UBI still need to invalid this block for 
 above mentioned 
   condition. So I use
   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR  err != 
 UBI_IO_FF)
   to judge. 

In case of UBI_IO_FF (all FFs) UBI will erase the eraseblock before
using it anyway, so invalidation is not necessary.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

[PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

[qiwang]

Hi:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

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

[qiwang]

Hi:
As we talked in mail before, please check my patch as below:

From: Qi Wang 

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 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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   if (!err)
-   return 0;
+   err = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, _hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, _hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+   err != UBI_IO_FF){
+   addr += ubi->vid_hdr_aloffset;
+   err1 = mtd_write(ubi->mtd, addr, 4, , (void *));
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+   ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

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

[qiwang]

Hi:
As we talked in mail before, please check my patch as below:

From: Qi Wang qiw...@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 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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
+   struct ubi_ec_hdr ec_hdr;
/*
 * Note, we cannot generally define VID header buffers on stack,
 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
struct ubi_vid_hdr vid_hdr;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if(err1)
+   goto error;
}
 
-   /*
-* We failed to write to the media. This was observed with Spansion
-* S29GL512N NOR flash. 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.
-*/
-   err1 = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF) {
-   struct ubi_ec_hdr ec_hdr;
-
-   err1 = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0);
-   if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-   err1 == UBI_IO_FF)
-   /*
-* Both VID and EC headers are corrupted, so we can
-* safely erase this PEB and not afraid that it will be
-* treated as a valid PEB in case of an unclean reboot.
-*/
-   return 0;
+   err = ubi_io_read_vid_hdr(ubi, pnum, vid_hdr, 0);
+   if (err != UBI_IO_BAD_HDR_EBADMSG  err != UBI_IO_BAD_HDR 
+   err != UBI_IO_FF){
+   addr += ubi-vid_hdr_aloffset;
+   err1 = mtd_write(ubi-mtd, addr, 4, written, (void *)data);
+   if (err1)
+   goto error; 
}
+   return 0;
 
+error:
/*
-* The PEB contains a valid VID header, but we cannot invalidate it.
+* The PEB contains a valid VID or EC header, but we cannot invalidate 
it.
 * Supposedly the flash media or the driver is screwed up, so return an
 * error.
 */
-   ubi_err(cannot invalidate PEB %d, write returned %d read returned %d,
+   ubi_err(cannot invalidate PEB %d, read returned %d write returned %d,
pnum, err, err1);
ubi_dump_flash(ubi, pnum, 0, ubi-peb_size);
return -EIO;
---

I have tested this patch on Micron NOR flash, part number is：JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiw...@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

On Tue, 2013-10-29 at 11:19 +, Artem Bityutskiy wrote:
> On Mon, 2013-10-28 at 04:54 +, Qi Wang 王起 (qiwang) wrote:
> > On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
> > > On Thu, 2013-10-10 at 08:28 +, 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 
> > 
> > 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 
> > ---
> > 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, , (void *));
> > -   if (!err) {
> > -   addr += ubi->vid_hdr_aloffset;
> > -   err = mtd_write(ubi->mtd, addr, 4, , (void *));
> > -   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


Hi Artem:
Please check below patch. 

From: Qi Wang 

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 
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0fdaca9 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,59 +499,44 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
-   /*
-* Note, we cannot generally define VID header buffers on stack,
-* because of the way we deal with these buffers (see the header
-* comment in this file). But we know this is a NOR-specific piece of
-* code, so we can do this. But yes, this is error-prone and we should
-* (pre-)allocate VID header buffer instead.
-*/
struct ubi_vid_hdr vid_hdr;
+   struct ubi_ec_hdr ec_hdr;
+
+   addr = (loff_t)pnum * ubi->peb_size;
 
/*
+* If VID or EC is valid, need to corrupt it before erase operation.  
 * It is important to first invalidate the EC header, and then the VID
 * header. Otherwise a power cut may

RE: [PATCH 1/1] MTD: UBI: try to avoid program data to NOR flash after erasure interrupted

[qiwang]

On Tue, 2013-10-29 at 11:19 +, Artem Bityutskiy wrote:
 On Mon, 2013-10-28 at 04:54 +, Qi Wang 王起 (qiwang) wrote:
  On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
   On Thu, 2013-10-10 at 08:28 +, 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 qiw...@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@qiw...@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


Hi Artem:
Please check below patch. 

From: Qi Wang qiw...@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@qiw...@micron.com
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0fdaca9 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,59 +499,44 @@ static int nor_erase_prepare(struct ubi_device *ubi, int 
pnum)
size_t written;
loff_t addr;
uint32_t data = 0;
-   /*
-* Note, we cannot generally define VID header buffers on stack,
-* because of the way we deal with these buffers (see the header
-* comment in this file). But we know this is a NOR-specific piece of
-* code, so we can do this. But yes, this is error-prone and we should
-* (pre-)allocate VID header buffer instead.
-*/
struct ubi_vid_hdr vid_hdr;
+   struct ubi_ec_hdr ec_hdr;
+
+   addr = (loff_t)pnum * ubi-peb_size;
 
/*
+* If VID or EC is valid, need to corrupt it 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;
+   err = ubi_io_read_ec_hdr(ubi, pnum, ec_hdr, 0

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

[qiwang]

On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
>On Thu, 2013-10-10 at 08:28 +, 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 

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 
---
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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   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, _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, , (void *));
+   if (!err) {
+   addr += ubi->vid_hdr_aloffset;
+   err = mtd_write(ubi->mtd, addr, 4, , (void *));
+   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.
---

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

[qiwang]

On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
>On Thu, 2013-10-10 at 08:28 +, 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 

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 
---
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, , (void *));
-   if (!err) {
-   addr += ubi->vid_hdr_aloffset;
-   err = mtd_write(ubi->mtd, addr, 4, , (void *));
-   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, _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, , (void *));
+   if (!err) {
+   addr += ubi->vid_hdr_aloffset;
+   err = mtd_write(ubi->mtd, addr, 4, , (void *));
+   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.
---

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

[qiwang]

On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
On Thu, 2013-10-10 at 08:28 +, 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 qiw...@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@qiw...@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;
-   }
-
-   /*
-* 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.
---

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

[qiwang]

On Sa, 2013-10-26 at 05:19 +, Artem Bityutskiy wrote:
On Thu, 2013-10-10 at 08:28 +, 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 qiw...@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@qiw...@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;
-   }
-
-   /*
-* 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.
---