Patchwork UBI: corrupted PEB detection for Numonyx P30

login
register
mail settings
Submitter Holger Brunck
Date Jan. 24, 2011, 1:12 p.m.
Message ID <4D3D7AB5.50401@keymile.com>
Download mbox | patch
Permalink /patch/80152/
State New
Headers show

Comments

Holger Brunck - Jan. 24, 2011, 1:12 p.m.
Hi all,
we have on a powerpc based board with a new Numonyx P30 65nm 1GBit flash an
error seen on the PEB detection of UBI.

If we do a power cut on the board we get an similar situation as on Spansion
S29GL512N. After a reboot the eraseblock is unwriteable and we can not
invalidate the VID and EC header. In u-boot the area looks like:

=> md 0x56180000

56180000: 00000000 00000000 00000000 00000000    ................
56180010: 00000000 00000000 00000000 00000000    ................
56180020: ffffffff ffffffff ffffffff ffffffff    ................
56180030: ffffffff ffffffff ffffffff ffffffff    ................
56180040: ffffffff ffffffff ffffffff ffffffff    ................
56180050: ffffffff ffffffff ffffffff ffffffff    ................

During bootime we hit the checks for valid VID and EC header in nor_erase_prepare:
err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);

Both functions were only checked for (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 ==
UBI_IO_BAD_HDR). But in our case we have a VID header with all FFs and the
return code is UBI_IO_FF. So I think it must be checked also for UBI_IO_FF and
UBI_IO_FF_BITFLIPS.

The following patch seems to solve our problem:


Does anyone see a problem with this patch? If not I can post it as a git patch
on the mailing list.

Best regards
Holger Brunck
Artem Bityutskiy - Jan. 25, 2011, 11:11 a.m.
On Mon, 2011-01-24 at 14:12 +0100, Holger Brunck wrote:
>         err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
> -       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR) {
> +       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
> +           err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
>                 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)
> +               if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
> +                   err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
>                         /*
>                          * Both VID and EC headers are corrupted, so we can
>                          * safely erase this PEB and not afraid that it will be
> 
> Does anyone see a problem with this patch? If not I can post it as a git patch
> on the mailing list.

Hi, I agree with checking for UBI_IO_FF, but not for UBI_IO_FF_BITFLIPS,
because (a) NOR should not have them and (b) bit-flip means the the
header is correct, but there was a correctable bit-blip. But we want to
find the headers corrupted in this piece of code. 

I've prepared and pushed almost the same patch, made you the author:
http://git.infradead.org/ubi-2.6.git/commit/4782254b5f76c22726d270ea753816cefa088ae9

Please, take a look and check. If you do not like it, let me know or
just send the patch you like.

Thanks!
Holger Brunck - Jan. 25, 2011, 11:31 a.m.
Artem Bityutskiy wrote:
> On Mon, 2011-01-24 at 14:12 +0100, Holger Brunck wrote:
>>         err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
>> -       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR) {
>> +       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
>> +           err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
>>                 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)
>> +               if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
>> +                   err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
>>                         /*
>>                          * Both VID and EC headers are corrupted, so we can
>>                          * safely erase this PEB and not afraid that it will be
>>
>> Does anyone see a problem with this patch? If not I can post it as a git patch
>> on the mailing list.
> 
> Hi, I agree with checking for UBI_IO_FF, but not for UBI_IO_FF_BITFLIPS,
> because (a) NOR should not have them and (b) bit-flip means the the
> header is correct, but there was a correctable bit-blip. But we want to
> find the headers corrupted in this piece of code. 
> 

Ok.

> I've prepared and pushed almost the same patch, made you the author:
> http://git.infradead.org/ubi-2.6.git/commit/4782254b5f76c22726d270ea753816cefa088ae9
> 
> Please, take a look and check. If you do not like it, let me know or
> just send the patch you like.
> 

Yes this is ok for me.

Thanks.

Best regards
Holger Brunck

Patch

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 811775a..407aa46 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -507,11 +507,13 @@  static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
         * PEB.
         */
        err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR) {
+       if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
+           err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
                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)
+               if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
+                   err1 == UBI_IO_FF || err1 == UBI_IO_FF_BITFLIPS) {
                        /*
                         * Both VID and EC headers are corrupted, so we can
                         * safely erase this PEB and not afraid that it will be