[PATCH v2] mtd: spi-nor: add support for ISSI is25lp128

[PATCH v2] mtd: spi-nor: add support for ISSI is25lp128

[Angelo Dureghello]

Add support for ISSI is25lp128 spi nor flash.

Signed-off-by: Angelo Dureghello <angelo@sysam.it>
---
 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 19c000722cbc..88cd10a0ee9e 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1005,6 +1005,7 @@ static const struct flash_info spi_nor_ids[] = {
 
 	/* ISSI */
 	{ "is25cd512", INFO(0x7f9d20, 0, 32 * 1024,   2, SECT_4K) },
+	{ "is25lp128", INFO(0x9d6018, 0, 64 * 1024,   256, SECT_4K) },
 
 	/* Macronix */
 	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
-- 
2.15.0

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

[Cyrille Pitchen]

Hi Angelo,

Le 16/11/2017 à 20:09, Angelo Dureghello a écrit :
> Add support for ISSI is25lp128 spi nor flash.
> 
> Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> ---
>  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 19c000722cbc..88cd10a0ee9e 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1005,6 +1005,7 @@ static const struct flash_info spi_nor_ids[] = {
>  
>  	/* ISSI */
>  	{ "is25cd512", INFO(0x7f9d20, 0, 32 * 1024,   2, SECT_4K) },
> +	{ "is25lp128", INFO(0x9d6018, 0, 64 * 1024,   256, SECT_4K) },

I've checked both datasheets for is25cd512 and is25lp128 because I was
surprised to see different manufacturer IDs in the JEDEC IDs: I suspected
some mistake in one of the two entries above.

Then I found this old discussion on patchwork:
https://patchwork.ozlabs.org/patch/581017/

I looked closer at the is25cd512 datasheet and actually I confirmed that it
is stated that the manufacturer ID is received as the 2nd data byte after
the 9Fh op code. So both entries are correct and this is not a bug of the
is25cd512 memory part: it is documented so it is a feature, right?

Anyway, I plan to add another patch on top of yours just to add a comment
otherwise it might not be the last time that someone is puzzled with this
quirk.

Then back to your patch, is25lp128's datasheet states that this memory
parts support the Fast Read 1-1-2 (Dual Output), the Fast Read 1-4-4 (Quad
I/O) but strangely not the Fast Read 1-1-4 (Quad Output). Also I see that
the Read SFDP (5Ah) op code is supported too.
So could you please add the SPI_NOR_DUAL_READ flag but _not_ the
SPI_NOR_QUAD_READ flag in your new entry for is25lp128 ?

Indeed, currently at least one of the two SPI_NOR_{DUAL|QUAD}_READ flags is
needed to enabled the parsing of SFDP tables and I would like this parsing
to be enabled for this memory part since its datasheet claims that SFDP is
supported. Right now, this is the only mean to allow using SPI 1-4-4 with
SPI controllers that support this SPI protocol too.

Best regards,

Cyrille

>  
>  	/* Macronix */
>  	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
> 

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

[Angelo Dureghello]

Hi Cyrille,

On Fri, Nov 17, 2017 at 06:10:05PM +0100, Cyrille Pitchen wrote:
> Hi Angelo,
> 
> Le 16/11/2017 à 20:09, Angelo Dureghello a écrit :
> > Add support for ISSI is25lp128 spi nor flash.
> > 
> > Signed-off-by: Angelo Dureghello <angelo@sysam.it>
> > ---
> >  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 19c000722cbc..88cd10a0ee9e 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -1005,6 +1005,7 @@ static const struct flash_info spi_nor_ids[] = {
> >  
> >  	/* ISSI */
> >  	{ "is25cd512", INFO(0x7f9d20, 0, 32 * 1024,   2, SECT_4K) },
> > +	{ "is25lp128", INFO(0x9d6018, 0, 64 * 1024,   256, SECT_4K) },
> 
> I've checked both datasheets for is25cd512 and is25lp128 because I was
> surprised to see different manufacturer IDs in the JEDEC IDs: I suspected
> some mistake in one of the two entries above.
> 
> Then I found this old discussion on patchwork:
> https://patchwork.ozlabs.org/patch/581017/
> 
> I looked closer at the is25cd512 datasheet and actually I confirmed that it
> is stated that the manufacturer ID is received as the 2nd data byte after
> the 9Fh op code. So both entries are correct and this is not a bug of the
> is25cd512 memory part: it is documented so it is a feature, right?
> 

Yes. I believe the reason for this is that is25cd512 was originally a
PMC-Sierra device so a different silicon design approach from ISSI.
ISSI bought PMC so i am supposing ISSI maintained the jedec id "way"
as it was for convenience and for back-compatibility.

> Anyway, I plan to add another patch on top of yours just to add a comment
> otherwise it might not be the last time that someone is puzzled with this
> quirk.
>
Sure, fine.
 
> Then back to your patch, is25lp128's datasheet states that this memory
> parts support the Fast Read 1-1-2 (Dual Output), the Fast Read 1-4-4 (Quad
> I/O) but strangely not the Fast Read 1-1-4 (Quad Output). Also I see that
> the Read SFDP (5Ah) op code is supported too.
> So could you please add the SPI_NOR_DUAL_READ flag but _not_ the
> SPI_NOR_QUAD_READ flag in your new entry for is25lp128 ?
> 
> Indeed, currently at least one of the two SPI_NOR_{DUAL|QUAD}_READ flags is
> needed to enabled the parsing of SFDP tables and I would like this parsing
> to be enabled for this memory part since its datasheet claims that SFDP is
> supported. Right now, this is the only mean to allow using SPI 1-4-4 with
> SPI controllers that support this SPI protocol too.
> 

Sure. Will send in short a v3 patch with SPI_NOR_DUAL_READ.
I cannot test quad mode right now, since my hardware here (the
mainlined arch/m68k/coldfire/stmark2 board) is wired as simple spi.
But i believe the flag cannot harm in anyway until someone can test it.

> Best regards,
> 
> Cyrille
> 

Thanks,
Best regards,
Angelo Dureghello


> >  
> >  	/* Macronix */
> >  	{ "mx25l512e",   INFO(0xc22010, 0, 64 * 1024,   1, SECT_4K) },
> > 
>