* [U-Boot] [PATCH] - fix "nand erase clean" problem
@ 2008-10-26 16:48 Ilko Iliev
2008-10-27 18:50 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Ilko Iliev @ 2008-10-26 16:48 UTC (permalink / raw)
To: u-boot
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
--
Mit freundlichen Gr??en/With best regards,
Ilko Iliev
Ronetix Development Tools GmbH
CPU Modules, JTAG/BDM Emulators and Flash Programmers
Waidhausenstrasse 13/5, 1140 Vienna, Austria
E-Mail: iliev at ronetix.at; Web: www.ronetix.at
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] - fix "nand erase clean" problem
2008-10-26 16:48 [U-Boot] [PATCH] - fix "nand erase clean" problem Ilko Iliev
@ 2008-10-27 18:50 ` Scott Wood
2008-10-27 19:39 ` Ilko Iliev
0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2008-10-27 18:50 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] - fix "nand erase clean" problem
2008-10-27 18:50 ` Scott Wood
@ 2008-10-27 19:39 ` Ilko Iliev
2008-10-27 19:49 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Ilko Iliev @ 2008-10-27 19:39 UTC (permalink / raw)
To: u-boot
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.
--
Mit freundlichen Gr??en/With best regards,
Ilko Iliev
Ronetix Development Tools GmbH
CPU Modules, JTAG/BDM Emulators and Flash Programmers
Waidhausenstrasse 13/5, 1140 Vienna, Austria
E-Mail: iliev at ronetix.at; Web: www.ronetix.at
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] - fix "nand erase clean" problem
2008-10-27 19:39 ` Ilko Iliev
@ 2008-10-27 19:49 ` Scott Wood
2008-10-28 10:45 ` Ilko Iliev
0 siblings, 1 reply; 6+ messages in thread
From: Scott Wood @ 2008-10-27 19:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] - fix "nand erase clean" problem
2008-10-27 19:49 ` Scott Wood
@ 2008-10-28 10:45 ` Ilko Iliev
2008-10-28 16:55 ` Scott Wood
0 siblings, 1 reply; 6+ messages in thread
From: Ilko Iliev @ 2008-10-28 10:45 UTC (permalink / raw)
To: u-boot
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.
--
Mit freundlichen Gr??en/With best regards,
Ilko Iliev
Ronetix Development Tools GmbH
CPU Modules, JTAG/BDM Emulators and Flash Programmers
Waidhausenstrasse 13/5, 1140 Vienna, Austria
E-Mail: iliev at ronetix.at; Web: www.ronetix.at
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] - fix "nand erase clean" problem
2008-10-28 10:45 ` Ilko Iliev
@ 2008-10-28 16:55 ` Scott Wood
0 siblings, 0 replies; 6+ messages in thread
From: Scott Wood @ 2008-10-28 16:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-28 16:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-10-26 16:48 [U-Boot] [PATCH] - fix "nand erase clean" problem Ilko Iliev
2008-10-27 18:50 ` Scott Wood
2008-10-27 19:39 ` Ilko Iliev
2008-10-27 19:49 ` Scott Wood
2008-10-28 10:45 ` Ilko Iliev
2008-10-28 16:55 ` Scott Wood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox