Hi Vignesh,
-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Monday, January 27, 2020 10:48 AM To: Sagar Kadam sagar.kadam@sifive.com; u-boot@lists.denx.de Cc: Paul Walmsley ( Sifive) paul.walmsley@sifive.com; anup.patel@wdc.com; atish.patra@wdc.com; jagan@amarulasolutions.com; Rick Chen rick@andestech.com Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for is25wp256 device
On 24/01/20 11:58 am, Sagar Kadam wrote:
Hello Vignesh,
-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, January 24, 2020 10:24 AM To: Sagar Kadam sagar.kadam@sifive.com; u-boot@lists.denx.de Cc: Paul Walmsley ( Sifive) paul.walmsley@sifive.com; anup.patel@wdc.com; ick@andestech.com; atish.patra@wdc.com; jagan@amarulasolutions.com Subject: Re: [U-Boot Patch v1 6/7] nor: add post bfpt fix handler for is25wp256 device
Hi,
On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
Update vendor id for ISSI flash, enable SFDP as ISSI flash supports it and add support for spi_nor_fixups similar to that done in linux. Flash vendor specific fixups can be registered in spi_nor_ids, and will be called after BFPT parsing to fix any wrong parameter read from SFDP.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
board/sifive/fu540/Kconfig | 1 + drivers/mtd/spi/sf_internal.h | 16 +++++++++++ drivers/mtd/spi/spi-nor-core.c | 63 ++++++++++++++++++++++++++++++++++++++++-- drivers/mtd/spi/spi-nor-ids.c | 7 ++++- include/linux/mtd/spi-nor.h | 1 + 5 files changed, 85 insertions(+), 3 deletions(-)
diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig index 75661f3..d9e4956 100644 --- a/board/sifive/fu540/Kconfig +++ b/board/sifive/fu540/Kconfig @@ -42,6 +42,7 @@ config BOARD_SPECIFIC_OPTIONS # dummy imply SPI imply SPI_SIFIVE imply SPI_FLASH
- imply SPI_FLASH_SFDP_SUPPORT imply SPI_FLASH_ISSI imply MMC imply MMC_SPI
This does not belong to this patch. Its better that this change goes via
SiFive
SoC tree.
Thanks for your inputs. I will move it. Just that I understand this correctly shall I add this config change as a
separate patch apart from the series
or as a different patch containing only this change within this series.
Unless there is a real dependency, it would be great to separate out SPI-NOR related changes to different series, so that it could go via Jagan's SPI tree.
Regards Vignesh
Yes, I will exclude this patch from this series.
-BR, Sagar
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 5c64303..856866f 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -66,8 +66,24 @@ struct flash_info { #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables
*/
#define USE_CLSR BIT(14) /* use CLSR command */ #define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock
via BPR */
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
Change above ifdef to:
#if CONFIG_IS_ENABLED(SPI_FLASH_SFDP_SUPPORT)
throughout the patch to take care of both SPL and U-Boot builds
Sure, will modify this and send it in V2.
- /* Part specific fixup hooks */
- const struct spi_nor_fixups *fixups;
+#endif };
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT +/*
- Declare manufacturer specific fixup handlers that
- can be registered as fixup's in flash info table
- so as to update any wrong/broken SFDP parameter.
- */
+#ifdef CONFIG_SPI_FLASH_ISSI +extern struct spi_nor_fixups is25wp256_fixups; #endif #endif
extern const struct flash_info spi_nor_ids[];
#define JEDEC_MFR(info) ((info)->id[0]) diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 6e7fc23..c55116f 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -296,7 +296,6 @@ static void spi_nor_set_4byte_opcodes(struct
spi_nor *nor,
nor->mtd.erasesize = info->sector_size; break;
- default:
Is this an intentional change?
No this was not intentional, got reverted in 7/7 I will correct it.
break;}
@@ -1800,6 +1799,57 @@ static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT +/**
- struct spi_nor_fixups - SPI NOR fixup hooks
- @post_bfpt: called after the BFPT table has been parsed
- Those hooks can be used to tweak the SPI NOR configuration when
+the SFDP
- table is broken or not available.
- */
+struct spi_nor_fixups {
- int (*post_bfpt)(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params); };+static int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header*bfpt_header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params) {- if (nor->info->fixups && nor->info->fixups->post_bfpt)
return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,params);- return 0;
+}
+static int is25wp256_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header*bfpt_header,const struct sfdp_bfpt *bfpt,struct spi_nor_flash_parameter *params)+{
- /* IS25WP256 supports 4B opcodes, but the BFPT advertises a
* BFPT_DWORD1_ADDRESS_BYTES_3_ONLY address width.* Overwrite the address width advertised by the BFPT.*/- if ((bfpt->dwords[BFPT_DWORD(1)] &
BFPT_DWORD1_ADDRESS_BYTES_MASK) ==
BFPT_DWORD1_ADDRESS_BYTES_3_ONLY)nor->addr_width = 4;- return 0;
+}
+struct spi_nor_fixups is25wp256_fixups = {
- .post_bfpt = is25wp256_post_bfpt_fixups, }; #endif
/**
- spi_nor_parse_bfpt() - read and parse the Basic Flash Parameter
Table.
- @nor: pointer to a 'struct spi_nor'
@@ -1971,7 +2021,13 @@ static int spi_nor_parse_bfpt(struct spi_nor
*nor,
return -EINVAL;}
- return 0;
+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
- err = spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
params);Isn't this function (spi_nor_parse_bfpt()) already under CONFIG_IS_ENABLED(CONFIG_SPI_FLASH_SFDP_SUPPORT)?
Oh yes, thanks for catching this I will remove this guard here.
+#else
- err = 0;
+#endif
- return err;
}
/** @@ -2209,6 +2265,9 @@ static int spi_nor_init_params(struct spi_nor
*nor,
!(info->flags & SPI_NOR_SKIP_SFDP)) { struct spi_nor_flash_parameter sfdp_params;
/* Update flash structure information to nor structure */nor->info = info;Instead, could you update spi_nor_scan() to initialize nor->info before
calling
spi_nor_init_params()?
Yes, I will move it to spi_nor_scan()
Thanks, -Sagar
memcpy(&sfdp_params, params, sizeof(sfdp_params)); if (spi_nor_parse_sfdp(nor, &sfdp_params)) { nor->addr_width = 0;diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c index 973b6f8..5a29c8b 100644 --- a/drivers/mtd/spi/spi-nor-ids.c +++ b/drivers/mtd/spi/spi-nor-ids.c @@ -135,7 +135,12 @@ const struct flash_info spi_nor_ids[] = { { INFO("is25wp128", 0x9d7018, 0, 64 * 1024, 256, SECT_4K | SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ) },
{ INFO("is25wp256", 0x9d7019, 0, 64 * 1024, 512,
SECT_4K | SPI_NOR_DUAL_READ |SPI_NOR_QUAD_READ) },
SECT_4K | SPI_NOR_4B_OPCODES |SPI_NOR_DUAL_READ
| SPI_NOR_QUAD_READ)+#ifdef CONFIG_SPI_FLASH_SFDP_SUPPORT
.fixups = &is25wp256_fixups+#endif
- },
#endif #ifdef CONFIG_SPI_FLASH_MACRONIX /* MACRONIX */ /* Macronix */ diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 1d91177..44b7479 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -23,6 +23,7 @@ #define SNOR_MFR_ST CFI_MFR_ST /* ST Micro <--> Micron
*/
#define SNOR_MFR_MICRON CFI_MFR_MICRON /* ST
Micro <--> Micron */
#define SNOR_MFR_MACRONIX CFI_MFR_MACRONIX +#define SNOR_MFR_ISSI CFI_MFR_PMC #define SNOR_MFR_SPANSION CFI_MFR_AMD #define SNOR_MFR_SST CFI_MFR_SST #define SNOR_MFR_WINBOND 0xef /* Also used by some Spansion
*/
-- Regards Vignesh
-- Regards Vignesh