[U-Boot Patch v1 0/7] Fix currently available support for flash on HiFive Unleashed

Currently device ID for flash mounted on HiFive Unleashed is added to U-Boot. Also there are few patches to go mainline (Thanks to Jagan Tekki and Bin Meng).
This series addresses few issues discussed there: Patch 1: Includes hifive-unleashed-a00-u-boot.dts for building DTB Patch 2: Prints fdt base address which can be used for debugging purpose. Patch 3,4: Number of chip select's for the spi nodes and adds method to claim and release the bus Earlier it was observed that sf probe 0:2/4/6/8 etc.. used to detect the flash device even though it is only connected to chip select 0, with these patches flash is now probed only CS0. Patch 5,7: A workaround is added to make flash device accessible in single bit mode & is tagged with TODO. Patch 6: Introduce spi_nor_fixups method similar to linux to fix wrongly read parameters from SFDP. This series is based on commit 052170c6a043 ("configs: Resync with savedefconfig") with two below mentioned patches from [1] [U-Boot,v2,4/5] riscv: dts: hifive-unleashed-a00: Add -u-boot.dtsi [U-Boot,v2,5/5] sifive: fu540: Enable spi-nor flash support
[1] https://patchwork.ozlabs.org/patch/1177979/
The above series is available for testing here[2] [2] https://github.com/sagsifive/u-boot/tree/dev/sagark/test_spi-nor
================Log for reference===================== => bdinfo boot_params = 0x0000000000000000 DRAM bank = 0x0000000000000000 -> start = 0x0000000080000000 -> size = 0x0000000200000000 relocaddr = 0x00000000fff8c000 reloc off = 0x000000007fd8c000 fdt_blob = 0x00000000ff768530 ->FDT base address ethaddr = 70:b3:d5:92:f2:c8 IP addr = <NULL> baudrate = 115200 bps => fdt addr 0x00000000ff768530 => fdt print /aliases aliases { serial0 = "/soc/serial@10010000"; serial1 = "/soc/serial@10011000"; ethernet0 = "/soc/ethernet@10090000"; spi0 = "/soc/spi@10040000"; -> Alias nodes from patch1 spi2 = "/soc/spi@10050000"; }; =>---------------------------------------------------------------- Full flash memory erase/write/read/validate
=> sf probe 0:0 SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB => mw 0x80600000 0x12348765 0x800000 => sf erase 0x0 0x2000000 SF: 33554432 bytes @ 0x0 Erased: OK => sf write 0x80600000 0x0 0x2000000 device 0 whole chip SF: 33554432 bytes @ 0x0 Written: OK => sf read 0x82600000 0x0 0x2000000 device 0 whole chip SF: 33554432 bytes @ 0x0 Read: OK => cmp.b 0x80600000 0x82600000 0x2000000 Total of 33554432 byte(s) were the same =>---------------------------------------------------------------- Flash detection only at valid Chip select => sf probe 0:0 SF: Detected is25wp256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB => sf probe 0:1 Invalid cs number = 1 Failed to initialize SPI flash at 0:1 (error -22) => sf probe 0:2 Invalid cs number = 2 Failed to initialize SPI flash at 0:2 (error -22) => sf probe 0:4 Invalid cs number = 4 Failed to initialize SPI flash at 0:4 (error -22) => sf probe 0:8 Invalid cs number = 8 Failed to initialize SPI flash at 0:8 (error -22) =>----------------------------------------------------------------
Sagar Shrikant Kadam (7): riscv: dts: include -u-boot for dtb bdinfo: fu540: print fdt descriptor base for debug fu540: dtsi: spi: add num-cs info to dt spi: fu540: add claim and release method to spi-sifive.c spi: fu540: fix: use spi xfer bitlen for spi transfer nor: add post bfpt fix handler for is25wp256 device fu540: spi-nor: modify the flash read and program opcodes
arch/riscv/dts/fu540-c000.dtsi | 3 ++ arch/riscv/dts/hifive-unleashed-a00.dts | 1 + board/sifive/fu540/Kconfig | 1 + cmd/bdinfo.c | 1 + drivers/mtd/spi/sf_internal.h | 16 +++++++ drivers/mtd/spi/spi-nor-core.c | 76 ++++++++++++++++++++++++++++++++- drivers/mtd/spi/spi-nor-ids.c | 7 ++- drivers/spi/spi-sifive.c | 55 +++++++++++++++++++++--- include/linux/mtd/spi-nor.h | 1 + 9 files changed, 153 insertions(+), 8 deletions(-)

