[PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and
differences comparing to other Spansion/Cypress flash familes are:
  - 4-byte address mode by factory default
  - Quad mode is enabled by factory default
  - Supports mixture of 128KB and 64KB sectors by OTP configuration
    (this patch supports uniform 128KB only due to complexity of
     non-uniform layout)

Tested on Xilinx Zynq-7000 FPGA board.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
Datasheet:
https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?fileId=8ac78c8c80027ecd0180740c5a46707a

zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25fs256t
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342b190f0890
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080101ff00000114000100ff84000102500100ffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff
ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1
ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000
0000ffffffff710600fedcdcffff
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
13ecce2f195c4c71648e90d4a7e4a0df  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
zynq> test_qspi.sh
random: crng init done
6+0 records in
6+0 records out
6291456 bytes (6.0MB) copied, 2.787435 seconds, 2.2MB/s
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Erased 6291456 bytes from address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_test
2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_read
---
 drivers/mtd/spi-nor/spansion.c | 64 ++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index b621cdfd506f..7d1d2c27ad71 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -24,6 +24,7 @@
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
 #define SPINOR_OP_CYPRESS_RD_FAST		0xee
+#define SPINOR_REG_CYPRESS_ARCFN		0x00000006
 
 /* Cypress SPI NOR flash operations. */
 #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
@@ -213,6 +214,66 @@ static int cypress_nor_set_page_size(struct spi_nor *nor)
 	return 0;
 }
 
+static int
+s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
+			  const struct sfdp_parameter_header *bfpt_header,
+			  const struct sfdp_bfpt *bfpt)
+{
+	struct spi_mem_op op;
+	int ret;
+
+	/* 4-byte address mode is enabled by default */
+	nor->params->addr_mode_nbytes = 4;
+
+	/* Read Architecture Configuration Register (ARCFN) */
+	op = (struct spi_mem_op)
+		CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
+					  SPINOR_REG_CYPRESS_ARCFN,
+					  nor->bouncebuf);
+	op.dummy.nbytes = 1;
+	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+	if (ret)
+		return ret;
+
+	/* ARCFN value must be 0 if uniform sector is selected  */
+	if (nor->bouncebuf[0])
+		return -EOPNOTSUPP;
+
+	return cypress_nor_set_page_size(nor);
+}
+
+static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
+{
+	struct spi_nor_flash_parameter *params = nor->params;
+
+	/*
+	 * READ_FAST is omitted in 4BAIT parse since OP_READ_FAST_4B(0Ch) is not
+	 * supported. Enable OP_READ_FAST(0Bh) that can work in 4-byte address
+	 * mode.
+	 */
+	params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST], 0, 8,
+				  SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1);
+
+	/* PP_1_1_4_4B is supported but missing in SFDP. */
+	params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
+	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+				SPINOR_OP_PP_1_1_4_4B,
+				SNOR_PROTO_1_1_4);
+}
+
+static void s25fs256t_late_init(struct spi_nor *nor)
+{
+	/* The writesize should be ECC data unit size */
+	nor->params->writesize = 16;
+}
+
+static struct spi_nor_fixups s25fs256t_fixups = {
+	.post_bfpt = s25fs256t_post_bfpt_fixup,
+	.post_sfdp = s25fs256t_post_sfdp_fixup,
+	.late_init = s25fs256t_late_init,
+};
+
 static int
 s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
 			const struct sfdp_parameter_header *bfpt_header,
