Skip to contentSkip to conversation

last commit breaks the AMD NOR flash functionality?

DISCUSSION Vitaly Wool 2 participants 4 messages May 24, 2005 Memory technology devices (MTD)

lore
Share & tools
lore

Vitaly Wool

author
#0 lore raw

Greetings,

the latest commit for cfi_cmdset_0002.c:

Quoted 10 lines

Index: cfi_cmdset_0002.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/cfi_cmdset_0002.c,v retrieving revision 1.115
retrieving revision 1.116
diff -u -r1.115 -r1.116

cfi_cmdset_0002.c line 1326

Quoted 8 lines
cfi_cmdset_0002.c +1 -1
@@ -1320,7 +1320,7 @@
 		cfi_spin_lock(chip->mutex);
 	}
 	/* Did we succeed? */
-	if (chip_good(map, adr, map_word_ff(map))) {
+	if (!chip_good(map, adr, map_word_ff(map))) {
 		/* reset on all failures. */
 		map_write( map, CMD(0xF0), chip->start );
 		/* FIXME - should have reset delay before continuing */

resulted in the following behavior for my AMD NOR flash:

# flash_eraseall /dev/mtd/3
Erasing 64 Kibyte @ 0 --  0 % complete.
flash_eraseall: /dev/mtd/3: MTD Erase failure: Input/output error
Erasing 64 Kibyte @ 10000 -- 25 % complete.
flash_eraseall: /dev/mtd/3: MTD Erase failure: Input/output error
Erasing 64 Kibyte @ 20000 -- 50 % complete.
flash_eraseall: /dev/mtd/3: MTD Erase failure: Input/output error
Erasing 64 Kibyte @ 30000 -- 75 % complete.
flash_eraseall: /dev/mtd/3: MTD Erase failure: Input/output error

If I roll it back, it starts working.
I also suspected something strange with 'if(chip_good(...)) ...' instead of 'if(!chip_good(...)) ...' but it worked before and it doesn't work now. Can anyone elaborate this a bit?

Best regards,
Vitaly

#1 lore raw
Quoted 11 lines · Vitaly Wool
Quoted 7 lines
	/* Did we succeed? */
(quoted diff) +1 -1
-	if (chip_good(map, adr, map_word_ff(map))) {
+	if (!chip_good(map, adr, map_word_ff(map))) {

		/* reset on all failures. */
		map_write( map, CMD(0xF0), chip->start );
		/* FIXME - should have reset delay before continuing */

If I roll it back, it starts working.
I also suspected something strange with 'if(chip_good(...)) ...' instead of 'if(!chip_good(...)) ...' but it worked before and it doesn't work now. Can anyone elaborate this a bit?

I had the same problem you see before the fix. It took a bit to figure out what was going wrong.

I'm quite sure, that my fix is correct, because the previous revision exited via the error path in case of no error.

I suspect, that something goes really wrong with your erase. You have to check the results in chip_good.

The chip_good function reads the same address twice and compares whether the results are equal. The chip good result also requires that the reads are all 0xff.

I suggest to add a check into chip_good, where you can identify which of the requirements is not fulfilled to further narrow down the source of trouble

tglx

author
#2 lore raw

Thanks Thomas,
I think I found the bug.

This is the 'problematic' part code in cfi_cmdset_0002.c:

Quoted 8 lines
        /* Did we succeed? */
        if (!chip_good(map, adr, map_word_ff(map))) {
                /* reset on all failures. */
                map_write( map, CMD(0xF0), chip->start );
                /* FIXME - should have reset delay before continuing */

                ret = -EIO;
        }

I inserted a stupid printk into the chip_good function:

Quoted 11 lines

static int chip_good(struct map_info *map, unsigned long addr, map_word expected)

{
        map_word oldd, curd;

        oldd = map_read(map, addr);
        curd = map_read(map, addr);

        printk("FFFFFF %ld %ld %ld\n", *oldd.x, *curd.x, *expected.x);
        return  map_word_equal(map, oldd, curd) &&
                map_word_equal(map, curd, expected);
}

And what I saw was:

...
FFFFFF 65535 65535 -1
...

And that's because map_word_ff is the following:

Quoted 10 lines

static inline map_word map_word_ff(struct map_info *map)

{
        map_word r;
        int i;

        for (i=0; i<map_words(map); i++) {
                r.x[i] = ~0UL;
        }
        return r;
}

which won't work in my case (16-bit flash access). The suggested fix is the following:

Index: include/linux/mtd/map.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/mtd/map.h,v
retrieving revision 1.48

linux/mtd/map.h 24 May 2005 15:51:28 -0000 renamed from linux/mtd/map.h 16 Feb 2005 15:54:59 -0000 1.48 +4 -1
diff -u -r1.48 map.h
@@ -346,7 +346,10 @@
       int i;
 
       for (i=0; i<map_words(map); i++) {
-               r.x[i] = ~0UL;
+               if (map_bankwidth_is_1(map) || map_bankwidth_is_2(map))
+                       r.x[i] = (1 << (8*map_bankwidth(map))) - 1;
+               else
+                       r.x[i] = ~0UL;
       }
       return r;

}

This fix at least works for my case :)

Best regards,
Vitaly

Quoted 40 lines · Thomas Gleixner
Quoted 13 lines · Vitaly Wool
Quoted 9 lines
	/* Did we succeed? */
(quoted diff) +1 -1
-	if (chip_good(map, adr, map_word_ff(map))) {
+	if (!chip_good(map, adr, map_word_ff(map))) {

		/* reset on all failures. */
		map_write( map, CMD(0xF0), chip->start );
		/* FIXME - should have reset delay before continuing */

If I roll it back, it starts working.
I also suspected something strange with 'if(chip_good(...)) ...' instead of 'if(!chip_good(...)) ...' but it worked before and it doesn't work now. Can anyone elaborate this a bit?

I had the same problem you see before the fix. It took a bit to figure out what was going wrong.

I'm quite sure, that my fix is correct, because the previous revision exited via the error path in case of no error.

I suspect, that something goes really wrong with your erase. You have to check the results in chip_good.

The chip_good function reads the same address twice and compares whether the results are equal. The chip good result also requires that the reads are all 0xff.

I suggest to add a check into chip_good, where you can identify which of the requirements is not fulfilled to further narrow down the source of trouble

tglx

#3 lore raw

Vitaly,

Quoted 2 lines · Vitaly Wool

which won't work in my case (16-bit flash access). The suggested fix is the following:

Thanks for pointing that out. I fixed it in CVS so it is working for 64bit machines too.

tglx