Include hifive-unleashed-a00-u-boot.dtsi introduced earlier so that it gets compiled within the dt-blob.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- arch/riscv/dts/hifive-unleashed-a00.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/riscv/dts/hifive-unleashed-a00.dts b/arch/riscv/dts/hifive-unleashed-a00.dts index 88cfcb9..89cc4be 100644 --- a/arch/riscv/dts/hifive-unleashed-a00.dts +++ b/arch/riscv/dts/hifive-unleashed-a00.dts @@ -2,6 +2,7 @@ /* Copyright (c) 2018-2019 SiFive, Inc */
#include "fu540-c000.dtsi" +#include "hifive-unleashed-a00-u-boot.dtsi"
/* Clock frequency (in Hz) of the PCB crystal for rtcclk */ #define RTCCLK_FREQ 1000000

Hello All,
-----Original Message----- From: Sagar Kadam sagar.kadam@sifive.com Sent: Friday, January 24, 2020 1:46 AM To: 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; vigneshr@ti.com; Sagar Kadam sagar.kadam@sifive.com Subject: [U-Boot Patch v1 1/7] riscv: dts: include -u-boot for dtb
Include hifive-unleashed-a00-u-boot.dtsi introduced earlier so that it gets compiled within the dt-blob.
Realised that this patch is not needed. Please drop this patch. Apologies for the same.
Thanks, Sagar
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
arch/riscv/dts/hifive-unleashed-a00.dts | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/riscv/dts/hifive-unleashed-a00.dts b/arch/riscv/dts/hifive- unleashed-a00.dts index 88cfcb9..89cc4be 100644 --- a/arch/riscv/dts/hifive-unleashed-a00.dts +++ b/arch/riscv/dts/hifive-unleashed-a00.dts @@ -2,6 +2,7 @@ /* Copyright (c) 2018-2019 SiFive, Inc */
#include "fu540-c000.dtsi" +#include "hifive-unleashed-a00-u-boot.dtsi"
/* Clock frequency (in Hz) of the PCB crystal for rtcclk */
#define RTCCLK_FREQ 1000000
2.7.4

Add fdt->gd info to bdinfo so that it is useful for debugging and easily use it with fdt util.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- cmd/bdinfo.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index d6a7175..96892b3 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -433,6 +433,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) print_bi_dram(bd); print_num("relocaddr", gd->relocaddr); print_num("reloc off", gd->reloc_off); + print_num("fdt_blob", (ulong)gd->fdt_blob); print_eth_ip_addr(); print_baudrate();

Add number of chip select information to spi nodes which can be used by spi-uclass for error handling if invlaid cs number passed from command.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- arch/riscv/dts/fu540-c000.dtsi | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/riscv/dts/fu540-c000.dtsi b/arch/riscv/dts/fu540-c000.dtsi index afa43c7..9c6ab21 100644 --- a/arch/riscv/dts/fu540-c000.dtsi +++ b/arch/riscv/dts/fu540-c000.dtsi @@ -191,6 +191,7 @@ clocks = <&prci PRCI_CLK_TLCLK>; #address-cells = <1>; #size-cells = <0>; + num-cs = <1>; status = "disabled"; }; qspi1: spi@10041000 { @@ -202,6 +203,7 @@ clocks = <&prci PRCI_CLK_TLCLK>; #address-cells = <1>; #size-cells = <0>; + num-cs = <1>; status = "disabled"; }; qspi2: spi@10050000 { @@ -212,6 +214,7 @@ clocks = <&prci PRCI_CLK_TLCLK>; #address-cells = <1>; #size-cells = <0>; + num-cs = <1>; status = "disabled"; }; eth0: ethernet@10090000 {

