Re: [PATCH] mtd: spi-nor: Add support for Infineon S25FS-S family
On 4/5/2024 7:10 PM, Pratyush Yadav wrote:
> Hi,
>
> Just a couple small comments. Looks good otherwise.
>
> On Fri, Apr 05 2024, tkuw584...@gmail.com wrote:
>
>> From: Takahiro Kuwano
>>
>> The S25FS064S, S25FS128S, and S25FS256S are the same family of SPI NOR
>> Flash devices with S25FS512S. Some difference depending on the device
>> densities are taken care in fixup hooks.
>>
>> Signed-off-by: Takahiro Kuwano
>> ---
>> Datasheets:
>> https://www.infineon.com/dgdl/Infineon-S25FS064S_64_Mb_8_MB_FS-S_Flash_SPI_Multi-I_O_1-DataSheet-v10_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed526b25412
>> https://www.infineon.com/dgdl/Infineon-S25FS128S_S25FS256S_1.8_V_Serial_Peripheral_Interface_with_Multi-I_O_MirrorBit(R)_Non-Volatile_Flash-DataSheet-v15_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed6b5ab5758
>> ---
>> drivers/mtd/spi/spi-nor-core.c | 28 +---
>> drivers/mtd/spi/spi-nor-ids.c | 7 +--
>> 2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>> index f86003ca8c..9b81b31e8e 100644
>> --- a/drivers/mtd/spi/spi-nor-core.c
>> +++ b/drivers/mtd/spi/spi-nor-core.c
>> @@ -3299,8 +3299,10 @@ static int s25fs_s_quad_enable(struct spi_nor *nor)
>>
>> static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr)
>> {
>> +u8 opcode = nor->addr_width == 4 ? SPINOR_OP_BE_4K_4B : SPINOR_OP_BE_4K;
>> +
>> /* Support 8 x 4KB sectors at bottom */
>> -return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0,
>> SZ_32K);
>> +return spansion_erase_non_uniform(nor, addr, opcode, 0, SZ_32K);
>
> Looks like an unrelated bugfix. Should this be a separate patch?
>
The existing S25FS512S uses 4-byte address so it's OK with 4B opcode.
But, yes, this is a bugfix and should be separated.
>> }
>>
>> static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info,
>> @@ -3354,12 +3356,24 @@ static int s25fs_s_post_bfpt_fixup(struct spi_nor
>> *nor,
>> static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor,
>> struct spi_nor_flash_parameter *params)
>> {
>> -/* READ_1_1_2 is not supported */
>> -params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
>> -/* READ_1_1_4 is not supported */
>> -params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
>> -/* PP_1_1_4 is not supported */
>> -params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
>> +/*
>> + * The S25FS064S(8MB) supports 1-1-2 and 1-1-4 commands, but params for
>> + * read ops in SFDP are wrong. The other density parts do not support
>> + * 1-1-2 and 1-1-4 commands.
>> + */
>> +if (params->size == SZ_8M) {
>> +spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_2],
>> + 0, 8, SPINOR_OP_READ_1_1_2,
>> + SNOR_PROTO_1_1_2);
>> +spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_4],
>> + 0, 8, SPINOR_OP_READ_1_1_4,
>> + SNOR_PROTO_1_1_4);
>> +} else {
>> +params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
>> +params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
>> +params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
>> +}
>> +
>> /* Use volatile register to enable quad */
>> params->quad_enable = s25fs_s_quad_enable;
>> }
>> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>> index 4e83b8c94c..9ca1f244f0 100644
>> --- a/drivers/mtd/spi/spi-nor-ids.c
>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>> @@ -338,9 +338,12 @@ const struct flash_info spi_nor_ids[] = {
>> */
>> { INFO("s25sl032p", 0x010215, 0x4d00, 64 * 1024, 64,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> -{ INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> -{ INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> +{ INFO6("s25fl256s0", 0x010219, 0x4d0080, 256 * 1024, 128,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> +{ INFO6("s25fl256s1", 0x010219, 0x4d0180, 64 * 1024, 512,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>
> Unrelated change. Is this intentional? If so, please make it a separate
> patch.
>
Yes, it's intentional. The 6th byte is needed to distinguish S25FL-S and
S25FS-S. Will make it separate patch in next version.
>> { INFO6("s25fl512s", 0x010220, 0x4d0080, 256 * 1024, 256,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> +{ INFO6("s25fs064s", 0x010217, 0x4d0181, 64 * 1024, 128,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
>> +{ INFO6("s25fs128s", 0x012018, 0x4d0181, 64 * 1024, 256,
>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
Re: [PATCH] mtd: spi-nor: Add support for Infineon S25FS-S family
Hi,
Just a couple small comments. Looks good otherwise.
On Fri, Apr 05 2024, tkuw584...@gmail.com wrote:
> From: Takahiro Kuwano
>
> The S25FS064S, S25FS128S, and S25FS256S are the same family of SPI NOR
> Flash devices with S25FS512S. Some difference depending on the device
> densities are taken care in fixup hooks.
>
> Signed-off-by: Takahiro Kuwano
> ---
> Datasheets:
> https://www.infineon.com/dgdl/Infineon-S25FS064S_64_Mb_8_MB_FS-S_Flash_SPI_Multi-I_O_1-DataSheet-v10_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed526b25412
> https://www.infineon.com/dgdl/Infineon-S25FS128S_S25FS256S_1.8_V_Serial_Peripheral_Interface_with_Multi-I_O_MirrorBit(R)_Non-Volatile_Flash-DataSheet-v15_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed6b5ab5758
> ---
> drivers/mtd/spi/spi-nor-core.c | 28 +---
> drivers/mtd/spi/spi-nor-ids.c | 7 +--
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index f86003ca8c..9b81b31e8e 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -3299,8 +3299,10 @@ static int s25fs_s_quad_enable(struct spi_nor *nor)
>
> static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr)
> {
> + u8 opcode = nor->addr_width == 4 ? SPINOR_OP_BE_4K_4B : SPINOR_OP_BE_4K;
> +
> /* Support 8 x 4KB sectors at bottom */
> - return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0,
> SZ_32K);
> + return spansion_erase_non_uniform(nor, addr, opcode, 0, SZ_32K);
Looks like an unrelated bugfix. Should this be a separate patch?
> }
>
> static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info,
> @@ -3354,12 +3356,24 @@ static int s25fs_s_post_bfpt_fixup(struct spi_nor
> *nor,
> static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor,
> struct spi_nor_flash_parameter *params)
> {
> - /* READ_1_1_2 is not supported */
> - params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
> - /* READ_1_1_4 is not supported */
> - params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
> - /* PP_1_1_4 is not supported */
> - params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
> + /*
> + * The S25FS064S(8MB) supports 1-1-2 and 1-1-4 commands, but params for
> + * read ops in SFDP are wrong. The other density parts do not support
> + * 1-1-2 and 1-1-4 commands.
> + */
> + if (params->size == SZ_8M) {
> + spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_2],
> + 0, 8, SPINOR_OP_READ_1_1_2,
> + SNOR_PROTO_1_1_2);
> + spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_4],
> + 0, 8, SPINOR_OP_READ_1_1_4,
> + SNOR_PROTO_1_1_4);
> + } else {
> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
> + params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
> + }
> +
> /* Use volatile register to enable quad */
> params->quad_enable = s25fs_s_quad_enable;
> }
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index 4e83b8c94c..9ca1f244f0 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -338,9 +338,12 @@ const struct flash_info spi_nor_ids[] = {
>*/
> { INFO("s25sl032p", 0x010215, 0x4d00, 64 * 1024, 64,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> { INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> - { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> - { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fl256s0", 0x010219, 0x4d0080, 256 * 1024, 128,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fl256s1", 0x010219, 0x4d0180, 64 * 1024, 512,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
Unrelated change. Is this intentional? If so, please make it a separate
patch.
> { INFO6("s25fl512s", 0x010220, 0x4d0080, 256 * 1024, 256,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fs064s", 0x010217, 0x4d0181, 64 * 1024, 128,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fs128s", 0x012018, 0x4d0181, 64 * 1024, 256,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> + { INFO6("s25fs256s", 0x010219, 0x4d0181, 64 * 1024, 512,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> { INFO6("s25fs512s", 0x010220, 0x4d0081, 256 * 1024, 256,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
> { INFO("s25fl512s_256k", 0x010220, 0x4d00, 256 * 1024, 256,
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
[PATCH] mtd: spi-nor: Add support for Infineon S25FS-S family
From: Takahiro Kuwano
The S25FS064S, S25FS128S, and S25FS256S are the same family of SPI NOR
Flash devices with S25FS512S. Some difference depending on the device
densities are taken care in fixup hooks.
Signed-off-by: Takahiro Kuwano
---
Datasheets:
https://www.infineon.com/dgdl/Infineon-S25FS064S_64_Mb_8_MB_FS-S_Flash_SPI_Multi-I_O_1-DataSheet-v10_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed526b25412
https://www.infineon.com/dgdl/Infineon-S25FS128S_S25FS256S_1.8_V_Serial_Peripheral_Interface_with_Multi-I_O_MirrorBit(R)_Non-Volatile_Flash-DataSheet-v15_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed6b5ab5758
---
drivers/mtd/spi/spi-nor-core.c | 28 +---
drivers/mtd/spi/spi-nor-ids.c | 7 +--
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index f86003ca8c..9b81b31e8e 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -3299,8 +3299,10 @@ static int s25fs_s_quad_enable(struct spi_nor *nor)
static int s25fs_s_erase_non_uniform(struct spi_nor *nor, loff_t addr)
{
+ u8 opcode = nor->addr_width == 4 ? SPINOR_OP_BE_4K_4B : SPINOR_OP_BE_4K;
+
/* Support 8 x 4KB sectors at bottom */
- return spansion_erase_non_uniform(nor, addr, SPINOR_OP_BE_4K_4B, 0,
SZ_32K);
+ return spansion_erase_non_uniform(nor, addr, opcode, 0, SZ_32K);
}
static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info,
@@ -3354,12 +3356,24 @@ static int s25fs_s_post_bfpt_fixup(struct spi_nor *nor,
static void s25fs_s_post_sfdp_fixup(struct spi_nor *nor,
struct spi_nor_flash_parameter *params)
{
- /* READ_1_1_2 is not supported */
- params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
- /* READ_1_1_4 is not supported */
- params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
- /* PP_1_1_4 is not supported */
- params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
+ /*
+* The S25FS064S(8MB) supports 1-1-2 and 1-1-4 commands, but params for
+* read ops in SFDP are wrong. The other density parts do not support
+* 1-1-2 and 1-1-4 commands.
+*/
+ if (params->size == SZ_8M) {
+ spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_2],
+ 0, 8, SPINOR_OP_READ_1_1_2,
+ SNOR_PROTO_1_1_2);
+ spi_nor_set_read_settings(>reads[SNOR_CMD_READ_1_1_4],
+ 0, 8, SPINOR_OP_READ_1_1_4,
+ SNOR_PROTO_1_1_4);
+ } else {
+ params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2;
+ params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4;
+ params->hwcaps.mask &= ~SNOR_HWCAPS_PP_1_1_4;
+ }
+
/* Use volatile register to enable quad */
params->quad_enable = s25fs_s_quad_enable;
}
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 4e83b8c94c..9ca1f244f0 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -338,9 +338,12 @@ const struct flash_info spi_nor_ids[] = {
*/
{ INFO("s25sl032p", 0x010215, 0x4d00, 64 * 1024, 64,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("s25sl064p", 0x010216, 0x4d00, 64 * 1024, 128,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
- { INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
- { INFO("s25fl256s1", 0x010219, 0x4d01, 64 * 1024, 512,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fl256s0", 0x010219, 0x4d0080, 256 * 1024, 128,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fl256s1", 0x010219, 0x4d0180, 64 * 1024, 512,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO6("s25fl512s", 0x010220, 0x4d0080, 256 * 1024, 256,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fs064s", 0x010217, 0x4d0181, 64 * 1024, 128,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fs128s", 0x012018, 0x4d0181, 64 * 1024, 256,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+ { INFO6("s25fs256s", 0x010219, 0x4d0181, 64 * 1024, 512,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO6("s25fs512s", 0x010220, 0x4d0081, 256 * 1024, 256,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_256k", 0x010220, 0x4d00, 256 * 1024, 256,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_64k", 0x010220, 0x4d01, 64 * 1024, 1024,
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
--
2.34.1
