2022-05-23 08:05:30

by Tom Fitzhenry

[permalink] [raw]
Subject: [PATCH] mtd: spi-nor: gigadevice: add support for gd25lq128e

The GD25LQ128EWIGR[0] reports an JEDEC id with a different vendor,
otherwise it seems to fit neatly in the gigadevice list.

Tested to work on the Pine64 PinePhone Pro[1].

Attribution: initial version was written by
Martijn Braam <[email protected]> for the Pine64 vendor kernel[2].

Also in use in vendor u-boot builds[3].

0. https://www.gigadevice.com/flash-memory/gd25lq128e/
1. https://wiki.pine64.org/wiki/PinePhone_Pro
2. https://gitlab.com/pine64-org/linux/-/merge_requests/29
3. https://github.com/Tow-Boot/Tow-Boot/blob/b94838dfb8971cdeb841d3922051aaa8e108b085/boards/pine64-pinephonePro/0001-mtd-spi-nor-ids-Add-GigaDevice-GD25LQ128E-entry.patch

Signed-off-by: Tom Fitzhenry <[email protected]>
---
drivers/mtd/spi-nor/gigadevice.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
index 33895002eeea..871c9dee11dc 100644
--- a/drivers/mtd/spi-nor/gigadevice.c
+++ b/drivers/mtd/spi-nor/gigadevice.c
@@ -56,6 +56,10 @@ static const struct flash_info gigadevice_nor_parts[] = {
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ) },
+ { "gd25lq128e", INFO(0x257018, 0, 64 * 1024, 256)
+ FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+ NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
+ SPI_NOR_QUAD_READ) },
{ "gd25q128", INFO(0xc84018, 0, 64 * 1024, 256)
FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
--
2.36.0



2022-05-23 08:39:57

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] mtd: spi-nor: gigadevice: add support for gd25lq128e

Am 2022-05-23 07:55, schrieb Tom Fitzhenry:
> The GD25LQ128EWIGR[0] reports an JEDEC id with a different vendor,
> otherwise it seems to fit neatly in the gigadevice list.

According to JEP106BC the vendor code 0x25 is Tristar. I'm
not sure what is going on here.

> Tested to work on the Pine64 PinePhone Pro[1].
>
> Attribution: initial version was written by
> Martijn Braam <[email protected]> for the Pine64 vendor kernel[2].
>
> Also in use in vendor u-boot builds[3].
>
> 0. https://www.gigadevice.com/flash-memory/gd25lq128e/
> 1. https://wiki.pine64.org/wiki/PinePhone_Pro
> 2. https://gitlab.com/pine64-org/linux/-/merge_requests/29
> 3.
> https://github.com/Tow-Boot/Tow-Boot/blob/b94838dfb8971cdeb841d3922051aaa8e108b085/boards/pine64-pinephonePro/0001-mtd-spi-nor-ids-Add-GigaDevice-GD25LQ128E-entry.patch

Do we need this? If you like you can provide a datasheet
with the Link: tag at the end of the commit.

This flash supports SFDP, please provide an SFDP dump, see [1].

> Signed-off-by: Tom Fitzhenry <[email protected]>
> ---
> drivers/mtd/spi-nor/gigadevice.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/gigadevice.c
> b/drivers/mtd/spi-nor/gigadevice.c
> index 33895002eeea..871c9dee11dc 100644
> --- a/drivers/mtd/spi-nor/gigadevice.c
> +++ b/drivers/mtd/spi-nor/gigadevice.c
> @@ -56,6 +56,10 @@ static const struct flash_info
> gigadevice_nor_parts[] = {
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ) },
> + { "gd25lq128e", INFO(0x257018, 0, 64 * 1024, 256)
> + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)

Did you test locking?

> + NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
> + SPI_NOR_QUAD_READ) },

As this flash supports SFDP, please use SNOR_ID3(0x257018)
and drop both the INFO() and the NO_SFDP_FLAGS(). You'll
need my SNOR_ID3() patches [2].

> { "gd25q128", INFO(0xc84018, 0, 64 * 1024, 256)
> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |

-michael

[1]
https://lore.kernel.org/linux-mtd/[email protected]/
[2]
https://lore.kernel.org/linux-mtd/[email protected]/

2022-05-24 23:45:44

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] mtd: spi-nor: gigadevice: add support for gd25lq128e

Am 2022-05-24 22:50, schrieb Tom Fitzhenry:
> On 23/5/22 18:03, Michael Walle wrote:
>> According to JEP106BC the vendor code 0x25 is Tristar. I'm
>> not sure what is going on here.
>
> Nor I! The board schematic[0] reports the use of GD25LQ128EWIGR
> (vendor Gigadevices), but indeed the chip itself reports vendor
> Tristar.
>
> I will ask the community/vendor about this discrepancy.