Add missing bus claim/release method to spi driver for HiFive Unleashed, and handle num_cs generously so that it generates error if invalid cs number is passed to sf probe.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- drivers/spi/spi-sifive.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
diff --git a/drivers/spi/spi-sifive.c b/drivers/spi/spi-sifive.c index 969bd4b..f990ad6 100644 --- a/drivers/spi/spi-sifive.c +++ b/drivers/spi/spi-sifive.c @@ -186,6 +186,36 @@ static void sifive_spi_tx(struct sifive_spi *spi, const u8 *tx_ptr) writel(tx_data, spi->regs + SIFIVE_SPI_REG_TXDATA); }
+static int sifive_spi_claim_bus(struct udevice *dev) +{ + int ret; + struct udevice *bus = dev->parent; + struct sifive_spi *spi = dev_get_priv(bus); + struct dm_spi_slave_platdata *slave = dev_get_parent_platdata(dev); + + if (!(slave->cs < spi->num_cs)) { + printf("Invalid cs number = %d\n", slave->cs); + return -EINVAL; + } + + sifive_spi_prep_device(spi, slave); + + ret = sifive_spi_set_cs(spi, slave); + if (ret) + return ret; + + return 0; +} + +static int sifive_spi_release_bus(struct udevice *dev) +{ + struct sifive_spi *spi = dev_get_priv(dev->parent); + + sifive_spi_clear_cs(spi); + + return 0; +} + static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen, const void *dout, void *din, unsigned long flags) { @@ -345,6 +375,10 @@ static int sifive_spi_probe(struct udevice *bus) /* init the sifive spi hw */ sifive_spi_init_hw(spi);
+ /* Fetch number of chip selects from DT if present */ + ret = dev_read_u32_default(bus, "num-cs", spi->num_cs); + spi->num_cs = ret; + return 0; }
@@ -353,6 +387,8 @@ static const struct dm_spi_ops sifive_spi_ops = { .set_speed = sifive_spi_set_speed, .set_mode = sifive_spi_set_mode, .cs_info = sifive_spi_cs_info, + .claim_bus = sifive_spi_claim_bus, + .release_bus = sifive_spi_release_bus, };
static const struct udevice_id sifive_spi_ids[] = {

Use bitlen passed by dm_spi_ops rather than using spi-tx/ rx-bus-width from the device tree, to set the mode bits in format register of spi controller present in FU540-C000 SoC on HiFive Unleashed board.
This patch handles a case where controller mode in format register (0x40) is configured as per the width specified in the dt-node of the slave device. For instance if spi-tx- bus-width and spi-rx-bus-width in the flash device node in dt is set to 4 bit mode, the controller gets configured in QUAD mode, whereas the spi nor scan tries to read the JEDEC ID with the reg_proto set to SNOR_PROTO_1_1_1 and fails.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- drivers/spi/spi-sifive.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi-sifive.c b/drivers/spi/spi-sifive.c index f990ad6..038fdb7 100644 --- a/drivers/spi/spi-sifive.c +++ b/drivers/spi/spi-sifive.c @@ -85,6 +85,10 @@ #define SIFIVE_SPI_IP_TXWM BIT(0) #define SIFIVE_SPI_IP_RXWM BIT(1)
+#define SPI_NBITS_SINGLE BIT(0) +#define SPI_NBITS_DUAL BIT(1) +#define SPI_NBITS_QUAD BIT(2) + struct sifive_spi { void *regs; /* base address of the registers */ u32 fifo_depth; @@ -127,7 +131,7 @@ static void sifive_spi_clear_cs(struct sifive_spi *spi) }
static void sifive_spi_prep_transfer(struct sifive_spi *spi, - bool is_rx_xfer, + unsigned int bitlen, bool is_rx_xfer, struct dm_spi_slave_platdata *slave) { u32 cr; @@ -146,12 +150,17 @@ static void sifive_spi_prep_transfer(struct sifive_spi *spi,
/* Number of wires ? */ cr &= ~SIFIVE_SPI_FMT_PROTO_MASK; - if ((slave->mode & SPI_TX_QUAD) || (slave->mode & SPI_RX_QUAD)) + switch (bitlen) { + case SPI_NBITS_QUAD: cr |= SIFIVE_SPI_FMT_PROTO_QUAD; - else if ((slave->mode & SPI_TX_DUAL) || (slave->mode & SPI_RX_DUAL)) + break; + case SPI_NBITS_DUAL: cr |= SIFIVE_SPI_FMT_PROTO_DUAL; - else + break; + default: cr |= SIFIVE_SPI_FMT_PROTO_SINGLE; + break; + }
/* SPI direction in/out ? */ cr &= ~SIFIVE_SPI_FMT_DIR; @@ -235,7 +244,7 @@ static int sifive_spi_xfer(struct udevice *dev, unsigned int bitlen, return ret; }
- sifive_spi_prep_transfer(spi, true, slave); + sifive_spi_prep_transfer(spi, bitlen, true, slave);
remaining_len = bitlen / 8;

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 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 + /* 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: 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); +#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; + 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 */

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.
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 */

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.
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

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
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

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

This patch adds a workaround to change the read/write opcodes from QUAD to single bit mode. Idea here is to enable usage of spi-flash on the board.
TODO: -Enable QUAD mode for spi-flash on HiFive Unleashed A00 board.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com --- drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index c55116f..35d7772 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -295,7 +295,19 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor, nor->erase_opcode = SPINOR_OP_SE; nor->mtd.erasesize = info->sector_size; break; - +#ifdef CONFIG_TARGET_SIFIVE_FU540 + /* + * This flash device does support QUAD bit mode. But + * with tx-rx width specified to 4 bit mode in dt the spi + * driver is unable to access flash device. TODO: Once basic + * operational support is moved to mainline remove this workaround. + */ + case SNOR_MFR_ISSI: + nor->read_opcode = SPINOR_OP_READ_FAST; + nor->program_opcode = SPINOR_OP_PP; + break; +#endif + default: break; }
@@ -2636,6 +2648,7 @@ int spi_nor_scan(struct spi_nor *nor) /* enable 4-byte addressing if the device exceeds 16MiB */ nor->addr_width = 4; if (JEDEC_MFR(info) == SNOR_MFR_SPANSION || + JEDEC_MFR(info) == SNOR_MFR_ISSI || info->flags & SPI_NOR_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info); #else

On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
This patch adds a workaround to change the read/write opcodes from QUAD to single bit mode. Idea here is to enable usage of spi-flash on the board.
TODO: -Enable QUAD mode for spi-flash on HiFive Unleashed A00 board.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index c55116f..35d7772 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -295,7 +295,19 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor, nor->erase_opcode = SPINOR_OP_SE; nor->mtd.erasesize = info->sector_size; break;
+#ifdef CONFIG_TARGET_SIFIVE_FU540
- /*
* This flash device does support QUAD bit mode. But
* with tx-rx width specified to 4 bit mode in dt the spi
* driver is unable to access flash device. TODO: Once basic
* operational support is moved to mainline remove this workaround.
*/
- case SNOR_MFR_ISSI:
nor->read_opcode = SPINOR_OP_READ_FAST;
nor->program_opcode = SPINOR_OP_PP;
break;
+#endif
- default:
NACK.
Sorry, no Target specific #if'dery hacks in spi-nor-core please. Either debug and fix the issue or set tx-rx width to values that actually work in the mainline. You can switch tx-rx width to 4 when TODO is actually done.
break;
}
@@ -2636,6 +2648,7 @@ int spi_nor_scan(struct spi_nor *nor) /* enable 4-byte addressing if the device exceeds 16MiB */ nor->addr_width = 4; if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
info->flags & SPI_NOR_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info);JEDEC_MFR(info) == SNOR_MFR_ISSI ||
#else

Hello Vignesh,
-----Original Message----- From: Vignesh Raghavendra vigneshr@ti.com Sent: Friday, January 24, 2020 10:25 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 7/7] fu540: spi-nor: modify the flash read and program opcodes
On 24/01/20 1:46 am, Sagar Shrikant Kadam wrote:
This patch adds a workaround to change the read/write opcodes from QUAD to single bit mode. Idea here is to enable usage of spi-flash on the board.
TODO: -Enable QUAD mode for spi-flash on HiFive Unleashed A00 board.
Signed-off-by: Sagar Shrikant Kadam sagar.kadam@sifive.com
drivers/mtd/spi/spi-nor-core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index c55116f..35d7772 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -295,7 +295,19 @@ static void spi_nor_set_4byte_opcodes(struct
spi_nor *nor,
nor->erase_opcode = SPINOR_OP_SE; nor->mtd.erasesize = info->sector_size; break;
+#ifdef CONFIG_TARGET_SIFIVE_FU540
- /*
* This flash device does support QUAD bit mode. But
* with tx-rx width specified to 4 bit mode in dt the spi
* driver is unable to access flash device. TODO: Once basic
* operational support is moved to mainline remove this
workaround.
*/
- case SNOR_MFR_ISSI:
nor->read_opcode = SPINOR_OP_READ_FAST;
nor->program_opcode = SPINOR_OP_PP;
break;
+#endif
- default:
NACK.
Sorry, no Target specific #if'dery hacks in spi-nor-core please. Either debug and fix the issue or set tx-rx width to values that actually work in the mainline. You can switch tx-rx width to 4 when TODO is actually done.
Ok, no problem. So till TODO is actually done, I will modify the existing hifive-unleashed-a00.dts present in U-Boot and submit a V2 without these work-arounds.
Thanks & BR, -Sagar
break;
}
@@ -2636,6 +2648,7 @@ int spi_nor_scan(struct spi_nor *nor) /* enable 4-byte addressing if the device exceeds 16MiB */ nor->addr_width = 4; if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
info->flags & SPI_NOR_4B_OPCODES) spi_nor_set_4byte_opcodes(nor, info);JEDEC_MFR(info) == SNOR_MFR_ISSI ||
#else
-- Regards Vignesh
participants (3)
-
Sagar Kadam
-
Sagar Shrikant Kadam
-
Vignesh Raghavendra