[PATCH v2 0/4] mtd: spi-nor: Fix software reset; add mx66lm1g45g

Add support for mx66lm1g45g: - Introduce SPI_NOR_SOFT_RESET flash_info flag -> the flash does not define the SFDP tables and can't determine its reset sequence on its own. - Fix the opcode extension for the software reset sequence -> it was hardcoded.
Changes in v2: revert soft reset on boot.
Tudor Ambarus (4): mtd: spi-nor-core: Introduce SPI_NOR_SOFT_RESET flash_info flag mtd: spi-nor-core: macronix: Add support for mx66lm1g45g Revert "mtd: spi-nor-core: Perform a Soft Reset on boot" mtd: spi-nor-core: Fix the opcode extension for the software reset sequence
drivers/mtd/spi/Kconfig | 17 +++-- drivers/mtd/spi/sf_internal.h | 5 ++ drivers/mtd/spi/spi-nor-core.c | 116 +++++++++++++++++++++++---------- drivers/mtd/spi/spi-nor-ids.c | 6 ++ include/linux/mtd/spi-nor.h | 8 +++ 5 files changed, 107 insertions(+), 45 deletions(-)

Some flashes can determine if Soft Reset is supported by parsing BFPT_DWORD(16). There are however flashes that do not define any of the SFDP tables, so they can't discover this capability at SFDP parsing time. For such cases introduce the SPI_NOR_SOFT_RESET flash_info flag to be able to specify statically this support. This flag must be used together with the SPI_NOR_SKIP_SFDP flag. For flashes that support Soft Reset and define BFPT, but have the BFPT_DWORD(16) missing or with a wrong value, a post_sfdp() fixup hook should be used instead, where SNOR_F_SOFT_RESET should be set.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com --- drivers/mtd/spi/sf_internal.h | 5 +++++ drivers/mtd/spi/spi-nor-core.c | 3 +++ 2 files changed, 8 insertions(+)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index d3ef69ec74..9a16e07bc4 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -69,6 +69,11 @@ struct flash_info { #define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via BPR */ #define SPI_NOR_OCTAL_READ BIT(16) /* Flash supports Octal Read */ #define SPI_NOR_OCTAL_DTR_READ BIT(17) /* Flash supports Octal DTR Read */ +#define SPI_NOR_SOFT_RESET BIT(18) /* + * Used by flashes that do not define + * any SFDP tables, i.e. in conjunction + * with SPI_NOR_SKIP_SFDP. + */ };
extern const struct flash_info spi_nor_ids[]; diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 4388a08a90..ce1ecae899 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3813,6 +3813,9 @@ int spi_nor_scan(struct spi_nor *nor) if (info->flags & SPI_NOR_NO_ERASE) mtd->flags |= MTD_NO_ERASE;
+ if (info->flags & SPI_NOR_SOFT_RESET) + nor->flags |= SNOR_F_SOFT_RESET; + nor->page_size = params.page_size; mtd->writebufsize = nor->page_size;

On 04/11/21 01:49AM, Tudor Ambarus wrote:
Some flashes can determine if Soft Reset is supported by parsing BFPT_DWORD(16). There are however flashes that do not define any of the SFDP tables, so they can't discover this capability at SFDP parsing time. For such cases introduce the SPI_NOR_SOFT_RESET flash_info flag to be able to specify statically this support. This flag must be used together with the SPI_NOR_SKIP_SFDP flag. For flashes that support Soft Reset and define BFPT, but have the BFPT_DWORD(16) missing or with a wrong value, a post_sfdp() fixup hook should be used instead, where SNOR_F_SOFT_RESET should be set.
Reviewed-by: Pratyush Yadav p.yadav@ti.com

mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. Tested in 1-1-1 and 8d-8d-8d modes using microchip's Octal SPI IP.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com --- drivers/mtd/spi/Kconfig | 8 ++++ drivers/mtd/spi/spi-nor-core.c | 74 ++++++++++++++++++++++++++++++++++ drivers/mtd/spi/spi-nor-ids.c | 6 +++ include/linux/mtd/spi-nor.h | 8 ++++ 4 files changed, 96 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 408a53f861..2d2b71c52d 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -160,6 +160,14 @@ config SPI_FLASH_MACRONIX help Add support for various Macronix SPI flash chips (MX25Lxxx)
+config SPI_FLASH_MX66LM1G45G + bool "Macronix MX66LM1G45G chip support" + depends on SPI_FLASH_MACRONIX + help + Add support for the Macronix mx66lm1g45g chip. This is a separate + config because the fixup hooks for this flash add extra size overhead. + Boards that don't use the flash can disable this to save space. + config SPI_FLASH_SPANSION bool "Spansion SPI flash support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index ce1ecae899..df66a73495 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3526,6 +3526,75 @@ static struct spi_nor_fixups mt35xu512aba_fixups = { }; #endif /* CONFIG_SPI_FLASH_MT35XU */
+#ifdef CONFIG_SPI_FLASH_MX66LM1G45G +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor) +{ + struct spi_mem_op op; + u8 buf; + int ret; + + /* Use 20 dummy cycles for memory array reads. */ + buf = SPINOR_REG_CR2_DUMMY_20; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_DUMMY_ADDR, + 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); + + ret = write_enable(nor); + if (ret) + return ret; + + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) + return ret; + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + buf = SPINOR_REG_CR2_DTR_OPI_ENABLE; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRITE_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_CR2_MODE_ADDR, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); + + ret = write_enable(nor); + if (ret) + return ret; + + return spi_mem_exec_op(nor->spi, &op); +} + +static void mx66lm1g45g_default_init(struct spi_nor *nor) +{ + nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; +} + +static void mx66lm1g45g_post_sfdp(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) +{ + /* Set the Fast Read settings. */ + params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR; + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8_DTR], + 0, 20, SPINOR_OP_MX_DTR_RD, + SNOR_PROTO_8_8_8_DTR); + + params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR; + + nor->cmd_ext_type = SPI_NOR_EXT_INVERT; + params->rdsr_dummy = 4; + params->rdsr_addr_nbytes = 4; +} + +static struct spi_nor_fixups mx66lm1g45g_fixups = { + .default_init = mx66lm1g45g_default_init, + .post_sfdp = mx66lm1g45g_post_sfdp, +}; +#endif /* CONFIG_SPI_FLASH_MX66LM1G45G */ + /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed * @nor: pointer to a 'struct spi_nor' * @@ -3696,6 +3765,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) if (!strcmp(nor->info->name, "mt35xu512aba")) nor->fixups = &mt35xu512aba_fixups; #endif + +#ifdef CONFIG_SPI_FLASH_MX66LM1G45G + if (!strcmp(nor->info->name, "mx66lm1g45g")) + nor->fixups = &mx66lm1g45g_fixups; +#endif }
int spi_nor_scan(struct spi_nor *nor) diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 3ae7bb1ed7..d5bc106e68 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -179,6 +179,12 @@ const struct flash_info spi_nor_ids[] = { { INFO("mx25l1633e", 0xc22415, 0, 64 * 1024, 32, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) }, { INFO("mx25r6435f", 0xc22817, 0, 64 * 1024, 128, SECT_4K) }, { INFO("mx66uw2g345g", 0xc2943c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) }, +#ifdef CONFIG_SPI_FLASH_MX66LM1G45G + { INFO("mx66lm1g45g", 0xc2853b, 0, 64 * 1024, 2048, + SPI_NOR_SKIP_SFDP | SPI_NOR_SOFT_RESET | + SECT_4K | SPI_NOR_OCTAL_DTR_READ | SPI_NOR_4B_OPCODES) + }, +#endif #endif
#ifdef CONFIG_SPI_FLASH_STMICRO /* STMICRO */ diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 4ceeae623d..c8b8a242ba 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -119,6 +119,14 @@ /* Used for Macronix and Winbond flashes. */ #define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ #define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ +#define SPINOR_OP_READ_CR2 0x71 +#define SPINOR_OP_WRITE_CR2 0x72 +#define SPINOR_OP_MX_DTR_RD 0xee +#define SPINOR_REG_CR2_MODE_ADDR 0 +#define SPINOR_REG_CR2_DTR_OPI_ENABLE BIT(1) +#define SPINOR_REG_CR2_SPI 0 +#define SPINOR_REG_CR2_DUMMY_ADDR 0x300 +#define SPINOR_REG_CR2_DUMMY_20 0
/* Used for Spansion flashes only. */ #define SPINOR_OP_BRWR 0x17 /* Bank register write */

On 04/11/21 01:49AM, Tudor Ambarus wrote:
mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. Tested in 1-1-1 and 8d-8d-8d modes using microchip's Octal SPI IP.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 8 ++++ drivers/mtd/spi/spi-nor-core.c | 74 ++++++++++++++++++++++++++++++++++ drivers/mtd/spi/spi-nor-ids.c | 6 +++ include/linux/mtd/spi-nor.h | 8 ++++ 4 files changed, 96 insertions(+)
@@ -3696,6 +3765,11 @@ void spi_nor_set_fixups(struct spi_nor *nor) if (!strcmp(nor->info->name, "mt35xu512aba")) nor->fixups = &mt35xu512aba_fixups; #endif
+#ifdef CONFIG_SPI_FLASH_MX66LM1G45G
- if (!strcmp(nor->info->name, "mx66lm1g45g"))
nor->fixups = &mx66lm1g45g_fixups;
+#endif
*sigh* this is very ugly. Let's make it a bit better.
Takahiro, this shouldn't break the other Cypress flashes either, but I don't have them so I can't test them. A Tested-by would be nice.
-- 8< -- Subject: [PATCH] mtd: spi-nor-core: clean up how fixups are set
Currently fixups for flashes are set via the function spi_nor_set_fixups(). This was done since tiny SPI NOR does not support fixups so fixups could not be set directly in the flash_info entry.
The function identifies flashes by either strcmp()ing their name or checking their ID bytes. Neither approach is not very elegant and does not scale very well when there are multiple flashes specifying the fixups.
Add a macro that conditionally sets the fixups based on if tiny SPI NOR is enabled or not. Also, move the fixups from struct spi_nor to flash_info since they make more sense there. The fixups are defined in spi-nor-core.c so they need to be exported, so they are no longer static.
Tested with Cypress S28HS512T and Micron MT35XU512ABA.
Signed-off-by: Pratyush Yadav p.yadav@ti.com --- drivers/mtd/spi/sf_internal.h | 4 +++ drivers/mtd/spi/spi-nor-core.c | 57 ++++++++-------------------------- drivers/mtd/spi/spi-nor-ids.c | 42 +++++++++++++++++++------ include/linux/mtd/spi-nor.h | 1 - 4 files changed, 50 insertions(+), 54 deletions(-)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index d3ef69ec74..2e251c1d63 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -69,6 +69,10 @@ struct flash_info { #define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via BPR */ #define SPI_NOR_OCTAL_READ BIT(16) /* Flash supports Octal Read */ #define SPI_NOR_OCTAL_DTR_READ BIT(17) /* Flash supports Octal DTR Read */ + +#if !CONFIG_IS_ENABLED(SPI_FLASH_TINY) + const struct spi_nor_fixups *fixups; +#endif };
extern const struct flash_info spi_nor_ids[]; diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 4388a08a90..dd3d9ea4cf 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -2139,8 +2139,9 @@ spi_nor_post_bfpt_fixups(struct spi_nor *nor, const struct sfdp_bfpt *bfpt, struct spi_nor_flash_parameter *params) { - if (nor->fixups && nor->fixups->post_bfpt) - return nor->fixups->post_bfpt(nor, bfpt_header, bfpt, params); + if (nor->info->fixups && nor->info->fixups->post_bfpt) + return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt, + params);
return 0; } @@ -2605,14 +2606,14 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor, static void spi_nor_post_sfdp_fixups(struct spi_nor *nor, struct spi_nor_flash_parameter *params) { - if (nor->fixups && nor->fixups->post_sfdp) - nor->fixups->post_sfdp(nor, params); + if (nor->info->fixups && nor->info->fixups->post_sfdp) + nor->info->fixups->post_sfdp(nor, params); }
static void spi_nor_default_init_fixups(struct spi_nor *nor) { - if (nor->fixups && nor->fixups->default_init) - nor->fixups->default_init(nor); + if (nor->info->fixups && nor->info->fixups->default_init) + nor->info->fixups->default_init(nor); }
static int spi_nor_init_params(struct spi_nor *nor, @@ -3239,7 +3240,7 @@ static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor, params->quad_enable = s25hx_t_quad_enable; }
-static struct spi_nor_fixups s25hx_t_fixups = { +struct spi_nor_fixups s25hx_t_fixups = { .default_init = s25hx_t_default_init, .post_bfpt = s25hx_t_post_bfpt_fixup, .post_sfdp = s25hx_t_post_sfdp_fixup, @@ -3253,10 +3254,11 @@ static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
static void s25fl256l_default_init(struct spi_nor *nor) { - nor->setup = s25fl256l_setup; + if (CONFIG_IS_ENABLED(SPI_FLASH_BAR)) + nor->setup = s25fl256l_setup; }
-static struct spi_nor_fixups s25fl256l_fixups = { +struct spi_nor_fixups s25fl256l_fixups = { .default_init = s25fl256l_default_init, }; #endif @@ -3437,7 +3439,7 @@ static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor, return 0; }
-static struct spi_nor_fixups s28hs512t_fixups = { +struct spi_nor_fixups s28hs512t_fixups = { .default_init = s28hs512t_default_init, .post_sfdp = s28hs512t_post_sfdp_fixup, .post_bfpt = s28hs512t_post_bfpt_fixup, @@ -3520,7 +3522,7 @@ static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor, params->quad_enable = NULL; }
-static struct spi_nor_fixups mt35xu512aba_fixups = { +struct spi_nor_fixups mt35xu512aba_fixups = { .default_init = mt35xu512aba_default_init, .post_sfdp = mt35xu512aba_post_sfdp_fixup, }; @@ -3667,37 +3669,6 @@ int spi_nor_remove(struct spi_nor *nor) return 0; }
-void spi_nor_set_fixups(struct spi_nor *nor) -{ -#ifdef CONFIG_SPI_FLASH_SPANSION - if (JEDEC_MFR(nor->info) == SNOR_MFR_CYPRESS) { - switch (nor->info->id[1]) { - case 0x2a: /* S25HL (QSPI, 3.3V) */ - case 0x2b: /* S25HS (QSPI, 1.8V) */ - nor->fixups = &s25hx_t_fixups; - break; - - default: - break; - } - } - - if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) && - !strcmp(nor->info->name, "s25fl256l")) - nor->fixups = &s25fl256l_fixups; -#endif - -#ifdef CONFIG_SPI_FLASH_S28HS512T - if (!strcmp(nor->info->name, "s28hs512t")) - nor->fixups = &s28hs512t_fixups; -#endif - -#ifdef CONFIG_SPI_FLASH_MT35XU - if (!strcmp(nor->info->name, "mt35xu512aba")) - nor->fixups = &mt35xu512aba_fixups; -#endif -} - int spi_nor_scan(struct spi_nor *nor) { struct spi_nor_flash_parameter params; @@ -3754,8 +3725,6 @@ int spi_nor_scan(struct spi_nor *nor) return -ENOENT; nor->info = info;
- spi_nor_set_fixups(nor); - /* Parse the Serial Flash Discoverable Parameters table. */ ret = spi_nor_init_params(nor, info, ¶ms); if (ret) diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 3ae7bb1ed7..f063209440 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -19,6 +19,25 @@ #define INFO_NAME(_name) #endif
+/* Fixups are not implemented by tiny SPI NOR. Exclude them. */ +#if !CONFIG_IS_ENABLED(SPI_FLASH_TINY) +#define FIXUPS(_fixups) .fixups = _fixups, +#else +#define FIXUPS(_fixups) +#endif + +#ifdef CONFIG_SPI_FLASH_SPANSION +extern struct spi_nor_fixups s25hx_t_fixups, s25fl256l_fixups; +#endif + +#ifdef CONFIG_SPI_FLASH_S28HS512T +extern struct spi_nor_fixups s28hs512t_fixups; +#endif + +#ifdef CONFIG_SPI_FLASH_MT35XU +extern struct spi_nor_fixups mt35xu512aba_fixups; +#endif + /* Used when the "_ext_id" is two bytes at most */ #define INFO(_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \ INFO_NAME(_name) \ @@ -206,7 +225,8 @@ const struct flash_info spi_nor_ids[] = { { INFO("mt25qu02g", 0x20bb22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) }, { INFO("mt25ql02g", 0x20ba22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE | SPI_NOR_4B_OPCODES) }, #ifdef CONFIG_SPI_FLASH_MT35XU - { INFO("mt35xu512aba", 0x2c5b1a, 0, 128 * 1024, 512, USE_FSR | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_DTR_READ) }, + { INFO("mt35xu512aba", 0x2c5b1a, 0, 128 * 1024, 512, USE_FSR | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_DTR_READ) + FIXUPS(&mt35xu512aba_fixups) }, #endif /* CONFIG_SPI_FLASH_MT35XU */ { INFO("mt35xu02g", 0x2c5b1c, 0, 128 * 1024, 2048, USE_FSR | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) }, #endif @@ -237,25 +257,29 @@ 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) }, + { INFO("s25fl256l", 0x016019, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) FIXUPS(&s25fl256l_fixups) }, { INFO6("s25hl512t", 0x342a1a, 0x0f0390, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | - USE_CLSR) }, + USE_CLSR) FIXUPS(&s25hx_t_fixups) }, { INFO6("s25hl01gt", 0x342a1b, 0x0f0390, 256 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | - USE_CLSR) }, + USE_CLSR) FIXUPS(&s25hx_t_fixups) }, { INFO6("s25hl02gt", 0x342a1c, 0x0f0090, 256 * 1024, 1024, - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) + FIXUPS(&s25hx_t_fixups) }, { INFO6("s25hs512t", 0x342b1a, 0x0f0390, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | - USE_CLSR) }, + USE_CLSR) + FIXUPS(&s25hx_t_fixups) }, { INFO6("s25hs01gt", 0x342b1b, 0x0f0390, 256 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | - USE_CLSR) }, + USE_CLSR) + FIXUPS(&s25hx_t_fixups) }, { INFO6("s25hs02gt", 0x342b1c, 0x0f0090, 256 * 1024, 1024, - SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) }, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) + FIXUPS(&s25hx_t_fixups) }, #ifdef CONFIG_SPI_FLASH_S28HS512T - { INFO("s28hs512t", 0x345b1a, 0, 256 * 1024, 256, SPI_NOR_OCTAL_DTR_READ) }, + { INFO("s28hs512t", 0x345b1a, 0, 256 * 1024, 256, SPI_NOR_OCTAL_DTR_READ) FIXUPS(&s28hs512t_fixups) }, #endif #endif #ifdef CONFIG_SPI_FLASH_SST /* SST */ diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 4ceeae623d..7afb8396d8 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -539,7 +539,6 @@ struct spi_nor { u32 flags; u8 cmd_buf[SPI_NOR_MAX_CMD_SIZE]; enum spi_nor_cmd_ext cmd_ext_type; - struct spi_nor_fixups *fixups;
int (*setup)(struct spi_nor *nor, const struct flash_info *info, const struct spi_nor_flash_parameter *params);

On Thu, Nov 4, 2021 at 5:20 AM Tudor Ambarus tudor.ambarus@microchip.com wrote:
mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. Tested in 1-1-1 and 8d-8d-8d modes using microchip's Octal SPI IP.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 8 ++++ drivers/mtd/spi/spi-nor-core.c | 74 ++++++++++++++++++++++++++++++++++ drivers/mtd/spi/spi-nor-ids.c | 6 +++ include/linux/mtd/spi-nor.h | 8 ++++ 4 files changed, 96 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 408a53f861..2d2b71c52d 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -160,6 +160,14 @@ config SPI_FLASH_MACRONIX help Add support for various Macronix SPI flash chips (MX25Lxxx)
+config SPI_FLASH_MX66LM1G45G
bool "Macronix MX66LM1G45G chip support"
Apart from actual code, I don't like to have specific part configs, look like list is increasing like anything. Better use existing macronix.
Jagan.

On 11/13/21 3:48 PM, Jagan Teki wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
On Thu, Nov 4, 2021 at 5:20 AM Tudor Ambarus tudor.ambarus@microchip.com wrote:
mx66lm1g45g supports just 1-1-1, 8-8-8 and 8d-8d-8d modes. Tested in 1-1-1 and 8d-8d-8d modes using microchip's Octal SPI IP.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 8 ++++ drivers/mtd/spi/spi-nor-core.c | 74 ++++++++++++++++++++++++++++++++++ drivers/mtd/spi/spi-nor-ids.c | 6 +++ include/linux/mtd/spi-nor.h | 8 ++++ 4 files changed, 96 insertions(+)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 408a53f861..2d2b71c52d 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -160,6 +160,14 @@ config SPI_FLASH_MACRONIX help Add support for various Macronix SPI flash chips (MX25Lxxx)
+config SPI_FLASH_MX66LM1G45G
bool "Macronix MX66LM1G45G chip support"
Apart from actual code, I don't like to have specific part configs, look like list is increasing like anything. Better use existing macronix.
I don't like it either, I just followed the u-boot code. Will come up with something.
Cheers, ta

This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess, see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) ''' If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com --- drivers/mtd/spi/Kconfig | 9 --------- drivers/mtd/spi/spi-nor-core.c | 27 --------------------------- 2 files changed, 36 deletions(-)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 2d2b71c52d..14800b5e93 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET Enable support for xSPI Software Reset. It will be used to switch from Octal DTR mode to legacy mode on shutdown and boot (if enabled).
-config SPI_FLASH_SOFT_RESET_ON_BOOT - bool "Perform a Software Reset on boot on flashes that boot in stateful mode" - depends on SPI_FLASH_SOFT_RESET - help - Perform a Software Reset on boot to allow detecting flashes that are - handed to us in Octal DTR mode. Do not enable this config on flashes - that are not supposed to be handed to U-Boot in Octal DTR mode, even - if they _do_ support the Soft Reset sequence. - config SPI_FLASH_BAR bool "SPI flash Bank/Extended address register support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index df66a73495..caf764720c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3796,33 +3796,6 @@ int spi_nor_scan(struct spi_nor *nor)
nor->setup = spi_nor_default_setup;
-#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT - /* - * When the flash is handed to us in a stateful mode like 8D-8D-8D, it - * is difficult to detect the mode the flash is in. One option is to - * read SFDP in all modes and see which one gives the correct "SFDP" - * signature, but not all flashes support SFDP in 8D-8D-8D mode. - * - * Further, even if you detect the mode of the flash via SFDP, you - * still have the problem of actually reading the ID. The Read ID - * command is not standardized across flash vendors. Flashes can have - * different dummy cycles needed for reading the ID. Some flashes even - * expect a 4-byte dummy address with the Read ID command. All this - * information cannot be obtained from the SFDP table. - * - * So, perform a Software Reset sequence before reading the ID and - * initializing the flash. A Soft Reset will bring back the flash in - * its default protocol mode assuming no non-volatile configuration was - * set. This will let us detect the flash even if ROM hands it to us in - * Octal DTR mode. - * - * To accommodate cases where there is more than one flash on a board, - * and only one of them needs a soft reset, failure to reset is not - * made fatal, and we still try to read ID if possible. - */ - spi_nor_soft_reset(nor); -#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */ - info = spi_nor_read_id(nor); if (IS_ERR_OR_NULL(info)) return -ENOENT;

Hi Tudor,
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an 8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
I can implement such an algorithm but is it really worth the hassle?
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) ''' If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 9 --------- drivers/mtd/spi/spi-nor-core.c | 27 --------------------------- 2 files changed, 36 deletions(-)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 2d2b71c52d..14800b5e93 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET Enable support for xSPI Software Reset. It will be used to switch from Octal DTR mode to legacy mode on shutdown and boot (if enabled).
-config SPI_FLASH_SOFT_RESET_ON_BOOT
- bool "Perform a Software Reset on boot on flashes that boot in stateful mode"
- depends on SPI_FLASH_SOFT_RESET
- help
Perform a Software Reset on boot to allow detecting flashes that are
handed to us in Octal DTR mode. Do not enable this config on flashes
that are not supposed to be handed to U-Boot in Octal DTR mode, even
if they _do_ support the Soft Reset sequence.
config SPI_FLASH_BAR bool "SPI flash Bank/Extended address register support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index df66a73495..caf764720c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3796,33 +3796,6 @@ int spi_nor_scan(struct spi_nor *nor)
nor->setup = spi_nor_default_setup;
-#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
- /*
* When the flash is handed to us in a stateful mode like 8D-8D-8D, it
* is difficult to detect the mode the flash is in. One option is to
* read SFDP in all modes and see which one gives the correct "SFDP"
* signature, but not all flashes support SFDP in 8D-8D-8D mode.
*
* Further, even if you detect the mode of the flash via SFDP, you
* still have the problem of actually reading the ID. The Read ID
* command is not standardized across flash vendors. Flashes can have
* different dummy cycles needed for reading the ID. Some flashes even
* expect a 4-byte dummy address with the Read ID command. All this
* information cannot be obtained from the SFDP table.
*
* So, perform a Software Reset sequence before reading the ID and
* initializing the flash. A Soft Reset will bring back the flash in
* its default protocol mode assuming no non-volatile configuration was
* set. This will let us detect the flash even if ROM hands it to us in
* Octal DTR mode.
*
* To accommodate cases where there is more than one flash on a board,
* and only one of them needs a soft reset, failure to reset is not
* made fatal, and we still try to read ID if possible.
*/
- spi_nor_soft_reset(nor);
-#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */
- info = spi_nor_read_id(nor); if (IS_ERR_OR_NULL(info)) return -ENOENT;
-- 2.25.1

On 11/9/21 9:26 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
Hi, Pratyush,
Thanks for reviewing the series.
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
If flash supports SFDP, the read ID becomes of minor importance. We can as well not issue the read ID at all.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported
but we can have a dt property or a config option to indicate when to issue readSFDP.
commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
in case we can't determine the mode in which the flash is configured, we can adopt other approach, see below.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an
not for every flash, just the ones that we marked as SFDP compliant.
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command
right.
then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
not necessarily.
I can implement such an algorithm but is it really worth the hassle?
I find having such an alg would be of benefit, yes.
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) '''
Below is the approach for the flashes that are not SFDP compliant:
If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
Apart of the cmd_ext type, you need to also differentiate between the reset types. So a combination of reset and cmd extension type configs.
I prefer discovering the reset sequence dynamically whenever possible. You can choose to use the configs approach directly, if you don't want to spend time on the dynamic discovery approach, the two approaches will be mutual exclusive anyway.
So yes, I think we should revert this and at least have a better config approach.
Cheers, ta
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 9 --------- drivers/mtd/spi/spi-nor-core.c | 27 --------------------------- 2 files changed, 36 deletions(-)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 2d2b71c52d..14800b5e93 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET Enable support for xSPI Software Reset. It will be used to switch from Octal DTR mode to legacy mode on shutdown and boot (if enabled).
-config SPI_FLASH_SOFT_RESET_ON_BOOT
bool "Perform a Software Reset on boot on flashes that boot in stateful mode"
depends on SPI_FLASH_SOFT_RESET
help
Perform a Software Reset on boot to allow detecting flashes that are
handed to us in Octal DTR mode. Do not enable this config on flashes
that are not supposed to be handed to U-Boot in Octal DTR mode, even
if they _do_ support the Soft Reset sequence.
config SPI_FLASH_BAR bool "SPI flash Bank/Extended address register support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index df66a73495..caf764720c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3796,33 +3796,6 @@ int spi_nor_scan(struct spi_nor *nor)
nor->setup = spi_nor_default_setup;
-#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
/*
* When the flash is handed to us in a stateful mode like 8D-8D-8D, it
* is difficult to detect the mode the flash is in. One option is to
* read SFDP in all modes and see which one gives the correct "SFDP"
* signature, but not all flashes support SFDP in 8D-8D-8D mode.
*
* Further, even if you detect the mode of the flash via SFDP, you
* still have the problem of actually reading the ID. The Read ID
* command is not standardized across flash vendors. Flashes can have
* different dummy cycles needed for reading the ID. Some flashes even
* expect a 4-byte dummy address with the Read ID command. All this
* information cannot be obtained from the SFDP table.
*
* So, perform a Software Reset sequence before reading the ID and
* initializing the flash. A Soft Reset will bring back the flash in
* its default protocol mode assuming no non-volatile configuration was
* set. This will let us detect the flash even if ROM hands it to us in
* Octal DTR mode.
*
* To accommodate cases where there is more than one flash on a board,
* and only one of them needs a soft reset, failure to reset is not
* made fatal, and we still try to read ID if possible.
*/
spi_nor_soft_reset(nor);
-#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */
info = spi_nor_read_id(nor); if (IS_ERR_OR_NULL(info)) return -ENOENT;
-- 2.25.1
-- Regards, Pratyush Yadav Texas Instruments Inc.

On 10/11/21 08:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/9/21 9:26 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
Hi, Pratyush,
Thanks for reviewing the series.
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
If flash supports SFDP, the read ID becomes of minor importance. We can as well not issue the read ID at all.
We need to read the ID so we can apply fixups. One option is to specify Read ID type in device tree.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported
but we can have a dt property or a config option to indicate when to issue readSFDP.
Okay.
commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
in case we can't determine the mode in which the flash is configured, we can adopt other approach, see below.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an
not for every flash, just the ones that we marked as SFDP compliant.
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command
right.
then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
not necessarily.
I can implement such an algorithm but is it really worth the hassle?
I find having such an alg would be of benefit, yes.
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) '''
Below is the approach for the flashes that are not SFDP compliant:
If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
There are some problems with this approach. What if we have two flashes on the board and both use different reset types? How do we figure out which reset to apply? This applies to the current implementation as well. If there are two flashes then it will issue the reset to both even if one of them does not support/need it.
Apart of the cmd_ext type, you need to also differentiate between the reset types. So a combination of reset and cmd extension type configs.
I prefer discovering the reset sequence dynamically whenever possible. You can choose to use the configs approach directly, if you don't want to spend time on the dynamic discovery approach, the two approaches will be mutual exclusive anyway.
So yes, I think we should revert this and at least have a better config approach.
Cheers, ta
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 9 --------- drivers/mtd/spi/spi-nor-core.c | 27 --------------------------- 2 files changed, 36 deletions(-)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 2d2b71c52d..14800b5e93 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET Enable support for xSPI Software Reset. It will be used to switch from Octal DTR mode to legacy mode on shutdown and boot (if enabled).
-config SPI_FLASH_SOFT_RESET_ON_BOOT
bool "Perform a Software Reset on boot on flashes that boot in stateful mode"
depends on SPI_FLASH_SOFT_RESET
help
Perform a Software Reset on boot to allow detecting flashes that are
handed to us in Octal DTR mode. Do not enable this config on flashes
that are not supposed to be handed to U-Boot in Octal DTR mode, even
if they _do_ support the Soft Reset sequence.
config SPI_FLASH_BAR bool "SPI flash Bank/Extended address register support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index df66a73495..caf764720c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3796,33 +3796,6 @@ int spi_nor_scan(struct spi_nor *nor)
nor->setup = spi_nor_default_setup;
-#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
/*
* When the flash is handed to us in a stateful mode like 8D-8D-8D, it
* is difficult to detect the mode the flash is in. One option is to
* read SFDP in all modes and see which one gives the correct "SFDP"
* signature, but not all flashes support SFDP in 8D-8D-8D mode.
*
* Further, even if you detect the mode of the flash via SFDP, you
* still have the problem of actually reading the ID. The Read ID
* command is not standardized across flash vendors. Flashes can have
* different dummy cycles needed for reading the ID. Some flashes even
* expect a 4-byte dummy address with the Read ID command. All this
* information cannot be obtained from the SFDP table.
*
* So, perform a Software Reset sequence before reading the ID and
* initializing the flash. A Soft Reset will bring back the flash in
* its default protocol mode assuming no non-volatile configuration was
* set. This will let us detect the flash even if ROM hands it to us in
* Octal DTR mode.
*
* To accommodate cases where there is more than one flash on a board,
* and only one of them needs a soft reset, failure to reset is not
* made fatal, and we still try to read ID if possible.
*/
spi_nor_soft_reset(nor);
-#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */
info = spi_nor_read_id(nor); if (IS_ERR_OR_NULL(info)) return -ENOENT;
-- 2.25.1
-- Regards, Pratyush Yadav Texas Instruments Inc.

On 11/12/21 3:13 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
On 10/11/21 08:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/9/21 9:26 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
Hi, Pratyush,
Thanks for reviewing the series.
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
If flash supports SFDP, the read ID becomes of minor importance. We can as well not issue the read ID at all.
We need to read the ID so we can apply fixups. One option is to specify
Fixup hooks are there just because the manufacturers can't get their SFDP tables right. Read ID determines the static flags and parameters, which should be used just as a fallback.
Read ID type in device tree.
we may consider this when really needed. Until then we can use the recommended sequence I guess.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported
but we can have a dt property or a config option to indicate when to issue readSFDP.
Okay.
commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
in case we can't determine the mode in which the flash is configured, we can adopt other approach, see below.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an
not for every flash, just the ones that we marked as SFDP compliant.
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command
right.
then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
not necessarily.
I can implement such an algorithm but is it really worth the hassle?
I find having such an alg would be of benefit, yes.
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) '''
Below is the approach for the flashes that are not SFDP compliant:
If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
There are some problems with this approach. What if we have two flashes on the board and both use different reset types? How do we figure out which reset to apply? This applies to the current implementation as well. If there are two flashes then it will issue the reset to both even if one of them does not support/need it.
One would have to choose the NOR manufacturer with care next time. If we'll have to statically define the reset type for both the flashes, there's nothing much we can do. Maybe if you have a gpio reset line connected to the flash you can toggle that instead.
Cheers, ta
Apart of the cmd_ext type, you need to also differentiate between the reset types. So a combination of reset and cmd extension type configs.
I prefer discovering the reset sequence dynamically whenever possible. You can choose to use the configs approach directly, if you don't want to spend time on the dynamic discovery approach, the two approaches will be mutual exclusive anyway.
So yes, I think we should revert this and at least have a better config approach.
Cheers, ta
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/Kconfig | 9 --------- drivers/mtd/spi/spi-nor-core.c | 27 --------------------------- 2 files changed, 36 deletions(-)
diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig index 2d2b71c52d..14800b5e93 100644 --- a/drivers/mtd/spi/Kconfig +++ b/drivers/mtd/spi/Kconfig @@ -103,15 +103,6 @@ config SPI_FLASH_SOFT_RESET Enable support for xSPI Software Reset. It will be used to switch from Octal DTR mode to legacy mode on shutdown and boot (if enabled).
-config SPI_FLASH_SOFT_RESET_ON_BOOT
bool "Perform a Software Reset on boot on flashes that boot in stateful mode"
depends on SPI_FLASH_SOFT_RESET
help
Perform a Software Reset on boot to allow detecting flashes that are
handed to us in Octal DTR mode. Do not enable this config on flashes
that are not supposed to be handed to U-Boot in Octal DTR mode, even
if they _do_ support the Soft Reset sequence.
config SPI_FLASH_BAR bool "SPI flash Bank/Extended address register support" help diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index df66a73495..caf764720c 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3796,33 +3796,6 @@ int spi_nor_scan(struct spi_nor *nor)
nor->setup = spi_nor_default_setup;
-#ifdef CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT
/*
* When the flash is handed to us in a stateful mode like 8D-8D-8D, it
* is difficult to detect the mode the flash is in. One option is to
* read SFDP in all modes and see which one gives the correct "SFDP"
* signature, but not all flashes support SFDP in 8D-8D-8D mode.
*
* Further, even if you detect the mode of the flash via SFDP, you
* still have the problem of actually reading the ID. The Read ID
* command is not standardized across flash vendors. Flashes can have
* different dummy cycles needed for reading the ID. Some flashes even
* expect a 4-byte dummy address with the Read ID command. All this
* information cannot be obtained from the SFDP table.
*
* So, perform a Software Reset sequence before reading the ID and
* initializing the flash. A Soft Reset will bring back the flash in
* its default protocol mode assuming no non-volatile configuration was
* set. This will let us detect the flash even if ROM hands it to us in
* Octal DTR mode.
*
* To accommodate cases where there is more than one flash on a board,
* and only one of them needs a soft reset, failure to reset is not
* made fatal, and we still try to read ID if possible.
*/
spi_nor_soft_reset(nor);
-#endif /* CONFIG_SPI_FLASH_SOFT_RESET_ON_BOOT */
info = spi_nor_read_id(nor); if (IS_ERR_OR_NULL(info)) return -ENOENT;
-- 2.25.1
-- Regards, Pratyush Yadav Texas Instruments Inc.
-- Regards, Pratyush Yadav Texas Instruments Inc.

Hi Tudor,
I am not sure if you have sent a re-roll of this series. I am catching back up on my email backlog.
On 15/11/21 05:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/12/21 3:13 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
On 10/11/21 08:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/9/21 9:26 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
Hi, Pratyush,
Thanks for reviewing the series.
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
If flash supports SFDP, the read ID becomes of minor importance. We can as well not issue the read ID at all.
We need to read the ID so we can apply fixups. One option is to specify
Fixup hooks are there just because the manufacturers can't get their SFDP tables right. Read ID determines the static flags and parameters, which should be used just as a fallback.
Read ID type in device tree.
we may consider this when really needed. Until then we can use the recommended sequence I guess.
Right. And I have two flashes which both need fixups and both have different Read ID commands in 8D-8D-8D mode: Micron MT35X and Cypress S28H. Both of those fortunately have the correct data about how to reset them so we should be good for now.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported
but we can have a dt property or a config option to indicate when to issue readSFDP.
Okay.
commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
in case we can't determine the mode in which the flash is configured, we can adopt other approach, see below.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an
not for every flash, just the ones that we marked as SFDP compliant.
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command
right.
then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
not necessarily.
I can implement such an algorithm but is it really worth the hassle?
I find having such an alg would be of benefit, yes.
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) '''
Below is the approach for the flashes that are not SFDP compliant:
If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
There are some problems with this approach. What if we have two flashes on the board and both use different reset types? How do we figure out which reset to apply? This applies to the current implementation as well. If there are two flashes then it will issue the reset to both even if one of them does not support/need it.
One would have to choose the NOR manufacturer with care next time. If we'll have to statically define the reset type for both the flashes, there's nothing much we can do. Maybe if you have a gpio reset line connected to the flash you can toggle that instead.
Or we can specify the reset type in the device tree? That should neatly solve the problem I believe.

On 12/16/21 8:45 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
I am not sure if you have sent a re-roll of this series. I am catching back up on my email backlog.
I haven't, I was busy with other things, but I will.
On 15/11/21 05:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/12/21 3:13 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
On 10/11/21 08:44AM, Tudor.Ambarus@microchip.com wrote:
On 11/9/21 9:26 PM, Pratyush Yadav wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Hi Tudor,
Hi, Pratyush,
Thanks for reviewing the series.
On 04/11/21 01:49AM, Tudor Ambarus wrote:
This reverts commit 0be8ab1f166844d53477387dc9a1184161ef44ef.
There are multiple reset commands and it was used one at guess,
Correct.
see BFPT[dword(16)]. It is preferable to avoid issuing unsupported commands to a flash. Since there's no config in mainline that actually uses SPI_FLASH_SOFT_RESET_ON_BOOT, remove it entirely until proper
I have been lagging behind on that front. I have some defconfig patches for TI platforms but I did not get around to cleaning them up and posting them upstream. Still, this feature is very much needed on TI platforms.
support is added. One should instead determine the mode in which the flash device is configured, then to parse SFDP to determine the cmd_ext_type and then to issue a READID command if flash identification is really a must. JESD216D-01 proposes an algorithm to try to read the
Firstly, Read ID command is not standardized in 8D-8D-8D mode. For example, Cypress S28 flash family expects 4 address bytes for Read ID in 8D-8D-8D mode whereas Micron MT35 flash family does not. Number of dummy cycles also tends to vary. There is no way to determine these parameters from SFDP.
If flash supports SFDP, the read ID becomes of minor importance. We can as well not issue the read ID at all.
We need to read the ID so we can apply fixups. One option is to specify
Fixup hooks are there just because the manufacturers can't get their SFDP tables right. Read ID determines the static flags and parameters, which should be used just as a fallback.
Read ID type in device tree.
we may consider this when really needed. Until then we can use the recommended sequence I guess.
Right. And I have two flashes which both need fixups and both have different Read ID commands in 8D-8D-8D mode: Micron MT35X and Cypress S28H. Both of those fortunately have the correct data about how to reset them so we should be good for now.
Also, you would have to run the detection algorithm for _every_ flash that SPI NOR supports since we don't really know what we are dealing with at this point. This would include flashes that do not support the Read SFDP command at all. If your goal is to not send unsupported
but we can have a dt property or a config option to indicate when to issue readSFDP.
Okay.
commands to a flash then you won't get very far. On top of that Read SFDP is not mandatory in 8D-8D-8D mode per the xSPI standard so there is no guarantee that the detection algorithm would even work. It _should_ work for most flashes but we will eventually have to deal with the corner cases.
in case we can't determine the mode in which the flash is configured, we can adopt other approach, see below.
In other words, if you drop this then you would have to run the detection algorithm for every flash and see what mode it is in. If an
not for every flash, just the ones that we marked as SFDP compliant.
8D-8D-8D mode flash does support Read SFDP in 8D-8D-8D mode, then you would have to read SFDP, determine the extension and reset type, and then perform the reset. If it does not support the Read SFDP command
right.
then you are left with a non-working flash. You would still probably end up issuing unsupported commands.
not necessarily.
I can implement such an algorithm but is it really worth the hassle?
I find having such an alg would be of benefit, yes.
SFDP signature to determine the mode in which the flash is configured: ''' try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, these typically support SFDP read operation in both 1S-1S-1S mode and 8D-8D-8D mode. If the host controller does not know exactly which protocol mode is used for SFDP in 8D-8D-8D mode, this information can be found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device directly in 8D-8D-8D mode, the host controller may read from address 0, and count the number of dummy clocks required before the SFDP signature is received.) '''
Below is the approach for the flashes that are not SFDP compliant:
If the flash does not support SFDP at all, one should introduce dedicated configs for each reset type and issue just the needed reset command.
There are some problems with this approach. What if we have two flashes on the board and both use different reset types? How do we figure out which reset to apply? This applies to the current implementation as well. If there are two flashes then it will issue the reset to both even if one of them does not support/need it.
One would have to choose the NOR manufacturer with care next time. If we'll have to statically define the reset type for both the flashes, there's nothing much we can do. Maybe if you have a gpio reset line connected to the flash you can toggle that instead.
Or we can specify the reset type in the device tree? That should neatly solve the problem I believe.
It is surely an approach to consider. The downside with it is that people might abuse it, and use it regardless if the reset type is defined in SFDP or not. Do we care?
ta

On 17/12/21 06:27AM, Tudor.Ambarus@microchip.com wrote:
On 12/16/21 8:45 PM, Pratyush Yadav wrote:
> SFDP signature to determine the mode in which the flash is configured: > ''' > try to read the SFDP signature (see 6.1) in 4-4-4 mode, if that fails > try 2-2-2 mode, and if that fails try 1-1-1 mode. For Octal devices, > these typically support SFDP read operation in both 1S-1S-1S mode and > 8D-8D-8D mode. If the host controller does not know exactly which > protocol mode is used for SFDP in 8D-8D-8D mode, this information can be > found by reading SFDP in 1S-1S-1S mode first. (To read an unknown device > directly in 8D-8D-8D mode, the host controller may read from address 0, > and count the number of dummy clocks required before the SFDP signature > is received.) > '''
Below is the approach for the flashes that are not SFDP compliant:
> If the flash does not support SFDP at all, one should introduce dedicated > configs for each reset type and issue just the needed reset command.
There are some problems with this approach. What if we have two flashes on the board and both use different reset types? How do we figure out which reset to apply? This applies to the current implementation as well. If there are two flashes then it will issue the reset to both even if one of them does not support/need it.
One would have to choose the NOR manufacturer with care next time. If we'll have to statically define the reset type for both the flashes, there's nothing much we can do. Maybe if you have a gpio reset line connected to the flash you can toggle that instead.
Or we can specify the reset type in the device tree? That should neatly solve the problem I believe.
It is surely an approach to consider. The downside with it is that people might abuse it, and use it regardless if the reset type is defined in SFDP or not. Do we care?
I am not sure to be honest. Let's think through that once the problem actually comes up I suppose.

It was always hardcoded to SPI_NOR_EXT_REPEAT, while there are flashes that may use an SPI_NOR_EXT_INVERT opcode extension (mx66lm1g45g). Remove the hardcoded value and let flashes use their per flash opcode extension type.
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com --- drivers/mtd/spi/spi-nor-core.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index caf764720c..cbdad335b3 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3685,10 +3685,6 @@ static int spi_nor_soft_reset(struct spi_nor *nor) { struct spi_mem_op op; int ret; - enum spi_nor_cmd_ext ext; - - ext = nor->cmd_ext_type; - nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0), SPI_MEM_OP_NO_DUMMY, @@ -3698,7 +3694,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) ret = spi_mem_exec_op(nor->spi, &op); if (ret) { dev_warn(nor->dev, "Software reset enable failed: %d\n", ret); - goto out; + return ret; }
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0), @@ -3709,7 +3705,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) ret = spi_mem_exec_op(nor->spi, &op); if (ret) { dev_warn(nor->dev, "Software reset failed: %d\n", ret); - goto out; + return ret; }
/* @@ -3719,9 +3715,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) */ udelay(SPI_NOR_SRST_SLEEP_LEN);
-out: - nor->cmd_ext_type = ext; - return ret; + return 0; } #endif /* CONFIG_SPI_FLASH_SOFT_RESET */

