Re: [PATCH v2 2/4] mtd: spi-nor-core: macronix: Add support for mx66lm1g45g

[Pratyush Yadav <p.yadav@ti.com>]

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, &params);
 	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);
-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.