@@ -441,6 +502,9 @@ static const struct flash_info spansion_nor_parts[] = {
 	{ "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
+	{ "s25fs256t",  INFO6(0x342b19, 0x0f0890, 128 * 1024, 256)
+		PARSE_SFDP
+		.fixups = &s25fs256t_fixups },
 	{ "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
 		PARSE_SFDP
 		MFR_FLAGS(USE_CLSR)
-- 
2.25.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Michael Walle]

Hi,

Am 2022-12-19 02:55, schrieb tkuw584924@gmail.com:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and
> differences comparing to other Spansion/Cypress flash familes are:
>   - 4-byte address mode by factory default
>   - Quad mode is enabled by factory default
>   - Supports mixture of 128KB and 64KB sectors by OTP configuration
>     (this patch supports uniform 128KB only due to complexity of
>      non-uniform layout)
> 
> Tested on Xilinx Zynq-7000 FPGA board.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
> Datasheet:
> fileId=8ac78c8c80027ecd0180740c5a46707https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?a
> 
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
> s25fs256t
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
> 342b190f0890
> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
> spansion
> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 53464450080101ff00000114000100ff84000102500100ffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff
> ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1
> ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000
> 0000ffffffff710600fedcdcffff
> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> 13ecce2f195c4c71648e90d4a7e4a0df  
> /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
> zynq> test_qspi.sh
> random: crng init done
> 6+0 records in
> 6+0 records out
> 6291456 bytes (6.0MB) copied, 2.787435 seconds, 2.2MB/s
> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
> Erased 6291456 bytes from address 0x00000000 in flash
> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 0600000
> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
> 2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_test
> 2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_read
> ---
>  drivers/mtd/spi-nor/spansion.c | 64 ++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c 
> b/drivers/mtd/spi-nor/spansion.c
> index b621cdfd506f..7d1d2c27ad71 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -24,6 +24,7 @@
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
>  #define SPINOR_OP_CYPRESS_RD_FAST		0xee
> +#define SPINOR_REG_CYPRESS_ARCFN		0x00000006
> 
>  /* Cypress SPI NOR flash operations. */
>  #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
> @@ -213,6 +214,66 @@ static int cypress_nor_set_page_size(struct 
> spi_nor *nor)
>  	return 0;
>  }
> 
> +static int
> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
> +			  const struct sfdp_parameter_header *bfpt_header,
> +			  const struct sfdp_bfpt *bfpt)
> +{
> +	struct spi_mem_op op;
> +	int ret;
> +
> +	/* 4-byte address mode is enabled by default */
> +	nor->params->addr_mode_nbytes = 4;

Shouldn't this already be set in spi_nor_parse_4bait()?

> +
> +	/* Read Architecture Configuration Register (ARCFN) */
> +	op = (struct spi_mem_op)
> +		CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
> +					  SPINOR_REG_CYPRESS_ARCFN,
> +					  nor->bouncebuf);
> +	op.dummy.nbytes = 1;
> +	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> +	if (ret)
> +		return ret;
> +
> +	/* ARCFN value must be 0 if uniform sector is selected  */
> +	if (nor->bouncebuf[0])
> +		return -EOPNOTSUPP;

EOPNOTSUPP is wrong here. I'd say ENODEV.

> +
> +	return cypress_nor_set_page_size(nor);
> +}
> +
> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
> +{
> +	struct spi_nor_flash_parameter *params = nor->params;
> +
> +	/*
> +	 * READ_FAST is omitted in 4BAIT parse since OP_READ_FAST_4B(0Ch) is 
> not
> +	 * supported. Enable OP_READ_FAST(0Bh) that can work in 4-byte 
> address
> +	 * mode.
> +	 */
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST], 0, 8,
> +				  SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1);

Mh, this requires mode switching, the advantage of the opcodes in
the 4bait table are that they don't require mode switching.
OP_READ_FAST doesn't work here if the address mode is set to 3. (I
know this flash defaults to 1, but there is also a non-volatile
setting for this).

Regarding mode switching, I guess this is wrong for this flash, because
it is set to spansion_set_4byte_addr_mode() by default while it should
really be set to spi_nor_set_4byte_addr_mode().

Also I'm not sure when set_4byte_addr_mode() is called during init.
It seems slightly wrong to me because it will check wether
SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
!SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.

