[PATCH 0/2] mtd: spi-nor: Add support for Spansion S25FL256L

From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is a part of the S25FL-L family and has the same feature set as S25FL128L except the density.
This device does not support BAR. Not sure if patch #2 is a good way to avoid BAR. Please advise.
The datasheet can be found in the following link. https://www.cypress.com/file/316171/download
Tested on Xilinx Zynq-7000 FPGA board.
Takahiro Kuwano (2): mtd: spi-nor-ids: Add support for Spansion S25FL256L mtd: spi-nor-core: Add fixup for Spansion S25FL256L
drivers/mtd/spi/spi-nor-core.c | 21 +++++++++++++++++++++ drivers/mtd/spi/spi-nor-ids.c | 1 + 2 files changed, 22 insertions(+)

From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is a part of the S25FL-L family and has the same feature set as S25FL128L except the density.
The datasheet can be found in the following link. https://www.cypress.com/file/316171/download
Tested on Xilinx Zynq-7000 FPGA board.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- drivers/mtd/spi/spi-nor-ids.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 4aef1ddd6e..11d7d4fcaa 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -227,6 +227,7 @@ 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) }, + { INFO("s25fl256l", 0x016019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, { INFO6("s25hl512t", 0x342a1a, 0x0f0390, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | USE_CLSR) },

On 08/09/21 05:47PM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is a part of the S25FL-L family and has the same feature set as S25FL128L except the density.
The datasheet can be found in the following link. https://www.cypress.com/file/316171/download
Tested on Xilinx Zynq-7000 FPGA board.
I think you should add fixups and the flash entry in the same commit instead of adding them in separate ones. This would make sure the flash works correctly if someone lands on this commit when bisecting.
The patch LGTM otherwise.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
drivers/mtd/spi/spi-nor-ids.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 4aef1ddd6e..11d7d4fcaa 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -227,6 +227,7 @@ 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) },
- { INFO("s25fl256l", 0x016019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, { INFO6("s25hl512t", 0x342a1a, 0x0f0390, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | USE_CLSR) },
-- 2.25.1

From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is 32MB NOR Flash that does not support Bank Address Register. This fixup is activated if CONFIG_SPI_FLASH_BAR is enabled and returns ENOTSUPP in setup() hook to avoid further ops.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com --- drivers/mtd/spi/spi-nor-core.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..4940b35682 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3222,6 +3222,23 @@ static struct spi_nor_fixups s25hx_t_fixups = { .post_bfpt = s25hx_t_post_bfpt_fixup, .post_sfdp = s25hx_t_post_sfdp_fixup, }; + +#ifdef CONFIG_SPI_FLASH_BAR +static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info, + const struct spi_nor_flash_parameter *params) +{ + return -ENOTSUPP; /* Bank Address Register is not supported */ +} + +static void s25fl256l_default_init(struct spi_nor *nor) +{ + nor->setup = s25fl256l_setup; +} + +static struct spi_nor_fixups s25fl256l_fixups = { + .default_init = s25fl256l_default_init, +}; +#endif #endif
#ifdef CONFIG_SPI_FLASH_S28HS512T @@ -3644,6 +3661,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) break; } } +#ifdef CONFIG_SPI_FLASH_BAR + if (!strcmp(nor->info->name, "s25fl256l")) + nor->fixups = &s25fl256l_fixups; +#endif #endif
#ifdef CONFIG_SPI_FLASH_S28HS512T

On 08/09/21 05:47PM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is 32MB NOR Flash that does not support Bank Address Register. This fixup is activated if CONFIG_SPI_FLASH_BAR is enabled and returns ENOTSUPP in setup() hook to avoid further ops.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
drivers/mtd/spi/spi-nor-core.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..4940b35682 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3222,6 +3222,23 @@ static struct spi_nor_fixups s25hx_t_fixups = { .post_bfpt = s25hx_t_post_bfpt_fixup, .post_sfdp = s25hx_t_post_sfdp_fixup, };
+#ifdef CONFIG_SPI_FLASH_BAR +static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
const struct spi_nor_flash_parameter *params)
+{
- return -ENOTSUPP; /* Bank Address Register is not supported */
I am not very familiar with BAR. Can you please explain what would happen if we don't do this?
+}
+static void s25fl256l_default_init(struct spi_nor *nor) +{
- nor->setup = s25fl256l_setup;
+}
+static struct spi_nor_fixups s25fl256l_fixups = {
- .default_init = s25fl256l_default_init,
+}; +#endif #endif
#ifdef CONFIG_SPI_FLASH_S28HS512T @@ -3644,6 +3661,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) break; } } +#ifdef CONFIG_SPI_FLASH_BAR
We should avoid using #ifdef as much as possible. Change this to
if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) && !strcmp())
- if (!strcmp(nor->info->name, "s25fl256l"))
nor->fixups = &s25fl256l_fixups;
+#endif
*sigh* we really need to find a better way to specify fixups. Let me see if I can figure something out. In the meantime, this should be fine.
#endif
#ifdef CONFIG_SPI_FLASH_S28HS512T
2.25.1

On 9/18/2021 1:51 AM, Pratyush Yadav wrote:
On 08/09/21 05:47PM, tkuw584924@gmail.com wrote:
From: Takahiro Kuwano Takahiro.Kuwano@infineon.com
The S25FL256L is 32MB NOR Flash that does not support Bank Address Register. This fixup is activated if CONFIG_SPI_FLASH_BAR is enabled and returns ENOTSUPP in setup() hook to avoid further ops.
Signed-off-by: Takahiro Kuwano Takahiro.Kuwano@infineon.com
drivers/mtd/spi/spi-nor-core.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..4940b35682 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3222,6 +3222,23 @@ static struct spi_nor_fixups s25hx_t_fixups = { .post_bfpt = s25hx_t_post_bfpt_fixup, .post_sfdp = s25hx_t_post_sfdp_fixup, };
+#ifdef CONFIG_SPI_FLASH_BAR +static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
const struct spi_nor_flash_parameter *params)
+{
- return -ENOTSUPP; /* Bank Address Register is not supported */
I am not very familiar with BAR. Can you please explain what would happen if we don't do this?
The Bank Address Register (BAR) enables the 3-byte addressing opcodes to access beyond 16MB memory space by switching 16MB banks. If we don't do this, the spi-nor tries to use BAR with 3-byte addressing and the ops to the second half of 32MB will be performed in the first half of 32MB.
+}
+static void s25fl256l_default_init(struct spi_nor *nor) +{
- nor->setup = s25fl256l_setup;
+}
+static struct spi_nor_fixups s25fl256l_fixups = {
- .default_init = s25fl256l_default_init,
+}; +#endif #endif
#ifdef CONFIG_SPI_FLASH_S28HS512T @@ -3644,6 +3661,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) break; } } +#ifdef CONFIG_SPI_FLASH_BAR
We should avoid using #ifdef as much as possible. Change this to
if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) && !strcmp())
- if (!strcmp(nor->info->name, "s25fl256l"))
nor->fixups = &s25fl256l_fixups;
+#endif
*sigh* we really need to find a better way to specify fixups. Let me see if I can figure something out. In the meantime, this should be fine.
#endif
#ifdef CONFIG_SPI_FLASH_S28HS512T
2.25.1
participants (3)
-
Pratyush Yadav
-
Takahiro Kuwano
-
tkuw584924@gmail.com