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.ka...@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. > 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 > + /* 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? > 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)? > +#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()? > 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