> +
> +	/* PP_1_1_4_4B is supported but missing in SFDP. */
> +	params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> +				SPINOR_OP_PP_1_1_4_4B,
> +				SNOR_PROTO_1_1_4);
> +}
> +
> +static void s25fs256t_late_init(struct spi_nor *nor)
> +{
> +	/* The writesize should be ECC data unit size */
> +	nor->params->writesize = 16;

The datasheets mentions, a PP should be either 128 or 256 (for best
performance). So why 16?

> +}
> +
> +static struct spi_nor_fixups s25fs256t_fixups = {
> +	.post_bfpt = s25fs256t_post_bfpt_fixup,
> +	.post_sfdp = s25fs256t_post_sfdp_fixup,
> +	.late_init = s25fs256t_late_init,
> +};
> +
>  static int
>  s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
>  			const struct sfdp_parameter_header *bfpt_header,
> @@ -441,6 +502,9 @@ static const struct flash_info spansion_nor_parts[] 
> = {
>  	{ "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> +	{ "s25fs256t",  INFO6(0x342b19, 0x0f0890, 128 * 1024, 256)

Does INFO6(0x342b19, 0x0f0890, 0, 0) work for you?

> +		PARSE_SFDP
> +		.fixups = &s25fs256t_fixups },
>  	{ "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>  		PARSE_SFDP
>  		MFR_FLAGS(USE_CLSR)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Michael Walle]

Hi,

Am 2022-12-19 08:29, schrieb Michael Walle:
> Am 2022-12-19 02:55, schrieb tkuw584924@gmail.com:
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> 
>> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and
>> differences comparing to other Spansion/Cypress flash familes are:
>>   - 4-byte address mode by factory default
>>   - Quad mode is enabled by factory default
>>   - Supports mixture of 128KB and 64KB sectors by OTP configuration
>>     (this patch supports uniform 128KB only due to complexity of
>>      non-uniform layout)
>> 
>> Tested on Xilinx Zynq-7000 FPGA board.
>> 
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> ---
>> Datasheet:
>> fileId=8ac78c8c80027ecd0180740c5a46707https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?a

This should be a Link: tag in the commit message.

> Also I'm not sure when set_4byte_addr_mode() is called during init.
> It seems slightly wrong to me because it will check wether
> SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
> !SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.

Scrap that comment. I read the code wrong. The place I looked at
just translate the 3B opcodes to the 4B ones.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Takahiro Kuwano]

On 12/19/2022 4:29 PM, Michael Walle wrote:
> Hi,
> 
Hi Michael,

> Am 2022-12-19 02:55, schrieb tkuw584924@gmail.com:
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and
>> differences comparing to other Spansion/Cypress flash familes are:
>>   - 4-byte address mode by factory default
>>   - Quad mode is enabled by factory default
>>   - Supports mixture of 128KB and 64KB sectors by OTP configuration
>>     (this patch supports uniform 128KB only due to complexity of
>>      non-uniform layout)
>>
>> Tested on Xilinx Zynq-7000 FPGA board.
>>
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>> ---
>> Datasheet:
>> fileId=8ac78c8c80027ecd0180740c5a46707https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?a
>>
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
>> s25fs256t
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
>> 342b190f0890
>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
>> spansion
>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 53464450080101ff00000114000100ff84000102500100ffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
>> ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff
>> ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1
>> ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000
>> 0000ffffffff710600fedcdcffff
>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> 13ecce2f195c4c71648e90d4a7e4a0df  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
>> zynq> test_qspi.sh
>> random: crng init done
>> 6+0 records in
>> 6+0 records out
>> 6291456 bytes (6.0MB) copied, 2.787435 seconds, 2.2MB/s
>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
>> Erased 6291456 bytes from address 0x00000000 in flash
>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
>> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
>> *
>> 0600000
>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
>> 2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_test
>> 2c6e12c0a2346cab755ef72cdf2a72f827241d35  qspi_read
>> ---
>>  drivers/mtd/spi-nor/spansion.c | 64 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>> index b621cdfd506f..7d1d2c27ad71 100644
>> --- a/drivers/mtd/spi-nor/spansion.c
>> +++ b/drivers/mtd/spi-nor/spansion.c
>> @@ -24,6 +24,7 @@
>>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN    0x3
>>  #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS    0
>>  #define SPINOR_OP_CYPRESS_RD_FAST        0xee
>> +#define SPINOR_REG_CYPRESS_ARCFN        0x00000006
>>
>>  /* Cypress SPI NOR flash operations. */
>>  #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)        \
>> @@ -213,6 +214,66 @@ static int cypress_nor_set_page_size(struct spi_nor *nor)
>>      return 0;
>>  }
>>
>> +static int
>> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
>> +              const struct sfdp_parameter_header *bfpt_header,
>> +              const struct sfdp_bfpt *bfpt)
>> +{
>> +    struct spi_mem_op op;
>> +    int ret;
>> +
>> +    /* 4-byte address mode is enabled by default */
>> +    nor->params->addr_mode_nbytes = 4;
> 
> Shouldn't this already be set in spi_nor_parse_4bait()?
> 
No, params->addr_mode_nbytes is initialized in spi_nor_parse_bfpt().
In spi_nor_parse_4bait(), params->addr_nbytes is set to 4.

>> +
>> +    /* Read Architecture Configuration Register (ARCFN) */
>> +    op = (struct spi_mem_op)
>> +        CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
>> +                      SPINOR_REG_CYPRESS_ARCFN,
>> +                      nor->bouncebuf);
>> +    op.dummy.nbytes = 1;
>> +    ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /* ARCFN value must be 0 if uniform sector is selected  */
>> +    if (nor->bouncebuf[0])
>> +        return -EOPNOTSUPP;
> 
> EOPNOTSUPP is wrong here. I'd say ENODEV.
> 
OK.

