[PATCH v4 2/9] mtd: spi-nor-ids: Add Cypress s25hl-t/s25hs-t

[Pratyush Yadav]

On 28/01/21 01:36PM, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> 
> The S25HL-T/S25HS-T family is the Cypress Semper Flash with Quad SPI.
> The datasheets can be found in the following links.
> 
> https://www.cypress.com/file/424146/download (256Mb/512Mb/1Gb, single die)
> https://www.cypress.com/file/499246/download (2Gb/4Gb, dual/quad die)
> 
> Tested 512Mb/1Gb/2Gb parts on Xilinx Zynq-7000 FPGA board.

Why not test the 256 Mb and 4 Gb parts as well? They might work 
perfectly well but adding support for untested hardware sounds wrong to 
me.
 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
>  drivers/mtd/spi/spi-nor-ids.c | 36 +++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 5bd5dd3003..b78d13e980 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -217,6 +217,42 @@ const struct flash_info spi_nor_ids[] = {
>  	{ INFO("s25fl208k",  0x014014,      0,  64 * 1024,  16, SECT_4K | SPI_NOR_DUAL_READ) },
>  	{ INFO("s25fl064l",  0x016017,      0,  64 * 1024, 128, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  	{ INFO("s25fl128l",  0x016018,      0,  64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> +
> +	/* S25HL/HS-T (Semper Flash with Quad SPI) Family has overlaid 4KB

Nitpick: Please leave first line of multi-line comments blank like so:

/*
 * S25HL/HS-T (Semper Flash with Quad SPI) Family has overlaid 4KB
 * ...
 */

> +	 * sectors at top and/or bottom, depending on the device configuration.
> +	 * To support this, an erase hook makes overlaid sectors appear as
> +	 * uniform sectors.
> +	 */
> +	{ INFO6("s25hl256t",  0x342a19, 0x0f0390, 256 * 1024, 128,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hl512t",  0x342a1a, 0x0f0390, 256 * 1024, 256,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hl01gt",  0x342a1b, 0x0f0390, 256 * 1024, 512,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hl02gt",  0x342a1c, 0x0f0090, 256 * 1024, 1024,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hl04gt",  0x342a1d, 0x0f0090, 256 * 1024, 2048,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hs256t",  0x342b19, 0x0f0390, 256 * 1024, 128,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hs512t",  0x342b1a, 0x0f0390, 256 * 1024, 256,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hs01gt",  0x342b1b, 0x0f0390, 256 * 1024, 512,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hs02gt",  0x342b1c, 0x0f0090, 256 * 1024, 1024,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },
> +	{ INFO6("s25hs04gt",  0x342b1d, 0x0f0090, 256 * 1024, 2048,
> +		SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES |
> +		USE_CLSR) },

The datasheets you have linked do not specify a mapping between device 
ID and part number. So I can't verify that you are using the correct IDs 
here. But I'll trust you to get them right :-)

With the above comment addressed and 256 Mb and 4 Gb parts tested, 
please add

Reviewed-by: Pratyush Yadav <p.yadav at ti.com>

>  #endif
>  #ifdef CONFIG_SPI_FLASH_SST		/* SST */
>  	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
> -- 
> 2.25.1
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.