[PATCH v2] mtd: spi-nor: spansion: Add support for Infineon S25FS256T
Takahiro Kuwano
tkuw584924 at gmail.com
Tue Mar 14 20:57:31 PDT 2023
Hi Tudor,
On 2/28/2023 5:04 PM, Tudor Ambarus wrote:
>
>
> On 26.02.2023 21:34, Tudor Ambarus wrote:
>> Hi, Takahiro,
>>
>> On 03.02.2023 06:48, tkuw584924 at gmail.com wrote:
>>> From: Takahiro Kuwano <Takahiro.Kuwano at 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
>>> - OP_READ_FAST_4B(0Ch) is not supported
>>> - 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.
>>>
>>> Link: https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?fileId=8ac78c8c80027ecd0180740c5a46707a
>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>> ---
>>> Changes in v2:
>>> - s/EOPNOTSUPP/ENODEV
>>> - Drop READ_FAST support
>>> - Amend comment in late_init()
>>> - sector size and page size as 0 in sizeINFO6()
>>>
>>> ---
>>> 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.025928 seconds, 3.0MB/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
>>> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_test
>>> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_read
>>> ---
>>> drivers/mtd/spi-nor/spansion.c | 60 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 60 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
>>> index 12a256c0ef4c..a499ce393454 100644
>>> --- a/drivers/mtd/spi-nor/spansion.c
>>> +++ b/drivers/mtd/spi-nor/spansion.c
>>> @@ -29,6 +29,7 @@
>>> SPINOR_REG_CYPRESS_CFR5_OPI)
>>> #define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_DS SPINOR_REG_CYPRESS_CFR5_BIT6
>>> #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) \
>>> @@ -218,6 +219,62 @@ 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;
>>
>> you'll need to set params->addr_nbytes = 4; as well, even if it is set
>> in parse_4bait(). From post_bfpt() to parse_4bait() you're uncovered.
>> Also, 4bait is optional and if it fails, we just continue, so you'll end
>> up with a wrong configuration. Please update.
>>
>>> +
>>> + /* 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;
>>
>> From the datasheet I see that there are two flavors of the read any
>> register command, one for volatile registers which does not require
>> latency cycles and one for non volatile registers which needs latency
>> cycles. Please rename CYPRESS_NOR_RD_ANY_REG_OP to
>> CYPRESS_NOR_RD_ANY_VREG_OP and introduce a CYPRESS_NOR_RD_ANY_NVREG_OP
>> which will handle the latency cycles.
>>
>> Also you should go with the maximum number of latency cycles if you want
>> to be able to operate this flash at its maximum frequency, see Table 43
>> from the datasheet.
>>
>> BTW, I hate the dummy.nbytes nonsense, there's no such thing as
>> dummy/latency number of bytes. This should have been defined as latency
>> number of cycles. Sergiu Moga had a patch addressing this in MTD and SPI
>> but I don't think he's going to send a new version. If there are no
>> other volunteers I'll respin it myself.
>>
>>> + 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 -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;
>>> +
>>> + /* PP_1_1_4_4B is supported but missing in SFDP. */
>>
>> missing in the 4-Byte Address Table you mean, right?
>>
>> Overall the patch is good. I'd like to address the non volatile byte
>> address mode if possible, but it's not a strong requirement. I'll read
>> again the datasheet tomorrow. The problem is that even if the flash has
>> a default 4 byte address mode (factory default), users can change the
>> address mode to 3 by writing to the non-volatile configuration register
>> and we'll end up with an unusable flash because as of now we assume the
>> flash is in the 4 byte address mode state. I'd like to determine the
>> address mode at runtime if possible. For this flash we can't use your CRC idea to determine the mode, as the flash does not have such support.
>> I see in the datasheet that "RDARG_C_0 transaction can be used during embedded operations to read Status Register 1 (STR1V)." RDSR1_0_0 does
>> not need any address, so an idea is to use RDARG_3_0, RDARG_4_0 and
>> RDSR1_0_0, compare what we receive and determine the mode. If we're
>> devilish, we can also do a WREN, change a bit in SR and reread all to double verify the result, then a WRDIS.
>> Other idea is to force 4 byte address mode by default using EN4BA_0_0,
>> this command does not need address bytes. Not the most elegant method
>> though, but it should work.
>> Again, I'd like to re-read the datasheet. If you have other idea, shoot.
>>
>
> I've re-read the datasheet I couldn't find a better way to determine the
> address mode. In these conditions, I would do a hybrid approach of the
> above two methods:
> I would compare SR1 data by issuing RDSR1, RDARG_3_0, RDARG_4_0. If
> there's no match or all have the same value issue EN4BA_0_0 and return.
> If there's a match, do WREN and read all again and compare, then WRDIS. If there's no match or all are the same issue EN4BA_0_0 and return.
> Compare the address mode determined at the first iteration with the
> address mode determined at the second (with WREN). If there's no match,
> issue EN4BA_0_0 and return. If there is a match, save the determined address mode and return.
>
I implemented and tested this. A problem was that all of RDSR1, RDARG_3_0,
and RDARG_4_0 returned 00h in case the device is in 3-byte mode, resulting
EN4BA_0_0. I don't think this is our expectation since the EN4BA should be
the last resort.
I modified the flow a little and submitted:
https://patchwork.ozlabs.org/project/linux-mtd/patch/20230315034004.5535-1-Takahiro.Kuwano@infineon.com/
I tested it and confirmed S25Hx-T, S25FS256T, and S28Hx-T works.
> No need to implement this right now, we can work in an incremental
> fashion. Please send a v3 addressing the other comments and then please
> try to solve this as well. With this you'll have a working flash even if
> the address mode was changed to 3, and we'll stop making assumptions on
> the address mode.
>
> Cheers,
> ta
More information about the linux-mtd
mailing list