[PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips

[Guillaume LECERF]

Hi Wolfram,

>> > Okay, my remark was rubbish, yet the comment in the source was a bit confusing,
>> > too. It is correct, though maybe the term 'region' is a bit overloaded. What
>> > about replacing both comments with something a bit simpler like this:
>> >
>> > /*
>> >  * These flashes report two seperate eraseblock regions based on the
>> >  * sector_erase-size and block_erase-size, although they both operate on the
>> >  * same memory. This is not allowed according to CFI, so we just pick the
>> >  * sector_erase-size.
>> >  */
>> >
>
> Any opinion about this change?

I'm of course ok with this change, I integrated it in my patch.

> > This is according to the datasheets. You pick the block-data size here
>> > (ususally using command 0x50). Why is that? I tried sector_size on a
>> > SST39WF1601 and it works fine so far, testing with mtd_stresstest. (Sidenote: I
>> > have to use JEDEC-probing though, as my flashes don't report 0x701 but 0x002
>> > (AMD standard) as their primary command set. But they still need their custom
>> > unlock address :( )
>>
>> The 39VF3201 chip I use is on a brcm63xx board, running OpenWRT. It
>> reports 2 erase regions (as indicated in the datasheet):
>>
>> Number of Erase Block Regions: 2
>>   Erase Region #0: BlockSize 0x1000 bytes, 1024 blocks
>>   Erase Region #1: BlockSize 0x10000 bytes, 64 blocks
>
> Yes, same here.
>
>> But the function parse_cfe_partitions() in
>> drivers/mtd/maps/bcm963xx-flash.c [1] tries to read the partition
>> table at $(master->erasesize) (cf. line 67).
>> If I use the sector_erase-size, parse_cfe_partitions() tries to read
>> at 0x1000, and fails because the partition table is actually at
>> 0x10000 on my flash.
>
> I see the problem, but think it should be fixed differently (in the driver?).
> cfi_cmdset_0002.c uses CMD 0x30 for erasing (line 1604 in mtd/master). The SST
> datasheet now tells that the chip then expects the sector address (table 6 or
> figure 11). Given that I wonder if erasing truly works unless I missed
> something (at least it fails with mtd_stresstest for me if I change my
> jedec_probe to use 0x10000 instead of 0x1000).

Ok, you were right, the problem is in the driver.
I tried to use the hardcoded value 0x10000 instead of mtd->erasesize,
and use the sector_erase-size.
mtd_stresstest now runs smoothly, so I need to ask bcm963xx-flash.c
authors why they use this variable.

I continue my tests and discussions with the bcm963xx-flash.c authors
and get back when I've got a clean solution.

Thanks for your investigations !

Regards.
-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org