>> +
>> +    return cypress_nor_set_page_size(nor);
>> +}
>> +
>> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
>> +{
>> +    struct spi_nor_flash_parameter *params = nor->params;
>> +
>> +    /*
>> +     * READ_FAST is omitted in 4BAIT parse since OP_READ_FAST_4B(0Ch) is not
>> +     * supported. Enable OP_READ_FAST(0Bh) that can work in 4-byte address
>> +     * mode.
>> +     */
>> +    params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
>> +    spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST], 0, 8,
>> +                  SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1);
> 
> Mh, this requires mode switching, the advantage of the opcodes in
> the 4bait table are that they don't require mode switching.
> OP_READ_FAST doesn't work here if the address mode is set to 3. (I
> know this flash defaults to 1, but there is also a non-volatile
> setting for this).
> 
> Regarding mode switching, I guess this is wrong for this flash, because
> it is set to spansion_set_4byte_addr_mode() by default while it should
> really be set to spi_nor_set_4byte_addr_mode().
> 
Let me just drop fast read support so far and mention this in commit
description.

> Also I'm not sure when set_4byte_addr_mode() is called during init.
> It seems slightly wrong to me because it will check wether
> SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
> !SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.
> 
>> +
>> +    /* PP_1_1_4_4B is supported but missing in SFDP. */
>> +    params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
>> +    spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>> +                SPINOR_OP_PP_1_1_4_4B,
>> +                SNOR_PROTO_1_1_4);
>> +}
>> +
>> +static void s25fs256t_late_init(struct spi_nor *nor)
>> +{
>> +    /* The writesize should be ECC data unit size */
>> +    nor->params->writesize = 16;
> 
> The datasheets mentions, a PP should be either 128 or 256 (for best
> performance). So why 16?
> 
writesize is used as minimum IO size in UBI.
Please see:
https://lore.kernel.org/linux-mtd/20201118182459.18197-1-p.yadav@ti.com/

>> +}
>> +
>> +static struct spi_nor_fixups s25fs256t_fixups = {
>> +    .post_bfpt = s25fs256t_post_bfpt_fixup,
>> +    .post_sfdp = s25fs256t_post_sfdp_fixup,
>> +    .late_init = s25fs256t_late_init,
>> +};
>> +
>>  static int
>>  s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
>>              const struct sfdp_parameter_header *bfpt_header,
>> @@ -441,6 +502,9 @@ static const struct flash_info spansion_nor_parts[] = {
>>      { "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>>          NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>          FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> +    { "s25fs256t",  INFO6(0x342b19, 0x0f0890, 128 * 1024, 256)
> 
> Does INFO6(0x342b19, 0x0f0890, 0, 0) work for you?
> 
Yes, it works.

>> +        PARSE_SFDP
>> +        .fixups = &s25fs256t_fixups },
>>      { "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>>          PARSE_SFDP
>>          MFR_FLAGS(USE_CLSR)

Thanks,
Takahiro

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Takahiro Kuwano]

On 12/19/2022 5:27 PM, Michael Walle wrote:
> Hi,
> 
> Am 2022-12-19 08:29, schrieb Michael Walle:
>> Am 2022-12-19 02:55, schrieb tkuw584924@gmail.com:
>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>
>>> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and
>>> differences comparing to other Spansion/Cypress flash familes are:
>>>   - 4-byte address mode by factory default
>>>   - Quad mode is enabled by factory default
>>>   - Supports mixture of 128KB and 64KB sectors by OTP configuration
>>>     (this patch supports uniform 128KB only due to complexity of
>>>      non-uniform layout)
>>>
>>> Tested on Xilinx Zynq-7000 FPGA board.
>>>
>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>> ---
>>> Datasheet:
>>> fileId=8ac78c8c80027ecd0180740c5a46707https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?a
> 
> This should be a Link: tag in the commit message.
> 
Thank you!

>> Also I'm not sure when set_4byte_addr_mode() is called during init.
>> It seems slightly wrong to me because it will check wether
>> SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
>> !SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.
> 
> Scrap that comment. I read the code wrong. The place I looked at
> just translate the 3B opcodes to the 4B ones.
> 
> -michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Michael Walle]