On 04/11/21 01:49AM, Tudor Ambarus wrote:
It was always hardcoded to SPI_NOR_EXT_REPEAT, while there are flashes that may use an SPI_NOR_EXT_INVERT opcode extension (mx66lm1g45g). Remove the hardcoded value and let flashes use their per flash opcode extension type.
I suggested this exact patch, along with invert extension support for soft reset on boot in [0]. Once we decide on patch 3/4 this should be good to go.
[0] https://lore.kernel.org/u-boot/20211025194216.mpocldhlu3pdy63h@ti.com/
Signed-off-by: Tudor Ambarus tudor.ambarus@microchip.com
drivers/mtd/spi/spi-nor-core.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index caf764720c..cbdad335b3 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3685,10 +3685,6 @@ static int spi_nor_soft_reset(struct spi_nor *nor) { struct spi_mem_op op; int ret;
enum spi_nor_cmd_ext ext;
ext = nor->cmd_ext_type;
nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 0), SPI_MEM_OP_NO_DUMMY,
@@ -3698,7 +3694,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) ret = spi_mem_exec_op(nor->spi, &op); if (ret) { dev_warn(nor->dev, "Software reset enable failed: %d\n", ret);
goto out;
return ret;
}
op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 0),
@@ -3709,7 +3705,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) ret = spi_mem_exec_op(nor->spi, &op); if (ret) { dev_warn(nor->dev, "Software reset failed: %d\n", ret);
goto out;
return ret;
}
/*
@@ -3719,9 +3715,7 @@ static int spi_nor_soft_reset(struct spi_nor *nor) */ udelay(SPI_NOR_SRST_SLEEP_LEN);
-out:
- nor->cmd_ext_type = ext;
- return ret;
- return 0;
} #endif /* CONFIG_SPI_FLASH_SOFT_RESET */
-- 2.25.1
participants (4)
-
Jagan Teki
-
Pratyush Yadav
-
Tudor Ambarus
-
Tudor.Ambarus@microchip.com