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

[Takahiro Kuwano]

Hi Pratyush,

On 1/30/2021 3:08 AM, Pratyush Yadav wrote:
> 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.

The 256Mb and 4Gb parts are not yet officially sampled, although they are
described in the datasheet. They should work like other tested ones, but
as you commented, only tested devices should be added. I will remove 256Mb
and 4Gb in v5.

>  
>> 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
>  * ...
>  */
> 

I will fix it.

>> +	 * 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 :-)
> 

Thank you :-)

> 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
>>
> 

Best Regards,
Takahiro