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

[Michael Walle]

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 <martijn@xxxxxxxxx> 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 <tom@xxxxxxxxxxxxxxxxxxx>
> ---
>  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/4304e19f3399a0a6e856119d01ccabe0@xxxxxxxx/
[2] 
https://lore.kernel.org/linux-mtd/20220510140232.3519184-1-michael@xxxxxxxx/