Yes that would be great. Could also be one of the other
25h vendors, usually the continuation code is just ignored.
I'd bet it some china SPI flash.

I don't think it is a Gigadevice because the datasheet says its ID
is c86018.

>> This flash supports SFDP, please provide an SFDP dump, see [1].
>
> I will include this in my v2 patch. For posterity, here's the dump:
>
> $ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450060101ff00060110300000ff9d05010380000002ffffffffffff
> ffffffffffffffffffffffffffffffffffffe520f9ffffffff0744eb086b
> 083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
> 11c7cccd68467a757a75f7a2d55c4a422cfff030c080ffffffffffffffff
> ffffffffffffffff501950169cf9c0648fecffff
> $ md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> de4d6be54e479d60859b0ca8a0ee9216
> /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 257018
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> gd25lq128e
> $ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> gigadevice

Thanks.

> I have attached the parsed sfdp, according to
> https://github.com/petris/sfdp-parser
>
>> Did you test locking?
>
> No. The datasheet mentions Status Register locking, but I will look
> into how to adequately test this.

Or just drop the locking flags for now if you like.

>> As this flash supports SFDP, please use SNOR_ID3(0x257018)
>> and drop both the INFO() and the NO_SFDP_FLAGS(). You'll
>> need my SNOR_ID3() patches [2].
>
> SGTM, will do.

If you don't find the vendor and don't need locking, there is
also a generic SFDP flash driver [1]. You could give it a try
and add a Tested-by there.

-michael

[1]
https://lore.kernel.org/linux-mtd/[email protected]/

2022-05-25 19:52:08

by Tom Fitzhenry

[permalink] [raw]
Subject: Re: [PATCH] mtd: spi-nor: gigadevice: add support for gd25lq128e

On 23/5/22 18:03, Michael Walle wrote:
> According to JEP106BC the vendor code 0x25 is Tristar. I'm
> not sure what is going on here.

Nor I! The board schematic[0] reports the use of GD25LQ128EWIGR (vendor
Gigadevices), but indeed the chip itself reports vendor Tristar.

I will ask the community/vendor about this discrepancy.

> This flash supports SFDP, please provide an SFDP dump, see [1].

I will include this in my v2 patch. For posterity, here's the dump:

$ xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff9d05010380000002ffffffffffff
ffffffffffffffffffffffffffffffffffffe520f9ffffffff0744eb086b
083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
11c7cccd68467a757a75f7a2d55c4a422cfff030c080ffffffffffffffff
ffffffffffffffff501950169cf9c0648fecffff
$ md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
de4d6be54e479d60859b0ca8a0ee9216 /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
$ cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
257018
$ cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
gd25lq128e
$ cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
gigadevice

I have attached the parsed sfdp, according to
https://github.com/petris/sfdp-parser

> Did you test locking?

No. The datasheet mentions Status Register locking, but I will look into
how to adequately test this.

> As this flash supports SFDP, please use SNOR_ID3(0x257018)
> and drop both the INFO() and the NO_SFDP_FLAGS(). You'll
> need my SNOR_ID3() patches [2].

SGTM, will do.

Thanks for your feedback!


0.
https://files.pine64.org/doc/PinePhonePro/PinephonePro-Schematic-V1.0-20211127.pdf
page 11, "GD25LQ128EWIGR"


Attachments:
sfdp_parsed.txt (4.46 kB)

2022-05-29 03:21:44

by Tom Fitzhenry

[permalink] [raw]
Subject: ...

On 25/5/22 08:30, Michael Walle wrote:
> Yes that would be great. Could also be one of the other
> 25h vendors, usually the continuation code is just ignored.
> I'd bet it some china SPI flash.

Physical inspection of the SPI NOR chip shows the chip is a
"SiliconKaiser SK25LP128" on it. Thanks to biktorgj[0] for finding that.

Per
https://github.com/Yatekii/jep106/commit/caa92b6f811609d49b46777fce0d1f84a3550c31,
SiliconKaiser is a vendor rename from Chingis introduced in JEP106BD, at
0x25 in bank/continuation 8.

Unfortunately I have been unable to find a datasheet for this chip.

I will add a siliconkaiser.c file with this chip "sk25lp128" using the
SNOR_ID3 API, and submit a patch once I have tested this.

It's confusing that the chip's returned JEDEC ID has no continuation
codes to reflect being in bank 8. Does this risk collisions if another
25h vendor uses the same JEDEC ID?

0. https://github.com/Biktorgj