Am 2022-12-20 09:31, schrieb Takahiro Kuwano:


>>> +static int
>>> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
>>> +              const struct sfdp_parameter_header *bfpt_header,
>>> +              const struct sfdp_bfpt *bfpt)
>>> +{
>>> +    struct spi_mem_op op;
>>> +    int ret;
>>> +
>>> +    /* 4-byte address mode is enabled by default */
>>> +    nor->params->addr_mode_nbytes = 4;
>> 
>> Shouldn't this already be set in spi_nor_parse_4bait()?
>> 
> No, params->addr_mode_nbytes is initialized in spi_nor_parse_bfpt().
> In spi_nor_parse_4bait(), params->addr_nbytes is set to 4.

Ah right, my bad. In any case, shouldn't this read the actual
mode from the flash given that this is a non-volatile setting?

>>> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
>>> +{
>>> +    struct spi_nor_flash_parameter *params = nor->params;
>>> +
>>> +    /*
>>> +     * READ_FAST is omitted in 4BAIT parse since 
>>> OP_READ_FAST_4B(0Ch) is not
>>> +     * supported. Enable OP_READ_FAST(0Bh) that can work in 4-byte 
>>> address
>>> +     * mode.
>>> +     */
>>> +    params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
>>> +    spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST], 0, 
>>> 8,
>>> +                  SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1);
>> 
>> Mh, this requires mode switching, the advantage of the opcodes in
>> the 4bait table are that they don't require mode switching.
>> OP_READ_FAST doesn't work here if the address mode is set to 3. (I
>> know this flash defaults to 1, but there is also a non-volatile
>> setting for this).
>> 
>> Regarding mode switching, I guess this is wrong for this flash, 
>> because
>> it is set to spansion_set_4byte_addr_mode() by default while it should
>> really be set to spi_nor_set_4byte_addr_mode().
>> 
> Let me just drop fast read support so far and mention this in commit
> description.

Then the flash will likely be impossible to be used in x1 mode, because
we don't support different frequencies for now. And the SPI frequency
will likely be too fast for the normal read command.
If you skip that, then please make sure, the flash doesn't probe at all.
I.e. returning ENODEV like above.

Maybe we can use the spi bus max_frequency as an indicator if you can
use the normal read opcode (4b variant of course). I've just looked
at some spi drivers and not all are using the speed_hz property of
an SPI transfer message, but just use the max_speed_hz of the SPI
device. Sigh.

