diff mbox series

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

Message ID 1cd1d3190617911faa9e96dfeedb28141476862e.1611729896.git.Takahiro.Kuwano@infineon.com
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series mtd: spi-nor: Add support for Cypress s25hl-t/s25hs-t | expand

Commit Message

Takahiro Kuwano Jan. 28, 2021, 4:36 a.m. UTC 
From: Takahiro Kuwano <Takahiro.Kuwano@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.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi/spi-nor-ids.c | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Pratyush Yadav Jan. 29, 2021, 6:08 p.m. UTC | #1 
On 28/01/21 01:36PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@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@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@ti.com>

>  #endif
>  #ifdef CONFIG_SPI_FLASH_SST		/* SST */
>  	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
> -- 
> 2.25.1
>
Takahiro Kuwano Feb. 9, 2021, 5:44 a.m. UTC | #2 
Hi Pratyush,

On 1/30/2021 3:08 AM, Pratyush Yadav wrote:
> On 28/01/21 01:36PM, tkuw584924@gmail.com wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano@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@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@ti.com>
> 
>>  #endif
>>  #ifdef CONFIG_SPI_FLASH_SST		/* SST */
>>  	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
>> -- 
>> 2.25.1
>>
> 

Best Regards,
Takahiro
diff mbox series

Patch

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
+	 * 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) },
 #endif
 #ifdef CONFIG_SPI_FLASH_SST		/* SST */
 	/* SST -- large erase sizes are "overlays", "sectors" are 4K */