[U-Boot] [PATCH] - fix "nand erase clean" problem
With this patch "nand erase clean" writes correctly the cleanmarkers. Without this patch "nand erase clean" fills the OOB with zeros which marks all blocks as bad.
Signed-off-by: Ilko Iliev iliev@ronetix.at --- drivers/mtd/nand/nand_util.c | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 52b3d21..a601772 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -156,10 +156,19 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) /* format for JFFS2 ? */ if (opts->jffs2) {
- chip->ops.len = chip->ops.ooblen = 64; + if ( chip->ecc.layout->oobfree->length < cleanmarker.totlen ) { + memset(buf, 0xFF, sizeof(buf)); + chip->ops.oobbuf = buf; + chip->ops.ooboffs = chip->badblockpos & ~0x01; + chip->ops.len = chip->ops.ooblen = meminfo->oobsize; + } + else { + chip->ops.oobbuf = (uint8_t *)&cleanmarker; + chip->ops.ooboffs = chip->ecc.layout->oobfree->offset; + chip->ops.len = chip->ops.ooblen = cleanmarker.totlen; + } + chip->ops.datbuf = NULL; - chip->ops.oobbuf = buf; - chip->ops.ooboffs = chip->badblockpos & ~0x01;
result = meminfo->write_oob(meminfo, erase.addr + meminfo->oobsize, @@ -170,7 +179,8 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) continue; } else - printf("%s: MTD writeoob at 0x%08x\n",mtd_device, erase.addr + meminfo->oobsize ); + MTDDEBUG (MTD_DEBUG_LEVEL3, "%s: MTD writeoob at 0x%08x\n", + mtd_device, erase.addr + meminfo->oobsize ); }
if (!opts->quiet) { @@ -189,12 +199,13 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) if (percent != percent_complete) { percent_complete = percent;
- printf("\rErasing at 0x%x -- %3d%% complete.", - erase.addr, percent); + printf("\rErasing %sat 0x%x -- %3d%% complete.", + opts->jffs2 ? "and writing cleanmarker ": "", + erase.addr, percent);
if (opts->jffs2 && result == 0) - printf(" Cleanmarker written at 0x%x.", - erase.addr); + MTDDEBUG (MTD_DEBUG_LEVEL3, " Cleanmarker written at 0x%x.", + erase.addr); } } } -- 1.5.2.2
On Sun, Oct 26, 2008 at 05:48:47PM +0100, Ilko Iliev wrote:
With this patch "nand erase clean" writes correctly the cleanmarkers. Without this patch "nand erase clean" fills the OOB with zeros which marks all blocks as bad.
Signed-off-by: Ilko Iliev iliev@ronetix.at
drivers/mtd/nand/nand_util.c | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 52b3d21..a601772 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -156,10 +156,19 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) /* format for JFFS2 ? */ if (opts->jffs2) {
chip->ops.len = chip->ops.ooblen = 64;
if ( chip->ecc.layout->oobfree->length <cleanmarker.totlen ) {
Patch is linewrapped. Also, no space after ( or before ).
Why must the cleanmarker fit in the first free segment?
memset(buf, 0xFF, sizeof(buf));chip->ops.oobbuf = buf;chip->ops.ooboffs = chip->badblockpos &~0x01;
chip->ops.len = chip->ops.ooblen =meminfo->oobsize;
What if oobsize > 64 (as with 4k pages)? Why write anything at all if you're not going to write the cleanmarker? Why badblockpos & ~1 (I know existing code does it, but why)?
}else {
} else {
chip->ops.oobbuf = (uint8_t *)&cleanmarker;chip->ops.ooboffs =chip->ecc.layout->oobfree->offset;
chip->ops.len = chip->ops.ooblen =cleanmarker.totlen;
}
Set ooboffs to zero, and use MTD_OOB_AUTO.
-Scott
Dear Scott,
Scott Wood wrote:
On Sun, Oct 26, 2008 at 05:48:47PM +0100, Ilko Iliev wrote:
With this patch "nand erase clean" writes correctly the cleanmarkers. Without this patch "nand erase clean" fills the OOB with zeros which marks all blocks as bad.
Signed-off-by: Ilko Iliev iliev@ronetix.at
drivers/mtd/nand/nand_util.c | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 52b3d21..a601772 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -156,10 +156,19 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) /* format for JFFS2 ? */ if (opts->jffs2) {
chip->ops.len = chip->ops.ooblen = 64;
if ( chip->ecc.layout->oobfree->length <cleanmarker.totlen ) {
Patch is linewrapped. Also, no space after ( or before ).
I will send again with git-send-email.
Why must the cleanmarker fit in the first free segment?
The Linux NAND driver looks for the cleanmarkers at this place.
memset(buf, 0xFF, sizeof(buf));chip->ops.oobbuf = buf;chip->ops.ooboffs = chip->badblockpos &~0x01;
chip->ops.len = chip->ops.ooblen =meminfo->oobsize;
What if oobsize > 64 (as with 4k pages)? Why write anything at all if you're not going to write the cleanmarker? Why badblockpos & ~1 (I know existing code does it, but why)?
The current Linux NAND Flash driver supports 8, 16 and 64 bytes OOB. The "badblockpos & ~1" is in the current file - I have no idea why.
}else {} else {
chip->ops.oobbuf = (uint8_t *)&cleanmarker;chip->ops.ooboffs =chip->ecc.layout->oobfree->offset;
chip->ops.len = chip->ops.ooblen =cleanmarker.totlen;
}Set ooboffs to zero, and use MTD_OOB_AUTO.
-Scott
I think the NAND driver should work not only with MTD_OOB_AUTO.
Ilko Iliev wrote:
Why must the cleanmarker fit in the first free segment?
The Linux NAND driver looks for the cleanmarkers at this place.
AFAICT, it does a read using MTD_OOB_AUTO, which can span multiple free segments.
What if oobsize > 64 (as with 4k pages)? Why write anything at all if you're not going to write the cleanmarker? Why badblockpos & ~1 (I know existing code does it, but why)?
The current Linux NAND Flash driver supports 8, 16 and 64 bytes OOB.
No need to add a place that will silently break if that changes, though.
I think what needs to be done is a write to offset zero using MTD_OOB_AUTO. If it doesn't fit, then an error will be returned.
Set ooboffs to zero, and use MTD_OOB_AUTO.
I think the NAND driver should work not only with MTD_OOB_AUTO.
Explain? MTD_OOB_AUTO is a feature of the NAND subsystem, which automatically places user OOB data in the free regions described by the low-level driver. It's not some hardware feature that may or may not be present.
-Scott
Dear Scott,
Scott Wood wrote:
Ilko Iliev wrote:
Why must the cleanmarker fit in the first free segment?
The Linux NAND driver looks for the cleanmarkers at this place.
AFAICT, it does a read using MTD_OOB_AUTO, which can span multiple free segments.
Yes, but the current U-BOOT uses MTD_OOB_PLACE and the command "nand erase clean" marks all blocks as bad.
What if oobsize > 64 (as with 4k pages)? Why write anything at all if you're not going to write the cleanmarker? Why badblockpos & ~1 (I know existing code does it, but why)?
The current Linux NAND Flash driver supports 8, 16 and 64 bytes OOB.
No need to add a place that will silently break if that changes, though.
No, I think it will work also with OOB=128 bytes
I think what needs to be done is a write to offset zero using MTD_OOB_AUTO. If it doesn't fit, then an error will be returned.
Set ooboffs to zero, and use MTD_OOB_AUTO.
I think the NAND driver should work not only with MTD_OOB_AUTO.
Explain? MTD_OOB_AUTO is a feature of the NAND subsystem, which automatically places user OOB data in the free regions described by the low-level driver. It's not some hardware feature that may or may not be present.
At the moment U-BOOT doesn't use MTD_OOB_AUTO and the NAND flash can't be erased with "nand erase clean". I think this bug should be corrected instead of to switch to MTD_OOB_AUTO.
On Tue, Oct 28, 2008 at 11:45:21AM +0100, Ilko Iliev wrote:
AFAICT, it does a read using MTD_OOB_AUTO, which can span multiple free segments.
Yes, but the current U-BOOT uses MTD_OOB_PLACE and the command "nand erase clean" marks all blocks as bad.
I'm not defending the current code.
What if oobsize > 64 (as with 4k pages)? Why write anything at all if you're not going to write the cleanmarker? Why badblockpos & ~1 (I know existing code does it, but why)?
The current Linux NAND Flash driver supports 8, 16 and 64 bytes OOB.
No need to add a place that will silently break if that changes, though.
No, I think it will work also with OOB=128 bytes
Your patch as-is will write random data to the second half of a 128-byte OOB if it doesn't have enough space in the first free segment for the clean marker.
I think the NAND driver should work not only with MTD_OOB_AUTO.
Explain? MTD_OOB_AUTO is a feature of the NAND subsystem, which automatically places user OOB data in the free regions described by the low-level driver. It's not some hardware feature that may or may not be present.
At the moment U-BOOT doesn't use MTD_OOB_AUTO and the NAND flash can't be erased with "nand erase clean". I think this bug should be corrected instead of to switch to MTD_OOB_AUTO.
I think the bug should be corrected *by* switching to MTD_OOB_AUTO. What do you have against it?
-Scott
participants (2)
-
Ilko Iliev -
Scott Wood