>> Also I'm not sure when set_4byte_addr_mode() is called during init.
>> It seems slightly wrong to me because it will check wether
>> SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
>> !SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.
>> 
>>> +
>>> +    /* PP_1_1_4_4B is supported but missing in SFDP. */
>>> +    params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
>>> +    
>>> spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>>> +                SPINOR_OP_PP_1_1_4_4B,
>>> +                SNOR_PROTO_1_1_4);
>>> +}
>>> +
>>> +static void s25fs256t_late_init(struct spi_nor *nor)
>>> +{
>>> +    /* The writesize should be ECC data unit size */
>>> +    nor->params->writesize = 16;
>> 
>> The datasheets mentions, a PP should be either 128 or 256 (for best
>> performance). So why 16?
>> 
> writesize is used as minimum IO size in UBI.
> Please see:
> https://lore.kernel.org/linux-mtd/20201118182459.18197-1-p.yadav@ti.com/

Ok. maybe it would make sense to amend that comment that the program
granularity is 16bytes for this flash, otherwise ECC will be disabled
if the same page is programmed again without an erase. See ch. 5.3.1
in the datasheet.

>>> +}
>>> +
>>> +static struct spi_nor_fixups s25fs256t_fixups = {
>>> +    .post_bfpt = s25fs256t_post_bfpt_fixup,
>>> +    .post_sfdp = s25fs256t_post_sfdp_fixup,
>>> +    .late_init = s25fs256t_late_init,
>>> +};
>>> +
>>>  static int
>>>  s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
>>>              const struct sfdp_parameter_header *bfpt_header,
>>> @@ -441,6 +502,9 @@ static const struct flash_info 
>>> spansion_nor_parts[] = {
>>>      { "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>>>          NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | 
>>> SPI_NOR_QUAD_READ)
>>>          FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>>> +    { "s25fs256t",  INFO6(0x342b19, 0x0f0890, 128 * 1024, 256)
>> 
>> Does INFO6(0x342b19, 0x0f0890, 0, 0) work for you?
>> 
> Yes, it works.

Please use that one then. I'm planning to replace all the INFO()/INFO6()
in the future and drop the sizes because they will get fetched from
SFDP.

-michael

>>> +        PARSE_SFDP
>>> +        .fixups = &s25fs256t_fixups },
>>>      { "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>>>          PARSE_SFDP
>>>          MFR_FLAGS(USE_CLSR)
> 
> Thanks,
> Takahiro

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: spansion: Add support for Infineon S25FS256T

[Takahiro Kuwano]

On 12/20/2022 7:01 PM, Michael Walle wrote:
> Am 2022-12-20 09:31, schrieb Takahiro Kuwano:
> 
> 
>>>> +static int
>>>> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor,
>>>> +              const struct sfdp_parameter_header *bfpt_header,
>>>> +              const struct sfdp_bfpt *bfpt)
>>>> +{
>>>> +    struct spi_mem_op op;
>>>> +    int ret;
>>>> +
>>>> +    /* 4-byte address mode is enabled by default */
>>>> +    nor->params->addr_mode_nbytes = 4;
>>>
>>> Shouldn't this already be set in spi_nor_parse_4bait()?
>>>
>> No, params->addr_mode_nbytes is initialized in spi_nor_parse_bfpt().
>> In spi_nor_parse_4bait(), params->addr_nbytes is set to 4.
> 
> Ah right, my bad. In any case, shouldn't this read the actual
> mode from the flash given that this is a non-volatile setting?
> 
To read out current address mode, we need to issue Read Any Reg with correct
address length (chicken-and-egg).

Please refer to commit description in:
https://patchwork.ozlabs.org/project/linux-mtd/patch/20220725092505.446315-6-tudor.ambarus@microchip.com/

And discussion about address mode discovery:
https://patchwork.ozlabs.org/project/linux-mtd/patch/e47ed0aa3e1a6fdca7689434ce7dea99ff4826e7.1659764848.git.Takahiro.Kuwano@infineon.com/

Unfortunately, this device does not support CRC...
https://patchwork.ozlabs.org/project/linux-mtd/patch/53d37eead9bdd09744af555b93381af477643a46.1660291281.git.Takahiro.Kuwano@infineon.com/

Until we develop good solution, I would rely on factory default setting.

>>>> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor)
>>>> +{
>>>> +    struct spi_nor_flash_parameter *params = nor->params;
>>>> +
>>>> +    /*
>>>> +     * READ_FAST is omitted in 4BAIT parse since OP_READ_FAST_4B(0Ch) is not
>>>> +     * supported. Enable OP_READ_FAST(0Bh) that can work in 4-byte address
>>>> +     * mode.
>>>> +     */
>>>> +    params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
>>>> +    spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST], 0, 8,
>>>> +                  SPINOR_OP_READ_FAST, SNOR_PROTO_1_1_1);
>>>
>>> Mh, this requires mode switching, the advantage of the opcodes in
>>> the 4bait table are that they don't require mode switching.
>>> OP_READ_FAST doesn't work here if the address mode is set to 3. (I
>>> know this flash defaults to 1, but there is also a non-volatile
>>> setting for this).
>>>
>>> Regarding mode switching, I guess this is wrong for this flash, because
>>> it is set to spansion_set_4byte_addr_mode() by default while it should
>>> really be set to spi_nor_set_4byte_addr_mode().
>>>
>> Let me just drop fast read support so far and mention this in commit
>> description.
> 
> Then the flash will likely be impossible to be used in x1 mode, because
> we don't support different frequencies for now. And the SPI frequency
> will likely be too fast for the normal read command.
> If you skip that, then please make sure, the flash doesn't probe at all.
> I.e. returning ENODEV like above.
> 
> Maybe we can use the spi bus max_frequency as an indicator if you can
> use the normal read opcode (4b variant of course). I've just looked
> at some spi drivers and not all are using the speed_hz property of
> an SPI transfer message, but just use the max_speed_hz of the SPI
> device. Sigh.
> 
I would like to leave it to spi_nor_spimem_check_readop(). If controller is
running or can adapt frequency to normal read, it is possible to use in x1
mode. If not, spi_nor_spimem_check_readop() should return error and probe
will be failed.

