[U-Boot] [PATCH 1/2] mtd: vf610_nfc: mark page as dirty on block erase

[Bill Pringlemeir]

On 24 Mar 2015, stefan at agner.ch wrote:

> The driver tries to re-use the page buffer by storing the page
> number of the current page in the buffer. The page is only read
> if the requested page number is not currently in the buffer. When
> a block is erased, the page number is marked as invalid if the
> erased page equals the one currently in the cache. However, since
> a erase block consists of multiple pages, also other page numbers
> could be affected.
>
> The commands to reproduce this issue (on a written page):
>> nand dump 0x800
>> nand erase 0x0 0x20000
>> nand dump 0x800
>
> The second nand dump command returns the data from the buffer,
> while in fact the page is erased (0xff).
>
> Avoid the hassle to calculate whether the page is affected or not,
> but set the page buffer unconditionally to invalid instead.
>
> Signed-off-by: Stefan Agner <stefan at agner.ch>
> ---
> This are two bug fixes which would be nice if they would still
> make it into 2015.04...
>
> drivers/mtd/nand/vf610_nfc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/vf610_nfc.c
> b/drivers/mtd/nand/vf610_nfc.c index 928d58b..9de971c 100644 ---
> a/drivers/mtd/nand/vf610_nfc.c +++ b/drivers/mtd/nand/vf610_nfc.c @@
> -369,8 +369,7 @@ static void vf610_nfc_command(struct mtd_info *mtd,
> unsigned command, break;
>
> 	case NAND_CMD_ERASE1: - if (nfc->page == page) - nfc->page =
> -1; + nfc->page = -1; vf610_nfc_send_commands(nfc->regs, command,
> NAND_CMD_ERASE2, ERASE_CMD_CODE); vf610_nfc_addr_cycle(mtd, column,
> page);

This change looks sensible.  It is also possible that because sub-pages
were removed that we could just remove the caching all together.  It is
possible that a higher layer may intentionally want to program and then
do a read to verify.

I had seen that different FS seem to do 'write' and then immediately
follow with a read.  If you believe the controller and the write status
was ok, then I think it is fine to reuse the existing buffer and keep
this caching.

For UBI, there were VID/EB header reads when sub-pages were enabled as
they are in the same page; but these are now seperate pages.
Especially, people may only care about code size in 'u-boot' and the
typical use will only be to read an image (or config) from NAND so this
optimization is probably not too helpful (execept maybe when flashing
new kernels).  The 2nd patch set is not needed if we do not re-use
existing data?

I guess we want to stay the same as the mainline Linux you are
submitting.  I haven't benchmarked the updates since sub-pages were
removed to see if this is worth it.  I think it was only ~10-20% in some
benchmark I was doing with the 'caching'.

At least in the small, this is a minimal change that is correct.

Ack-by: Bill Pringlemeir <bpringlemeir at nbsps.com>