Re: [PATCH] mtd: spi-nor: Add support for ISSI is25lp128

[Gabor Juhos <juhosg@openwrt.org>]

Hi Brian,

> + Gabor, who submitted the other ISSI entry; and some others
> 
> On Tue, Feb 09, 2016 at 09:12:43PM +0300, Nikita Nazarenko wrote:
>> Signed-off-by: Nikita Nazarenko <nnazarenko@radiofid.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index ed0c19c..e0bb151 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -739,6 +739,7 @@ static const struct flash_info spi_nor_ids[] = {
>>  
>>  	/* ISSI */
> 
> Both of these ISSI entries look wrong.
> 
>>  	{ "is25cd512", INFO(0x7f9d20, 0, 32 * 1024,   2, SECT_4K) },
> 
> Looking at the datasheet for this part [1], shouldn't the ID be
> 0x9d7f20 (i.e., the MFR ID is 0x9d, not 0x7f)? That would match the
> existing CFI definition for "PMC". And IIUC, PMC got bought by ISSI? Or
> some kind of merger, I don't follow these things.

The ID is definitely 0x7d9d20, the m28p80 driver detects the device correctly
with that:

  m25p80 spi0.0: found is25cd512, expected m25p80
  m25p80 spi0.0: is25cd512 (64 Kbytes)
  Creating 4 MTD partitions on "spi0.0":
  0x000000000000-0x00000000c000 : "routerboot"
  0x00000000c000-0x00000000d000 : "hard_config"
  0x00000000d000-0x00000000e000 : "bios"
  0x00000000e000-0x00000000f000 : "soft_config"

The first byte of the manufacturer ID is indeed 0x9d, but the device returns
that as the second byte throught the JEDEC ID READ command. Even though it is a
weird behaviour, it is described on page 11 of the datasheet.

>> +	{ "is25lp128", INFO(0x9d6018, 0, 32 * 1024,   512, SECT_4K) },
> 
> This datasheet [1] says that for SPI mode (not QPI mode), 4K erase is
> done with opcode 0xD7 (i.e., SPINOR_OP_BE_4K_PMC) not 0x20
> (SPINOR_OP_BE_4K). So we would need the SECT_4K_PMC, not the SECT_4K
> flag.

I don't have a board with such flash device, but Table 8.1 "Instruction Set" is
misleading a bit. In my understanding, erasing 4K sectors should work with both
commands. Look at Clause 8.10 "SECTOR ERASE OPERATION (SER, D7h/20h)" on page 33
in the datasheet. Both Figure 8.12 "Sector Erase Sequence" and Figure 8.13
"Sector Erase Sequence (QPI) " says 'D7h/20h'.

> Also, the datasheet for this device says it supports 64K sectors, and
> the 32K sectors require a different erase command (SPINOR_OP_BE_32K; not
> currently supported in this driver). Did you test erase to be sure it
> worked as expected? Or are one or more datasheets wrong?

It seems that you are correct about this.

-Gabor

> 
> Regards,
> Brian
> 
>>  	/* Macronix */
>>  	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
> 
> [1] I'm looking at these:
> 
> http://www.issi.com/WW/pdf/25LP128.pdf
> http://www.issi.com/WW/pdf/25CD512-010-020.pdf
>