>>> Also I'm not sure when set_4byte_addr_mode() is called during init.
>>> It seems slightly wrong to me because it will check wether
>>> SNOR_F_4B_OPCODES is set. But in the restore path, it is checked for
>>> !SNOR_F_4B_OPCODES before 3 byte mode is enabled again. Mhh.
>>>
>>>> +
>>>> +    /* PP_1_1_4_4B is supported but missing in SFDP. */
>>>> +    params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4;
>>>> +    spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>>>> +                SPINOR_OP_PP_1_1_4_4B,
>>>> +                SNOR_PROTO_1_1_4);
>>>> +}
>>>> +
>>>> +static void s25fs256t_late_init(struct spi_nor *nor)
>>>> +{
>>>> +    /* The writesize should be ECC data unit size */
>>>> +    nor->params->writesize = 16;
>>>
>>> The datasheets mentions, a PP should be either 128 or 256 (for best
>>> performance). So why 16?
>>>
>> writesize is used as minimum IO size in UBI.
>> Please see:
>> https://lore.kernel.org/linux-mtd/20201118182459.18197-1-p.yadav@ti.com/
> 
> Ok. maybe it would make sense to amend that comment that the program
> granularity is 16bytes for this flash, otherwise ECC will be disabled
> if the same page is programmed again without an erase. See ch. 5.3.1
> in the datasheet.
> 
OK.

>>>> +}
>>>> +
>>>> +static struct spi_nor_fixups s25fs256t_fixups = {
>>>> +    .post_bfpt = s25fs256t_post_bfpt_fixup,
>>>> +    .post_sfdp = s25fs256t_post_sfdp_fixup,
>>>> +    .late_init = s25fs256t_late_init,
>>>> +};
>>>> +
>>>>  static int
>>>>  s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
>>>>              const struct sfdp_parameter_header *bfpt_header,
>>>> @@ -441,6 +502,9 @@ static const struct flash_info spansion_nor_parts[] = {
>>>>      { "s25fl256l",  INFO(0x016019,      0,  64 * 1024, 512)
>>>>          NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>>>          FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>>>> +    { "s25fs256t",  INFO6(0x342b19, 0x0f0890, 128 * 1024, 256)
>>>
>>> Does INFO6(0x342b19, 0x0f0890, 0, 0) work for you?
>>>
>> Yes, it works.
> 
> Please use that one then. I'm planning to replace all the INFO()/INFO6()
> in the future and drop the sizes because they will get fetched from
> SFDP.
> 
OK.

> -michael
> 
>>>> +        PARSE_SFDP
>>>> +        .fixups = &s25fs256t_fixups },
>>>>      { "s25hl512t",  INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256)
>>>>          PARSE_SFDP
>>>>          MFR_FLAGS(USE_CLSR)
>>
>> Thanks,
>> Takahiro

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/