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

[Michael Walle <michael@walle.cc>]

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/