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

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

Hi Guillaume,

> > 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?

> > 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).

Kind regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]