* [U-Boot] [PATCH v2 0/2] Read SFDP parameters and access flash above 16MB
@ 2018-11-13 12:00 Rajat Srivastava
2018-11-13 12:00 ` [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework Rajat Srivastava
2018-11-13 12:00 ` [U-Boot] [PATCH v2 2/2] fsl_qspi: Access flash above 16MB using SFDP Rajat Srivastava
0 siblings, 2 replies; 13+ messages in thread
From: Rajat Srivastava @ 2018-11-13 12:00 UTC (permalink / raw)
To: u-boot
Add functionality to read and parse SFDP parameters to auto-detect
flash size, page size and address width of flash. This enables
flash access above 16MB using 4-byte addressing mode.
Add driver support to get SFDP information of flash and use it to
access flash above 16MB.
---
Changes in v2:
- Make SFDP parsing the default way to read flash parameters.
- Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_SKIP_SFDP to provide an
option to skip SFDP parsing for a particular flash.
- Convert patch-set of 3 patches to 2 patches.
---
Rajat Srivastava (2):
mtd/spi: Add JEDEC SFDP support in SPI framework
fsl_qspi: Access flash above 16MB using SFDP
drivers/mtd/spi/sf_internal.h | 4 +
drivers/mtd/spi/spi_flash.c | 301 +++++++++++++++++++++++++++++++++++++++---
drivers/spi/fsl_qspi.c | 103 ++++++++++++++-
include/spi.h | 2 +
include/spi_flash.h | 120 +++++++++++++++++
5 files changed, 502 insertions(+), 28 deletions(-)
--
2.14.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-13 12:00 [U-Boot] [PATCH v2 0/2] Read SFDP parameters and access flash above 16MB Rajat Srivastava
@ 2018-11-13 12:00 ` Rajat Srivastava
2018-11-15 9:22 ` Vignesh R
2018-11-13 12:00 ` [U-Boot] [PATCH v2 2/2] fsl_qspi: Access flash above 16MB using SFDP Rajat Srivastava
1 sibling, 1 reply; 13+ messages in thread
From: Rajat Srivastava @ 2018-11-13 12:00 UTC (permalink / raw)
To: u-boot
Add support for JESD216 rev B standard JEDEC Serial
Flash Discoverable Parameters (SFDP) tables to
dynamically initialize flash size, page size and
address width of the flash. More parameters can be
added as per requirement.
SFDP parsing is made default but already existing
method for parsing these parameters are not deprecated.
A flag is created to skip SFDP parsing for a particular
flash, if required.
SFDP data lets us auto-detect the addressing mode
supported by the flash which helps us access the
flash using 4-byte address.
Add a new argument in spi_flash_addr() function to create
commands with 3-byte or 4-byte address depending on the
SFDP data read. Add pointer to struct spi_flash in struct
spi_slave so that driver can have access to SFDP data.
Introduce new structures and functions to read and parse
SFDP data. This is loosely based on Linux SFDP framework.
Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
---
Changes in v2:
- Make SFDP parsing the default method.
- Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
provide an option to skip SFDP parsing for a particular flash.
---
drivers/mtd/spi/sf_internal.h | 4 +
drivers/mtd/spi/spi_flash.c | 301 +++++++++++++++++++++++++++++++++++++++---
include/spi.h | 2 +
include/spi_flash.h | 120 +++++++++++++++++
4 files changed, 406 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 26f5c7c995..87f05a88ba 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -26,7 +26,9 @@ enum spi_nor_option_flags {
};
#define SPI_FLASH_3B_ADDR_LEN 3
+#define SPI_FLASH_4B_ADDR_LEN 4
#define SPI_FLASH_CMD_LEN (1 + SPI_FLASH_3B_ADDR_LEN)
+#define SPI_FLASH_CMD_MAX_LEN (1 + SPI_FLASH_4B_ADDR_LEN)
#define SPI_FLASH_16MB_BOUN 0x1000000
/* CFI Manufacture ID's */
@@ -62,6 +64,7 @@ enum spi_nor_option_flags {
#define CMD_READ_STATUS1 0x35
#define CMD_READ_CONFIG 0x35
#define CMD_FLAG_STATUS 0x70
+#define CMD_READ_SFDP 0x5a
/* Bank addr access commands */
#ifdef CONFIG_SPI_FLASH_BAR
@@ -144,6 +147,7 @@ struct spi_flash_info {
#define RD_DUAL BIT(5) /* use Dual Read */
#define RD_QUADIO BIT(6) /* use Quad IO Read */
#define RD_DUALIO BIT(7) /* use Dual IO Read */
+#define SPI_FLASH_SKIP_SFDP BIT(8) /* parse SFDP to get flash info */
#define RD_FULL (RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
};
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index a87bacd4ac..f8bd8fac54 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -20,12 +20,24 @@
#include "sf_internal.h"
-static void spi_flash_addr(u32 addr, u8 *cmd)
+static void spi_flash_addr(struct spi_flash *flash, u32 addr, u8 *cmd)
{
/* cmd[0] is actual command */
- cmd[1] = addr >> 16;
- cmd[2] = addr >> 8;
- cmd[3] = addr >> 0;
+ int i;
+
+ /* Remember to unset addrwd_3_in_use */
+ if (flash->addrwd_3_in_use) {
+ flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
+ debug("SF: addrwd_3_in_use flag needs to be reset to false ");
+ debug("after the intended command is triggered to flash.\n");
+ }
+
+ flash->cmd_len = 1 + flash->addr_width;
+
+ for (i = flash->cmd_len - 1; i > 0; i--) {
+ cmd[i] = addr;
+ addr = addr >> 8;
+ }
}
static int read_sr(struct spi_flash *flash, u8 *rs)
@@ -314,7 +326,7 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
{
u32 erase_size, erase_addr;
- u8 cmd[SPI_FLASH_CMD_LEN];
+ u8 cmd[SPI_FLASH_CMD_MAX_LEN];
int ret = -1;
erase_size = flash->erase_size;
@@ -344,12 +356,13 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
if (ret < 0)
return ret;
#endif
- spi_flash_addr(erase_addr, cmd);
+ spi_flash_addr(flash, erase_addr, cmd);
debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1],
cmd[2], cmd[3], erase_addr);
- ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0);
+ ret = spi_flash_write_common(flash, cmd, flash->cmd_len,
+ NULL, 0);
if (ret < 0) {
debug("SF: erase failed\n");
break;
@@ -373,7 +386,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
unsigned long byte_addr, page_size;
u32 write_addr;
size_t chunk_len, actual;
- u8 cmd[SPI_FLASH_CMD_LEN];
+ u8 cmd[SPI_FLASH_CMD_MAX_LEN];
int ret = -1;
page_size = flash->page_size;
@@ -406,13 +419,13 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
chunk_len = min(chunk_len,
spi->max_write_size - sizeof(cmd));
- spi_flash_addr(write_addr, cmd);
+ spi_flash_addr(flash, write_addr, cmd);
debug("SF: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = %zu\n",
buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
- ret = spi_flash_write_common(flash, cmd, sizeof(cmd),
- buf + actual, chunk_len);
+ ret = spi_flash_write_common(flash, cmd, flash->cmd_len,
+ buf + actual, chunk_len);
if (ret < 0) {
debug("SF: write failed\n");
break;
@@ -487,7 +500,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
return 0;
}
- cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
+ cmdsz = flash->cmd_len + flash->dummy_byte;
u8 cmd[cmdsz];
cmd[0] = flash->read_cmd;
@@ -504,8 +517,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
return log_ret(ret);
bank_sel = flash->bank_curr;
#endif
- remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
- (bank_sel + 1)) - offset;
+ if (flash->cmd_len == SPI_FLASH_CMD_MAX_LEN)
+ remain_len = flash->size - offset;
+ else
+ remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
+ (bank_sel + 1)) - offset;
if (len < remain_len)
read_len = len;
else
@@ -514,7 +530,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
if (spi->max_read_size)
read_len = min(read_len, spi->max_read_size);
- spi_flash_addr(read_addr, cmd);
+ spi_flash_addr(flash, read_addr, cmd);
ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
if (ret < 0) {
@@ -1076,6 +1092,226 @@ static const struct spi_flash_info *spi_flash_read_id(struct spi_flash *flash)
return ERR_PTR(-ENODEV);
}
+/*
+ * Serial Flash Discoverable Parameters (SFDP) parsing
+ */
+
+/*
+ * spi_flash_read_sfdp() - read Serial Flash Discoverable Parameters.
+ * @flash: pointer to a 'struct spi_flash'
+ * @addr: offset in the SFDP area to start reading data from
+ * @len: number of bytes to read
+ * @buf: buffer where the SFDP data are copied into
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_flash_read_sfdp(struct spi_flash *flash, u32 addr, size_t len,
+ void *buf)
+{
+ u8 cmd[4];
+ int ret;
+
+ cmd[0] = CMD_READ_SFDP;
+
+ /*
+ * In case of flashes that support 3 or 4-byte addressing modes
+ * based on command fired, CMD_READ_SFDP is a 3-byte command.
+ * To make sure a 3-byte command is fired, change addrwd_3_in_use
+ * to true and reset it after triggering the command.
+ */
+ flash->addrwd_3_in_use = true;
+ spi_flash_addr(flash, addr, cmd);
+ flash->addrwd_3_in_use = false;
+
+ ret = spi_flash_read_common(flash, cmd, 4, buf, len);
+ if (ret)
+ return -EIO;
+
+ return 0;
+}
+
+/**
+ * spi_flash_parse_bfpt() - read and parse the Basic Flash Parameter Table.
+ * @flash: pointer to a 'struct spi_flash'
+ * @bfpt_header: pointer to the 'struct sfdp_parameter_header' describing
+ * the Basic Flash Parameter Table length and version
+ *
+ * The Basic Flash Parameter Table is the main and only mandatory table as
+ * defined by the SFDP (JESD216) specification.
+ * It provides us with the total size (memory density) of the data array, page
+ * size and the number of address bytes to perform flash operations, among
+ * other information.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_flash_parse_bfpt(struct spi_flash *flash,
+ const struct sfdp_parameter_header *bfpt_header)
+{
+ struct sfdp_bfpt bfpt;
+ size_t len;
+ int i, err;
+ u32 addr;
+
+ /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
+ if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
+ return -EINVAL;
+
+ /* Read the Basic Flash Parameter Table. */
+ len = min_t(size_t, sizeof(bfpt),
+ bfpt_header->length * sizeof(u32));
+ addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
+ memset(&bfpt, 0, sizeof(bfpt));
+ err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
+ if (err < 0)
+ return err;
+
+ /* Fix endianness of the BFPT DWORDs. */
+ for (i = 0; i < BFPT_DWORD_MAX; i++)
+ bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
+
+ /* Number of address bytes. */
+ switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
+ case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
+ flash->addr_width = 3;
+ break;
+
+ case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
+ printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
+ printf("mode on command\n");
+ /*
+ * By default, 4-byte addressing mode is set.
+ * To enforce 3-byte addressing mode, set addrwd_3_in_use flag
+ * in struct spi_flash for every command.
+ */
+ flash->addr_width = 4;
+ break;
+
+ case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
+ flash->addr_width = 4;
+ break;
+
+ default:
+ break;
+ }
+
+ /* Flash Memory Density (in bits). */
+ flash->size = bfpt.dwords[BFPT_DWORD(2)];
+ if (flash->size & BIT(31)) {
+ flash->size &= ~BIT(31);
+
+ /*
+ * Prevent overflows on flash->size. Anyway, a NOR of 2^64
+ * bits is unlikely to exist so this error probably means
+ * the BFPT we are reading is corrupted/wrong.
+ */
+ if (flash->size > 63)
+ return -EINVAL;
+
+ flash->size = 1ULL << flash->size;
+ } else {
+ flash->size++;
+ }
+ flash->size >>= 3; /* Convert to bytes. */
+
+ /* Stop here if not JESD216 rev A or later. */
+ if (bfpt_header->length < BFPT_DWORD_MAX)
+ return 0;
+
+ /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
+ flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
+ flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
+ flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
+ flash->page_size = 1U << flash->page_size;
+
+ return 0;
+}
+
+/*
+ * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
+ * @flash: pointer to a 'struct spi_flash'
+ *
+ * The Serial Flash Discoverable Parameters are described by the JEDEC JESD216
+ * specification. This is a standard which tends to supported by almost all
+ * (Q)SPI memory manufacturers. Those hard-coded tables allow us to learn at
+ * runtime the main parameters needed to perform basic SPI flash operations.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_flash_parse_sfdp(struct spi_flash *flash)
+{
+ const struct sfdp_parameter_header *param_header, *bfpt_header;
+ struct sfdp_parameter_header *param_headers = NULL;
+ struct sfdp_header header;
+ size_t psize;
+ int i, err;
+
+ /* Get the SFDP header. */
+ err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
+ if (err < 0)
+ return err;
+
+ /* Check the SFDP header version. */
+ if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
+ header.major != SFDP_JESD216_MAJOR)
+ return -EINVAL;
+
+ /*
+ * Verify that the first and only mandatory parameter header is a
+ * Basic Flash Parameter Table header as specified in JESD216.
+ */
+ bfpt_header = &header.bfpt_header;
+ if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
+ bfpt_header->major != SFDP_JESD216_MAJOR)
+ return -EINVAL;
+
+ /*
+ * Allocate memory then read all parameter headers with a single
+ * Read SFDP command. These parameter headers will actually be parsed
+ * twice: a first time to get the latest revision of the basic flash
+ * parameter table, then a second time to handle the supported optional
+ * tables.
+ * Hence we read the parameter headers once for all to reduce the
+ * processing time
+ */
+ if (header.nph) {
+ psize = header.nph * sizeof(*param_headers);
+
+ param_headers = malloc(psize);
+ if (!param_headers)
+ return -ENOMEM;
+
+ err = spi_flash_read_sfdp(flash, sizeof(header),
+ psize, param_headers);
+ if (err < 0) {
+ dev_err(dev, "failed to read SFDP parameter headers\n");
+ goto exit;
+ }
+ }
+
+ /*
+ * Check other parameter headers to get the latest revision of
+ * the basic flash parameter table.
+ */
+ for (i = 0; i < header.nph; i++) {
+ param_header = ¶m_headers[i];
+
+ if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID &&
+ param_header->major == SFDP_JESD216_MAJOR &&
+ (param_header->minor > bfpt_header->minor ||
+ (param_header->minor == bfpt_header->minor &&
+ param_header->length > bfpt_header->length)))
+ bfpt_header = param_header;
+ }
+
+ err = spi_flash_parse_bfpt(flash, bfpt_header);
+ if (err)
+ goto exit;
+
+exit:
+ free(param_headers);
+ return err;
+}
+
static int set_quad_mode(struct spi_flash *flash,
const struct spi_flash_info *info)
{
@@ -1130,7 +1366,7 @@ int spi_flash_scan(struct spi_flash *flash)
{
struct spi_slave *spi = flash->spi;
const struct spi_flash_info *info = NULL;
- int ret;
+ int ret, sfdp = 0;
info = spi_flash_read_id(flash);
if (IS_ERR_OR_NULL(info))
@@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
}
#endif
+ spi->flash = flash;
+ flash->addrwd_3_in_use = false;
+
+ /* Read Serial Flash Discoverable Parameters and initialize
+ * the following parameters of flash:
+ * 1. Flash size
+ * 2. Page size
+ * 3. Address width to be used for commands
+ */
+ if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
+ flash->size = 0;
+ sfdp = spi_flash_parse_sfdp(flash);
+ if (sfdp < 0)
+ debug("Unable to get SFDP information\n");
+ }
+
/* Compute the flash size */
flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
- flash->page_size = info->page_size;
+ if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
+ flash->page_size = info->page_size;
+ flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
+ }
/*
* The Spansion S25FS512S, S25FL032P and S25FL064P have 256b pages,
* yet use the 0x4d00 Extended JEDEC code. The rest of the Spansion
@@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash *flash)
}
flash->page_size <<= flash->shift;
flash->sector_size = info->sector_size << flash->shift;
- flash->size = flash->sector_size * info->n_sectors << flash->shift;
+ if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
+ flash->size = flash->sector_size * info->n_sectors <<
+ flash->shift;
+ }
#ifdef CONFIG_SF_DUAL_FLASH
if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
flash->size <<= 1;
@@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
#endif
#ifndef CONFIG_SPI_FLASH_BAR
- if (((flash->dual_flash == SF_SINGLE_FLASH) &&
- (flash->size > SPI_FLASH_16MB_BOUN)) ||
- ((flash->dual_flash > SF_SINGLE_FLASH) &&
+ if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
+ (flash->dual_flash == SF_SINGLE_FLASH &&
+ flash->size > SPI_FLASH_16MB_BOUN) ||
+ (flash->dual_flash > SF_SINGLE_FLASH &&
(flash->size > SPI_FLASH_16MB_BOUN << 1))) {
puts("SF: Warning - Only lower 16MiB accessible,");
puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
diff --git a/include/spi.h b/include/spi.h
index 938627bc01..7189e60581 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -10,6 +10,7 @@
#define _SPI_H_
#include <common.h>
+#include <spi_flash.h>
/* SPI mode flags */
#define SPI_CPHA BIT(0) /* clock phase */
@@ -103,6 +104,7 @@ struct spi_slave {
unsigned int bus;
unsigned int cs;
#endif
+ struct spi_flash *flash;
uint mode;
unsigned int wordlen;
unsigned int max_read_size;
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 0ec98fb55d..6558a4a1d5 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -47,6 +47,9 @@ struct spi_slave;
* @read_cmd: Read cmd - Array Fast, Extn read and quad read.
* @write_cmd: Write cmd - page and quad program.
* @dummy_byte: Dummy cycles for read operation.
+ * @cmd_len: Total length of command.
+ * @addr_width: Number of address bytes.
+ * @addrwd_3_in_use: Flag to send command in 3-byte address mode.
* @memory_map: Address of read-only SPI flash access
* @flash_lock: lock a region of the SPI Flash
* @flash_unlock: unlock a region of the SPI Flash
@@ -82,6 +85,9 @@ struct spi_flash {
u8 read_cmd;
u8 write_cmd;
u8 dummy_byte;
+ u8 cmd_len;
+ u8 addr_width;
+ bool addrwd_3_in_use;
void *memory_map;
@@ -107,6 +113,120 @@ struct spi_flash {
#endif
};
+/*
+ * Serial Flash Discoverable Parameter Headers
+ */
+struct sfdp_parameter_header {
+ u8 id_lsb;
+ u8 minor;
+ u8 major;
+ u8 length; /* in double words */
+ u8 parameter_table_pointer[3]; /* byte address */
+ u8 id_msb;
+};
+
+struct sfdp_header {
+ u32 signature; /* Ox50444653U <=> "SFDP" */
+ u8 minor;
+ u8 major;
+ u8 nph; /* 0-base number of parameter headers */
+ u8 unused;
+
+ /* Basic Flash Parameter Table. */
+ struct sfdp_parameter_header bfpt_header;
+};
+
+#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
+#define SFDP_PARAM_HEADER_PTP(p) \
+ (((p)->parameter_table_pointer[2] << 16) | \
+ ((p)->parameter_table_pointer[1] << 8) | \
+ ((p)->parameter_table_pointer[0] << 0))
+
+#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
+#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
+
+#define SFDP_SIGNATURE 0x50444653U
+#define SFDP_JESD216_MAJOR 1
+#define SFDP_JESD216_MINOR 0
+#define SFDP_JESD216A_MINOR 5
+#define SFDP_JESD216B_MINOR 6
+
+/* Basic Flash Parameter Table */
+
+/*
+ * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
+ * They are indexed from 1 but C arrays are indexed from 0.
+ */
+#define BFPT_DWORD(i) ((i) - 1)
+#define BFPT_DWORD_MAX 16
+
+/* The first version of JESB216 defined only 9 DWORDs. */
+#define BFPT_DWORD_MAX_JESD216 9
+
+/* 1st DWORD. */
+#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
+#define BFPT_DWORD1_ADDRESS_BYTES_MASK GENMASK(18, 17)
+#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
+#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
+#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
+#define BFPT_DWORD1_DTR BIT(19)
+#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
+#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
+#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
+
+/* 5th DWORD. */
+#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
+#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
+
+/* 11th DWORD. */
+#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
+#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7, 4)
+
+/* 15th DWORD. */
+
+/*
+ * (from JESD216 rev B)
+ * Quad Enable Requirements (QER):
+ * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
+ * reads based on instruction. DQ3/HOLD# functions are hold during
+ * instruction phase.
+ * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
+ * two data bytes where bit 1 of the second byte is one.
+ * [...]
+ * Writing only one byte to the status register has the side-effect of
+ * clearing status register 2, including the QE bit. The 100b code is
+ * used if writing one byte to the status register does not modify
+ * status register 2.
+ * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
+ * one data byte where bit 6 is one.
+ * [...]
+ * - 011b: QE is bit 7 of status register 2. It is set via Write status
+ * register 2 instruction 3Eh with one data byte where bit 7 is one.
+ * [...]
+ * The status register 2 is read using instruction 3Fh.
+ * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
+ * two data bytes where bit 1 of the second byte is one.
+ * [...]
+ * In contrast to the 001b code, writing one byte to the status
+ * register does not modify status register 2.
+ * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
+ * Read Status instruction 05h. Status register2 is read using
+ * instruction 35h. QE is set via Writ Status instruction 01h with
+ * two data bytes where bit 1 of the second byte is one.
+ * [...]
+ */
+#define BFPT_DWORD15_QER_MASK GENMASK(22, 20)
+#define BFPT_DWORD15_QER_NONE (0x0UL << 20) /* Micron */
+#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
+#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /* Macronix */
+#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
+#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
+#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
+
+struct sfdp_bfpt {
+ u32 dwords[BFPT_DWORD_MAX];
+};
+
struct dm_spi_flash_ops {
int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
int (*write)(struct udevice *dev, u32 offset, size_t len,
--
2.14.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 2/2] fsl_qspi: Access flash above 16MB using SFDP
2018-11-13 12:00 [U-Boot] [PATCH v2 0/2] Read SFDP parameters and access flash above 16MB Rajat Srivastava
2018-11-13 12:00 ` [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework Rajat Srivastava
@ 2018-11-13 12:00 ` Rajat Srivastava
1 sibling, 0 replies; 13+ messages in thread
From: Rajat Srivastava @ 2018-11-13 12:00 UTC (permalink / raw)
To: u-boot
Add functionality to read SFDP parameters in fsl_qspi driver.
Also, use the address width information from SFDP to enable
flash access above 16 MB.
Introduce a way to access parent structure by adding pointer
to struct spi_slave in struct fsl_qspi_priv.
Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
---
Changes in v2:
- none
---
drivers/spi/fsl_qspi.c | 103 +++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 96 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/fsl_qspi.c b/drivers/spi/fsl_qspi.c
index 1598c4f698..615f36e351 100644
--- a/drivers/spi/fsl_qspi.c
+++ b/drivers/spi/fsl_qspi.c
@@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define TX_BUFFER_SIZE 0x40
#endif
-#define OFFSET_BITS_MASK GENMASK(23, 0)
+#define SET_BITS_MASK(X) GENMASK(X, 0)
#define FLASH_STATUS_WEL 0x02
@@ -47,6 +47,7 @@ DECLARE_GLOBAL_DATA_PTR;
#endif
#define SEQID_WRAR 13
#define SEQID_RDAR 14
+#define SEQID_RDSFDP 15
/* QSPI CMD */
#define QSPI_CMD_PP 0x02 /* Page program (up to 256 bytes) */
@@ -57,6 +58,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define QSPI_CMD_CHIP_ERASE 0xc7 /* Erase whole flash chip */
#define QSPI_CMD_SE 0xd8 /* Sector erase (usually 64KiB) */
#define QSPI_CMD_RDID 0x9f /* Read JEDEC ID */
+#define QSPI_CMD_RDSFDP 0x5a /* Read SFDP parameters from flash */
/* Used for Micron, winbond and Macronix flashes */
#define QSPI_CMD_WREAR 0xc5 /* EAR register write */
@@ -132,6 +134,7 @@ struct fsl_qspi_priv {
u32 flash_num;
u32 num_chipselect;
struct fsl_qspi_regs *regs;
+ void *spi_slave;
};
@@ -363,6 +366,19 @@ static void qspi_set_lut(struct fsl_qspi_priv *priv)
qspi_write32(priv->flags, ®s->lut[lut_base + 1],
OPRND0(1) | PAD0(LUT_PAD1) | INSTR0(LUT_WRITE));
+ /* Read SFDP information */
+ lut_base = SEQID_RDSFDP * 4;
+ qspi_write32(priv->flags, ®s->lut[lut_base],
+ OPRND0(QSPI_CMD_RDSFDP) | PAD0(LUT_PAD1) |
+ INSTR0(LUT_CMD) | OPRND1(ADDR24BIT) |
+ PAD1(LUT_PAD1) | INSTR1(LUT_ADDR));
+ qspi_write32(priv->flags, ®s->lut[lut_base + 1],
+ OPRND0(8) | PAD0(LUT_PAD1) | INSTR0(LUT_DUMMY) |
+ OPRND1(RX_BUFFER_SIZE) | PAD1(LUT_PAD1) |
+ INSTR1(LUT_READ));
+ qspi_write32(priv->flags, ®s->lut[lut_base + 2], 0);
+ qspi_write32(priv->flags, ®s->lut[lut_base + 3], 0);
+
/* Lock the LUT */
qspi_write32(priv->flags, ®s->lutkey, LUT_KEY_VALUE);
qspi_write32(priv->flags, ®s->lckcr, QSPI_LCKCR_LOCK);
@@ -562,6 +578,61 @@ static void qspi_op_rdid(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len)
qspi_write32(priv->flags, ®s->mcr, mcr_reg);
}
+static void qspi_op_rdsfdp(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len)
+{
+ struct fsl_qspi_regs *regs = priv->regs;
+ u32 mcr_reg, data;
+ int i, size;
+ u32 to_or_from;
+ u32 seqid;
+
+ seqid = SEQID_RDSFDP;
+
+ mcr_reg = qspi_read32(priv->flags, ®s->mcr);
+ qspi_write32(priv->flags, ®s->mcr,
+ QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
+ QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
+ qspi_write32(priv->flags, ®s->rbct, QSPI_RBCT_RXBRD_USEIPS);
+
+ to_or_from = priv->sf_addr + priv->cur_amba_base;
+
+ while (len > 0) {
+ WATCHDOG_RESET();
+
+ qspi_write32(priv->flags, ®s->sfar, to_or_from);
+
+ size = (len > RX_BUFFER_SIZE) ?
+ RX_BUFFER_SIZE : len;
+
+ qspi_write32(priv->flags, ®s->ipcr,
+ (seqid << QSPI_IPCR_SEQID_SHIFT) |
+ size);
+ while (qspi_read32(priv->flags, ®s->sr) & QSPI_SR_BUSY_MASK)
+ ;
+
+ to_or_from += size;
+ len -= size;
+
+ i = 0;
+ while ((size < RX_BUFFER_SIZE) && (size > 0)) {
+ data = qspi_read32(priv->flags, ®s->rbdr[i]);
+ data = qspi_endian_xchg(data);
+ if (size < 4)
+ memcpy(rxbuf, &data, size);
+ else
+ memcpy(rxbuf, &data, 4);
+ rxbuf++;
+ size -= 4;
+ i++;
+ }
+ qspi_write32(priv->flags, ®s->mcr,
+ qspi_read32(priv->flags, ®s->mcr) |
+ QSPI_MCR_CLR_RXF_MASK);
+ }
+
+ qspi_write32(priv->flags, ®s->mcr, mcr_reg);
+}
+
/* If not use AHB read, read data from ip interface */
static void qspi_op_read(struct fsl_qspi_priv *priv, u32 *rxbuf, u32 len)
{
@@ -772,14 +843,25 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen,
{
u32 bytes = DIV_ROUND_UP(bitlen, 8);
static u32 wr_sfaddr;
- u32 txbuf;
+ u32 txbuf, bits_mask;
+ struct spi_flash *flash;
+
+ flash = ((struct spi_slave *)(priv->spi_slave))->flash;
WATCHDOG_RESET();
+ if (flash->cmd_len == 5 && flash->size > SZ_16M)
+ bits_mask = SET_BITS_MASK(27);
+ else
+ bits_mask = SET_BITS_MASK(23);
+
if (dout) {
if (flags & SPI_XFER_BEGIN) {
priv->cur_seqid = *(u8 *)dout;
- memcpy(&txbuf, dout, 4);
+ if (flash->size > SZ_16M && bytes > 4)
+ memcpy(&txbuf, dout + 1, 4);
+ else
+ memcpy(&txbuf, dout, 4);
}
if (flags == SPI_XFER_END) {
@@ -790,20 +872,21 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen,
if (priv->cur_seqid == QSPI_CMD_FAST_READ ||
priv->cur_seqid == QSPI_CMD_RDAR) {
- priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;
+ priv->sf_addr = swab32(txbuf) & bits_mask;
} else if ((priv->cur_seqid == QSPI_CMD_SE) ||
(priv->cur_seqid == QSPI_CMD_BE_4K)) {
- priv->sf_addr = swab32(txbuf) & OFFSET_BITS_MASK;
+ priv->sf_addr = swab32(txbuf) & bits_mask;
qspi_op_erase(priv);
} else if (priv->cur_seqid == QSPI_CMD_PP ||
priv->cur_seqid == QSPI_CMD_WRAR) {
- wr_sfaddr = swab32(txbuf) & OFFSET_BITS_MASK;
+ wr_sfaddr = swab32(txbuf) & bits_mask;
} else if ((priv->cur_seqid == QSPI_CMD_BRWR) ||
(priv->cur_seqid == QSPI_CMD_WREAR)) {
#ifdef CONFIG_SPI_FLASH_BAR
wr_sfaddr = 0;
#endif
- }
+ } else if (priv->cur_seqid == QSPI_CMD_RDSFDP)
+ priv->sf_addr = swab32(txbuf) & bits_mask;
}
if (din) {
@@ -819,6 +902,8 @@ int qspi_xfer(struct fsl_qspi_priv *priv, unsigned int bitlen,
qspi_op_rdid(priv, din, bytes);
else if (priv->cur_seqid == QSPI_CMD_RDSR)
qspi_op_rdsr(priv, din, bytes);
+ else if (priv->cur_seqid == QSPI_CMD_RDSFDP)
+ qspi_op_rdsfdp(priv, din, bytes);
#ifdef CONFIG_SPI_FLASH_BAR
else if ((priv->cur_seqid == QSPI_CMD_BRRD) ||
(priv->cur_seqid == QSPI_CMD_RDEAR)) {
@@ -1044,9 +1129,13 @@ static int fsl_qspi_xfer(struct udevice *dev, unsigned int bitlen,
{
struct fsl_qspi_priv *priv;
struct udevice *bus;
+ struct spi_slave *slave;
bus = dev->parent;
priv = dev_get_priv(bus);
+ slave = dev_get_parent_priv(dev);
+
+ priv->spi_slave = slave;
return qspi_xfer(priv, bitlen, dout, din, flags);
}
--
2.14.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-13 12:00 ` [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework Rajat Srivastava
@ 2018-11-15 9:22 ` Vignesh R
2018-11-16 9:40 ` Rajat Srivastava
0 siblings, 1 reply; 13+ messages in thread
From: Vignesh R @ 2018-11-15 9:22 UTC (permalink / raw)
To: u-boot
Hi Rajat,
On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> Add support for JESD216 rev B standard JEDEC Serial
> Flash Discoverable Parameters (SFDP) tables to
> dynamically initialize flash size, page size and
> address width of the flash. More parameters can be
> added as per requirement.
> SFDP parsing is made default but already existing
> method for parsing these parameters are not deprecated.
> A flag is created to skip SFDP parsing for a particular
> flash, if required.
>
> SFDP data lets us auto-detect the addressing mode
> supported by the flash which helps us access the
> flash using 4-byte address.
>
> Add a new argument in spi_flash_addr() function to create
> commands with 3-byte or 4-byte address depending on the
> SFDP data read. Add pointer to struct spi_flash in struct
> spi_slave so that driver can have access to SFDP data.
>
> Introduce new structures and functions to read and parse
> SFDP data. This is loosely based on Linux SFDP framework.
>
> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> ---
> Changes in v2:
> - Make SFDP parsing the default method.
> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
> provide an option to skip SFDP parsing for a particular flash.
> ---
[...]
> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> + const struct sfdp_parameter_header *bfpt_header)
> +{
> + struct sfdp_bfpt bfpt;
> + size_t len;
> + int i, err;
> + u32 addr;
> +
> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> + return -EINVAL;
> +
> + /* Read the Basic Flash Parameter Table. */
> + len = min_t(size_t, sizeof(bfpt),
> + bfpt_header->length * sizeof(u32));
> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> + memset(&bfpt, 0, sizeof(bfpt));
> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> + if (err < 0)
> + return err;
> +
> + /* Fix endianness of the BFPT DWORDs. */
> + for (i = 0; i < BFPT_DWORD_MAX; i++)
> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> +
> + /* Number of address bytes. */
> + switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> + flash->addr_width = 3;
> + break;
> +
> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> + printf("mode on command\n");
> + /*
> + * By default, 4-byte addressing mode is set.
> + * To enforce 3-byte addressing mode, set addrwd_3_in_use flag
> + * in struct spi_flash for every command.
> + */
> + flash->addr_width = 4;
> + break;
> +
> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> + flash->addr_width = 4;
> + break;
> +
I see your are updating flash->addr_width to 4 bytes here. But I don't
see any code in this patch that changes flash->read_cmd (and write/erase
cmds) to use 4B addressing opcodes. Also, I dont see any code that
bypasses BAR register configuration when CONFIG_SPI_FLASH_BAR is defined
and SFDP is available.
This patch will most certainly break SPI controller drivers(like
cadence_qspi.c) that expect sf layer to provide correct read opcode and
address width information, since there will be mismatch wrt opcode used
and number of address byte sent.
Not sure how this patch was tested, but I guess fsl_qspi modifies read
opcode internal to the driver and gets away with it.
Please modify the patch to update read_cmd/erase_cmd/write_cmd to use 4
byte addressing opcodes
Regards
Vigensh
> + default:
> + break;
> + }
> +
> + /* Flash Memory Density (in bits). */
> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> + if (flash->size & BIT(31)) {
> + flash->size &= ~BIT(31);
> +
> + /*
> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> + * bits is unlikely to exist so this error probably means
> + * the BFPT we are reading is corrupted/wrong.
> + */
> + if (flash->size > 63)
> + return -EINVAL;
> +
> + flash->size = 1ULL << flash->size;
> + } else {
> + flash->size++;
> + }
> + flash->size >>= 3; /* Convert to bytes. */
> +
> + /* Stop here if not JESD216 rev A or later. */
> + if (bfpt_header->length < BFPT_DWORD_MAX)
> + return 0;
> +
> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> + flash->page_size = 1U << flash->page_size;
> +
> + return 0;
> +}
> +
> +/*
> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
> + * @flash: pointer to a 'struct spi_flash'
> + *
> + * The Serial Flash Discoverable Parameters are described by the JEDEC JESD216
> + * specification. This is a standard which tends to supported by almost all
> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us to learn at
> + * runtime the main parameters needed to perform basic SPI flash operations.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_flash_parse_sfdp(struct spi_flash *flash)
> +{
> + const struct sfdp_parameter_header *param_header, *bfpt_header;
> + struct sfdp_parameter_header *param_headers = NULL;
> + struct sfdp_header header;
> + size_t psize;
> + int i, err;
> +
> + /* Get the SFDP header. */
> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> + if (err < 0)
> + return err;
> +
> + /* Check the SFDP header version. */
> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> + header.major != SFDP_JESD216_MAJOR)
> + return -EINVAL;
> +
> + /*
> + * Verify that the first and only mandatory parameter header is a
> + * Basic Flash Parameter Table header as specified in JESD216.
> + */
> + bfpt_header = &header.bfpt_header;
> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> + bfpt_header->major != SFDP_JESD216_MAJOR)
> + return -EINVAL;
> +
> + /*
> + * Allocate memory then read all parameter headers with a single
> + * Read SFDP command. These parameter headers will actually be parsed
> + * twice: a first time to get the latest revision of the basic flash
> + * parameter table, then a second time to handle the supported optional
> + * tables.
> + * Hence we read the parameter headers once for all to reduce the
> + * processing time
> + */
> + if (header.nph) {
> + psize = header.nph * sizeof(*param_headers);
> +
> + param_headers = malloc(psize);
> + if (!param_headers)
> + return -ENOMEM;
> +
> + err = spi_flash_read_sfdp(flash, sizeof(header),
> + psize, param_headers);
> + if (err < 0) {
> + dev_err(dev, "failed to read SFDP parameter headers\n");
> + goto exit;
> + }
> + }
> +
> + /*
> + * Check other parameter headers to get the latest revision of
> + * the basic flash parameter table.
> + */
> + for (i = 0; i < header.nph; i++) {
> + param_header = ¶m_headers[i];
> +
> + if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID &&
> + param_header->major == SFDP_JESD216_MAJOR &&
> + (param_header->minor > bfpt_header->minor ||
> + (param_header->minor == bfpt_header->minor &&
> + param_header->length > bfpt_header->length)))
> + bfpt_header = param_header;
> + }
> +
> + err = spi_flash_parse_bfpt(flash, bfpt_header);
> + if (err)
> + goto exit;
> +
> +exit:
> + free(param_headers);
> + return err;
> +}
> +
> static int set_quad_mode(struct spi_flash *flash,
> const struct spi_flash_info *info)
> {
> @@ -1130,7 +1366,7 @@ int spi_flash_scan(struct spi_flash *flash)
> {
> struct spi_slave *spi = flash->spi;
> const struct spi_flash_info *info = NULL;
> - int ret;
> + int ret, sfdp = 0;
>
> info = spi_flash_read_id(flash);
> if (IS_ERR_OR_NULL(info))
> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> }
> #endif
>
> + spi->flash = flash;
> + flash->addrwd_3_in_use = false;
> +
> + /* Read Serial Flash Discoverable Parameters and initialize
> + * the following parameters of flash:
> + * 1. Flash size
> + * 2. Page size
> + * 3. Address width to be used for commands
> + */
> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> + flash->size = 0;
> + sfdp = spi_flash_parse_sfdp(flash);
> + if (sfdp < 0)
> + debug("Unable to get SFDP information\n");
> + }
> +
> /* Compute the flash size */
> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
> - flash->page_size = info->page_size;
> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> + flash->page_size = info->page_size;
> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> + }
> /*
> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b pages,
> * yet use the 0x4d00 Extended JEDEC code. The rest of the Spansion
> @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash *flash)
> }
> flash->page_size <<= flash->shift;
> flash->sector_size = info->sector_size << flash->shift;
> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> + flash->size = flash->sector_size * info->n_sectors <<
> + flash->shift;
> + }
> #ifdef CONFIG_SF_DUAL_FLASH
> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> flash->size <<= 1;
> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> #endif
>
> #ifndef CONFIG_SPI_FLASH_BAR
> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> + (flash->dual_flash == SF_SINGLE_FLASH &&
> + flash->size > SPI_FLASH_16MB_BOUN) ||
> + (flash->dual_flash > SF_SINGLE_FLASH &&
> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> puts("SF: Warning - Only lower 16MiB accessible,");
> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
> diff --git a/include/spi.h b/include/spi.h
> index 938627bc01..7189e60581 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -10,6 +10,7 @@
> #define _SPI_H_
>
> #include <common.h>
> +#include <spi_flash.h>
>
> /* SPI mode flags */
> #define SPI_CPHA BIT(0) /* clock phase */
> @@ -103,6 +104,7 @@ struct spi_slave {
> unsigned int bus;
> unsigned int cs;
> #endif
> + struct spi_flash *flash;
> uint mode;
> unsigned int wordlen;
> unsigned int max_read_size;
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 0ec98fb55d..6558a4a1d5 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -47,6 +47,9 @@ struct spi_slave;
> * @read_cmd: Read cmd - Array Fast, Extn read and quad read.
> * @write_cmd: Write cmd - page and quad program.
> * @dummy_byte: Dummy cycles for read operation.
> + * @cmd_len: Total length of command.
> + * @addr_width: Number of address bytes.
> + * @addrwd_3_in_use: Flag to send command in 3-byte address mode.
> * @memory_map: Address of read-only SPI flash access
> * @flash_lock: lock a region of the SPI Flash
> * @flash_unlock: unlock a region of the SPI Flash
> @@ -82,6 +85,9 @@ struct spi_flash {
> u8 read_cmd;
> u8 write_cmd;
> u8 dummy_byte;
> + u8 cmd_len;
> + u8 addr_width;
> + bool addrwd_3_in_use;
>
> void *memory_map;
>
> @@ -107,6 +113,120 @@ struct spi_flash {
> #endif
> };
>
> +/*
> + * Serial Flash Discoverable Parameter Headers
> + */
> +struct sfdp_parameter_header {
> + u8 id_lsb;
> + u8 minor;
> + u8 major;
> + u8 length; /* in double words */
> + u8 parameter_table_pointer[3]; /* byte address */
> + u8 id_msb;
> +};
> +
> +struct sfdp_header {
> + u32 signature; /* Ox50444653U <=> "SFDP" */
> + u8 minor;
> + u8 major;
> + u8 nph; /* 0-base number of parameter headers */
> + u8 unused;
> +
> + /* Basic Flash Parameter Table. */
> + struct sfdp_parameter_header bfpt_header;
> +};
> +
> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
> +#define SFDP_PARAM_HEADER_PTP(p) \
> + (((p)->parameter_table_pointer[2] << 16) | \
> + ((p)->parameter_table_pointer[1] << 8) | \
> + ((p)->parameter_table_pointer[0] << 0))
> +
> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> +
> +#define SFDP_SIGNATURE 0x50444653U
> +#define SFDP_JESD216_MAJOR 1
> +#define SFDP_JESD216_MINOR 0
> +#define SFDP_JESD216A_MINOR 5
> +#define SFDP_JESD216B_MINOR 6
> +
> +/* Basic Flash Parameter Table */
> +
> +/*
> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> + * They are indexed from 1 but C arrays are indexed from 0.
> + */
> +#define BFPT_DWORD(i) ((i) - 1)
> +#define BFPT_DWORD_MAX 16
> +
> +/* The first version of JESB216 defined only 9 DWORDs. */
> +#define BFPT_DWORD_MAX_JESD216 9
> +
> +/* 1st DWORD. */
> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK GENMASK(18, 17)
> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> +#define BFPT_DWORD1_DTR BIT(19)
> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> +
> +/* 5th DWORD. */
> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> +
> +/* 11th DWORD. */
> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7, 4)
> +
> +/* 15th DWORD. */
> +
> +/*
> + * (from JESD216 rev B)
> + * Quad Enable Requirements (QER):
> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
> + * reads based on instruction. DQ3/HOLD# functions are hold during
> + * instruction phase.
> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> + * two data bytes where bit 1 of the second byte is one.
> + * [...]
> + * Writing only one byte to the status register has the side-effect of
> + * clearing status register 2, including the QE bit. The 100b code is
> + * used if writing one byte to the status register does not modify
> + * status register 2.
> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> + * one data byte where bit 6 is one.
> + * [...]
> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> + * [...]
> + * The status register 2 is read using instruction 3Fh.
> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> + * two data bytes where bit 1 of the second byte is one.
> + * [...]
> + * In contrast to the 001b code, writing one byte to the status
> + * register does not modify status register 2.
> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> + * Read Status instruction 05h. Status register2 is read using
> + * instruction 35h. QE is set via Writ Status instruction 01h with
> + * two data bytes where bit 1 of the second byte is one.
> + * [...]
> + */
> +#define BFPT_DWORD15_QER_MASK GENMASK(22, 20)
> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20) /* Micron */
> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /* Macronix */
> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
> +
> +struct sfdp_bfpt {
> + u32 dwords[BFPT_DWORD_MAX];
> +};
> +
> struct dm_spi_flash_ops {
> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> int (*write)(struct udevice *dev, u32 offset, size_t len,
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-15 9:22 ` Vignesh R
@ 2018-11-16 9:40 ` Rajat Srivastava
2018-11-16 10:20 ` Vignesh R
0 siblings, 1 reply; 13+ messages in thread
From: Rajat Srivastava @ 2018-11-16 9:40 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Vignesh R <vigneshr@ti.com>
> Sent: Thursday, November 15, 2018 2:52 PM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> Cc: jagan at openedev.com
> Subject: Re: [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI
> framework
>
> Hi Rajat,
>
> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> > Add support for JESD216 rev B standard JEDEC Serial Flash Discoverable
> > Parameters (SFDP) tables to dynamically initialize flash size, page
> > size and address width of the flash. More parameters can be added as
> > per requirement.
> > SFDP parsing is made default but already existing method for parsing
> > these parameters are not deprecated.
> > A flag is created to skip SFDP parsing for a particular flash, if
> > required.
> >
> > SFDP data lets us auto-detect the addressing mode supported by the
> > flash which helps us access the flash using 4-byte address.
> >
> > Add a new argument in spi_flash_addr() function to create commands
> > with 3-byte or 4-byte address depending on the SFDP data read. Add
> > pointer to struct spi_flash in struct spi_slave so that driver can
> > have access to SFDP data.
> >
> > Introduce new structures and functions to read and parse SFDP data.
> > This is loosely based on Linux SFDP framework.
> >
> > Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> > ---
> > Changes in v2:
> > - Make SFDP parsing the default method.
> > - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to provide an
> > option to skip SFDP parsing for a particular flash.
> > ---
>
> [...]
> > +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> > + const struct sfdp_parameter_header
> *bfpt_header) {
> > + struct sfdp_bfpt bfpt;
> > + size_t len;
> > + int i, err;
> > + u32 addr;
> > +
> > + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
> */
> > + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> > + return -EINVAL;
> > +
> > + /* Read the Basic Flash Parameter Table. */
> > + len = min_t(size_t, sizeof(bfpt),
> > + bfpt_header->length * sizeof(u32));
> > + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> > + memset(&bfpt, 0, sizeof(bfpt));
> > + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> > + if (err < 0)
> > + return err;
> > +
> > + /* Fix endianness of the BFPT DWORDs. */
> > + for (i = 0; i < BFPT_DWORD_MAX; i++)
> > + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> > +
> > + /* Number of address bytes. */
> > + switch (bfpt.dwords[BFPT_DWORD(1)] &
> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> > + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> > + flash->addr_width = 3;
> > + break;
> > +
> > + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> > + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> > + printf("mode on command\n");
> > + /*
> > + * By default, 4-byte addressing mode is set.
> > + * To enforce 3-byte addressing mode, set addrwd_3_in_use
> flag
> > + * in struct spi_flash for every command.
> > + */
> > + flash->addr_width = 4;
> > + break;
> > +
> > + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > + flash->addr_width = 4;
> > + break;
> > +
>
> I see your are updating flash->addr_width to 4 bytes here. But I don't see
> any code in this patch that changes flash->read_cmd (and write/erase
> cmds) to use 4B addressing opcodes. Also, I dont see any code that bypasses
> BAR register configuration when CONFIG_SPI_FLASH_BAR is defined and
> SFDP is available.
We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is defined?
> This patch will most certainly break SPI controller drivers(like
> cadence_qspi.c) that expect sf layer to provide correct read opcode and
> address width information, since there will be mismatch wrt opcode used
> and number of address byte sent.
> Not sure how this patch was tested, but I guess fsl_qspi modifies read
> opcode internal to the driver and gets away with it.
> Please modify the patch to update read_cmd/erase_cmd/write_cmd to use
> 4 byte addressing opcodes
If the flash supports only supports 4-byte opcodes then there will be no mismatch
wrt opcode used and number of address bytes sent since all of the opcodes that
the flash supports will be 4-bytes opcodes.
But if the flash supports both 3-byte as well as 4-byte address widths (which is a
separate case) then this code will default the address mode to 4-byte but gives an
option to use 3-byte by setting addrwd_3_in_use flag.
Regards
Rajat
>
> Regards
> Vigensh
>
>
> > + default:
> > + break;
> > + }
> > +
> > + /* Flash Memory Density (in bits). */
> > + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> > + if (flash->size & BIT(31)) {
> > + flash->size &= ~BIT(31);
> > +
> > + /*
> > + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> > + * bits is unlikely to exist so this error probably means
> > + * the BFPT we are reading is corrupted/wrong.
> > + */
> > + if (flash->size > 63)
> > + return -EINVAL;
> > +
> > + flash->size = 1ULL << flash->size;
> > + } else {
> > + flash->size++;
> > + }
> > + flash->size >>= 3; /* Convert to bytes. */
> > +
> > + /* Stop here if not JESD216 rev A or later. */
> > + if (bfpt_header->length < BFPT_DWORD_MAX)
> > + return 0;
> > +
> > + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> > + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> > + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> > + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> > + flash->page_size = 1U << flash->page_size;
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
> Parameters.
> > + * @flash: pointer to a 'struct spi_flash'
> > + *
> > + * The Serial Flash Discoverable Parameters are described by the
> > +JEDEC JESD216
> > + * specification. This is a standard which tends to supported by
> > +almost all
> > + * (Q)SPI memory manufacturers. Those hard-coded tables allow us to
> > +learn at
> > + * runtime the main parameters needed to perform basic SPI flash
> operations.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
> > + const struct sfdp_parameter_header *param_header,
> *bfpt_header;
> > + struct sfdp_parameter_header *param_headers = NULL;
> > + struct sfdp_header header;
> > + size_t psize;
> > + int i, err;
> > +
> > + /* Get the SFDP header. */
> > + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> > + if (err < 0)
> > + return err;
> > +
> > + /* Check the SFDP header version. */
> > + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> > + header.major != SFDP_JESD216_MAJOR)
> > + return -EINVAL;
> > +
> > + /*
> > + * Verify that the first and only mandatory parameter header is a
> > + * Basic Flash Parameter Table header as specified in JESD216.
> > + */
> > + bfpt_header = &header.bfpt_header;
> > + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> > + bfpt_header->major != SFDP_JESD216_MAJOR)
> > + return -EINVAL;
> > +
> > + /*
> > + * Allocate memory then read all parameter headers with a single
> > + * Read SFDP command. These parameter headers will actually be
> parsed
> > + * twice: a first time to get the latest revision of the basic flash
> > + * parameter table, then a second time to handle the supported
> optional
> > + * tables.
> > + * Hence we read the parameter headers once for all to reduce the
> > + * processing time
> > + */
> > + if (header.nph) {
> > + psize = header.nph * sizeof(*param_headers);
> > +
> > + param_headers = malloc(psize);
> > + if (!param_headers)
> > + return -ENOMEM;
> > +
> > + err = spi_flash_read_sfdp(flash, sizeof(header),
> > + psize, param_headers);
> > + if (err < 0) {
> > + dev_err(dev, "failed to read SFDP parameter
> headers\n");
> > + goto exit;
> > + }
> > + }
> > +
> > + /*
> > + * Check other parameter headers to get the latest revision of
> > + * the basic flash parameter table.
> > + */
> > + for (i = 0; i < header.nph; i++) {
> > + param_header = ¶m_headers[i];
> > +
> > + if (SFDP_PARAM_HEADER_ID(param_header) ==
> SFDP_BFPT_ID &&
> > + param_header->major == SFDP_JESD216_MAJOR &&
> > + (param_header->minor > bfpt_header->minor ||
> > + (param_header->minor == bfpt_header->minor &&
> > + param_header->length > bfpt_header->length)))
> > + bfpt_header = param_header;
> > + }
> > +
> > + err = spi_flash_parse_bfpt(flash, bfpt_header);
> > + if (err)
> > + goto exit;
> > +
> > +exit:
> > + free(param_headers);
> > + return err;
> > +}
> > +
> > static int set_quad_mode(struct spi_flash *flash,
> > const struct spi_flash_info *info) { @@ -1130,7
> +1366,7 @@ int
> > spi_flash_scan(struct spi_flash *flash) {
> > struct spi_slave *spi = flash->spi;
> > const struct spi_flash_info *info = NULL;
> > - int ret;
> > + int ret, sfdp = 0;
> >
> > info = spi_flash_read_id(flash);
> > if (IS_ERR_OR_NULL(info))
> > @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> > }
> > #endif
> >
> > + spi->flash = flash;
> > + flash->addrwd_3_in_use = false;
> > +
> > + /* Read Serial Flash Discoverable Parameters and initialize
> > + * the following parameters of flash:
> > + * 1. Flash size
> > + * 2. Page size
> > + * 3. Address width to be used for commands
> > + */
> > + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> > + flash->size = 0;
> > + sfdp = spi_flash_parse_sfdp(flash);
> > + if (sfdp < 0)
> > + debug("Unable to get SFDP information\n");
> > + }
> > +
> > /* Compute the flash size */
> > flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
> 0;
> > - flash->page_size = info->page_size;
> > + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > + flash->page_size = info->page_size;
> > + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> > + }
> > /*
> > * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
> pages,
> > * yet use the 0x4d00 Extended JEDEC code. The rest of the Spansion
> > @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash *flash)
> > }
> > flash->page_size <<= flash->shift;
> > flash->sector_size = info->sector_size << flash->shift;
> > - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> > + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > + flash->size = flash->sector_size * info->n_sectors <<
> > + flash->shift;
> > + }
> > #ifdef CONFIG_SF_DUAL_FLASH
> > if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> > flash->size <<= 1;
> > @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> > #endif
> >
> > #ifndef CONFIG_SPI_FLASH_BAR
> > - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> > - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> > - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> > + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> > + (flash->dual_flash == SF_SINGLE_FLASH &&
> > + flash->size > SPI_FLASH_16MB_BOUN) ||
> > + (flash->dual_flash > SF_SINGLE_FLASH &&
> > (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> > puts("SF: Warning - Only lower 16MiB accessible,");
> > puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); diff -
> -git
> > a/include/spi.h b/include/spi.h index 938627bc01..7189e60581 100644
> > --- a/include/spi.h
> > +++ b/include/spi.h
> > @@ -10,6 +10,7 @@
> > #define _SPI_H_
> >
> > #include <common.h>
> > +#include <spi_flash.h>
> >
> > /* SPI mode flags */
> > #define SPI_CPHA BIT(0) /* clock phase */
> > @@ -103,6 +104,7 @@ struct spi_slave {
> > unsigned int bus;
> > unsigned int cs;
> > #endif
> > + struct spi_flash *flash;
> > uint mode;
> > unsigned int wordlen;
> > unsigned int max_read_size;
> > diff --git a/include/spi_flash.h b/include/spi_flash.h index
> > 0ec98fb55d..6558a4a1d5 100644
> > --- a/include/spi_flash.h
> > +++ b/include/spi_flash.h
> > @@ -47,6 +47,9 @@ struct spi_slave;
> > * @read_cmd: Read cmd - Array Fast, Extn read and quad
> read.
> > * @write_cmd: Write cmd - page and quad program.
> > * @dummy_byte: Dummy cycles for read operation.
> > + * @cmd_len: Total length of command.
> > + * @addr_width: Number of address bytes.
> > + * @addrwd_3_in_use: Flag to send command in 3-byte address
> mode.
> > * @memory_map: Address of read-only SPI flash access
> > * @flash_lock: lock a region of the SPI Flash
> > * @flash_unlock: unlock a region of the SPI Flash
> > @@ -82,6 +85,9 @@ struct spi_flash {
> > u8 read_cmd;
> > u8 write_cmd;
> > u8 dummy_byte;
> > + u8 cmd_len;
> > + u8 addr_width;
> > + bool addrwd_3_in_use;
> >
> > void *memory_map;
> >
> > @@ -107,6 +113,120 @@ struct spi_flash { #endif };
> >
> > +/*
> > + * Serial Flash Discoverable Parameter Headers */ struct
> > +sfdp_parameter_header {
> > + u8 id_lsb;
> > + u8 minor;
> > + u8 major;
> > + u8 length; /* in double words */
> > + u8 parameter_table_pointer[3]; /* byte address */
> > + u8 id_msb;
> > +};
> > +
> > +struct sfdp_header {
> > + u32 signature; /* Ox50444653U <=> "SFDP" */
> > + u8 minor;
> > + u8 major;
> > + u8 nph; /* 0-base number of parameter headers */
> > + u8 unused;
> > +
> > + /* Basic Flash Parameter Table. */
> > + struct sfdp_parameter_header bfpt_header;
> > +};
> > +
> > +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
> > +#define SFDP_PARAM_HEADER_PTP(p) \
> > + (((p)->parameter_table_pointer[2] << 16) | \
> > + ((p)->parameter_table_pointer[1] << 8) | \
> > + ((p)->parameter_table_pointer[0] << 0))
> > +
> > +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
> */
> > +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> > +
> > +#define SFDP_SIGNATURE 0x50444653U
> > +#define SFDP_JESD216_MAJOR 1
> > +#define SFDP_JESD216_MINOR 0
> > +#define SFDP_JESD216A_MINOR 5
> > +#define SFDP_JESD216B_MINOR 6
> > +
> > +/* Basic Flash Parameter Table */
> > +
> > +/*
> > + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> > + * They are indexed from 1 but C arrays are indexed from 0.
> > + */
> > +#define BFPT_DWORD(i) ((i) - 1)
> > +#define BFPT_DWORD_MAX 16
> > +
> > +/* The first version of JESB216 defined only 9 DWORDs. */
> > +#define BFPT_DWORD_MAX_JESD216 9
> > +
> > +/* 1st DWORD. */
> > +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
> GENMASK(18, 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> > +#define BFPT_DWORD1_DTR BIT(19)
> > +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> > +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> > +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> > +
> > +/* 5th DWORD. */
> > +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> > +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> > +
> > +/* 11th DWORD. */
> > +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> > +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
> 4)
> > +
> > +/* 15th DWORD. */
> > +
> > +/*
> > + * (from JESD216 rev B)
> > + * Quad Enable Requirements (QER):
> > + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
> > + * reads based on instruction. DQ3/HOLD# functions are hold during
> > + * instruction phase.
> > + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + * Writing only one byte to the status register has the side-effect of
> > + * clearing status register 2, including the QE bit. The 100b code is
> > + * used if writing one byte to the status register does not modify
> > + * status register 2.
> > + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> > + * one data byte where bit 6 is one.
> > + * [...]
> > + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> > + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> > + * [...]
> > + * The status register 2 is read using instruction 3Fh.
> > + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + * In contrast to the 001b code, writing one byte to the status
> > + * register does not modify status register 2.
> > + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> > + * Read Status instruction 05h. Status register2 is read using
> > + * instruction 35h. QE is set via Writ Status instruction 01h with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + */
> > +#define BFPT_DWORD15_QER_MASK
> GENMASK(22, 20)
> > +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
> /* Micron */
> > +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> > +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> Macronix */
> > +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> > +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> > +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> Spansion */
> > +
> > +struct sfdp_bfpt {
> > + u32 dwords[BFPT_DWORD_MAX];
> > +};
> > +
> > struct dm_spi_flash_ops {
> > int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> > int (*write)(struct udevice *dev, u32 offset, size_t len,
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 9:40 ` Rajat Srivastava
@ 2018-11-16 10:20 ` Vignesh R
2018-11-16 10:23 ` Simon Goldschmidt
2018-11-16 10:43 ` Jagan Teki
0 siblings, 2 replies; 13+ messages in thread
From: Vignesh R @ 2018-11-16 10:20 UTC (permalink / raw)
To: u-boot
On 16/11/18 3:10 PM, Rajat Srivastava wrote:
>> Hi Rajat,
>>
>> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
>>> Add support for JESD216 rev B standard JEDEC Serial Flash Discoverable
>>> Parameters (SFDP) tables to dynamically initialize flash size, page
>>> size and address width of the flash. More parameters can be added as
>>> per requirement.
>>> SFDP parsing is made default but already existing method for parsing
>>> these parameters are not deprecated.
>>> A flag is created to skip SFDP parsing for a particular flash, if
>>> required.
>>>
>>> SFDP data lets us auto-detect the addressing mode supported by the
>>> flash which helps us access the flash using 4-byte address.
>>>
>>> Add a new argument in spi_flash_addr() function to create commands
>>> with 3-byte or 4-byte address depending on the SFDP data read. Add
>>> pointer to struct spi_flash in struct spi_slave so that driver can
>>> have access to SFDP data.
>>>
>>> Introduce new structures and functions to read and parse SFDP data.
>>> This is loosely based on Linux SFDP framework.
>>>
>>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
>>> ---
>>> Changes in v2:
>>> - Make SFDP parsing the default method.
>>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to provide an
>>> option to skip SFDP parsing for a particular flash.
>>> ---
>>
>> [...]
>>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
>>> + const struct sfdp_parameter_header
>> *bfpt_header) {
>>> + struct sfdp_bfpt bfpt;
>>> + size_t len;
>>> + int i, err;
>>> + u32 addr;
>>> +
>>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
>> */
>>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
>>> + return -EINVAL;
>>> +
>>> + /* Read the Basic Flash Parameter Table. */
>>> + len = min_t(size_t, sizeof(bfpt),
>>> + bfpt_header->length * sizeof(u32));
>>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
>>> + memset(&bfpt, 0, sizeof(bfpt));
>>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + /* Fix endianness of the BFPT DWORDs. */
>>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
>>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
>>> +
>>> + /* Number of address bytes. */
>>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
>> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>> + flash->addr_width = 3;
>>> + break;
>>> +
>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
>>> + printf("mode on command\n");
>>> + /*
>>> + * By default, 4-byte addressing mode is set.
>>> + * To enforce 3-byte addressing mode, set addrwd_3_in_use
>> flag
>>> + * in struct spi_flash for every command.
>>> + */
>>> + flash->addr_width = 4;
>>> + break;
>>> +
>>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>>> + flash->addr_width = 4;
>>> + break;
>>> +
>>
>> I see your are updating flash->addr_width to 4 bytes here. But I don't see
>> any code in this patch that changes flash->read_cmd (and write/erase
>> cmds) to use 4B addressing opcodes. Also, I dont see any code that bypasses
>> BAR register configuration when CONFIG_SPI_FLASH_BAR is defined and
>> SFDP is available.
>
> We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is defined?
>
I suggest skipping BAR configuration completely if flash supports
address width of 4 as per SFDP. BAR configuration is only needed if
flash does not support 4 byte addressing and flash size is > 16MB
>> This patch will most certainly break SPI controller drivers(like
>> cadence_qspi.c) that expect sf layer to provide correct read opcode and
>> address width information, since there will be mismatch wrt opcode used
>> and number of address byte sent.
>> Not sure how this patch was tested, but I guess fsl_qspi modifies read
>> opcode internal to the driver and gets away with it.
>> Please modify the patch to update read_cmd/erase_cmd/write_cmd to use
>> 4 byte addressing opcodes
>
> If the flash supports only supports 4-byte opcodes then there will be no mismatch
> wrt opcode used and number of address bytes sent since all of the opcodes that
> the flash supports will be 4-bytes opcodes.
> But if the flash supports both 3-byte as well as 4-byte address widths (which is a
> separate case) then this code will default the address mode to 4-byte but gives an
> option to use 3-byte by setting addrwd_3_in_use flag.
>
Lets assume flash supports both 3 and 4 byte addressing (AFAIK, majority
of flash parts do that). In this case, this patch is defaulting to 4
byte address width but flash->read_cmd/write_cmd/erase_cmd are still set
to 3 byte addressing opcodes.
So, SPI controller would end up send 3 addressing byte opcode but 4 byte
of address. The flash would interpret last byte of address as first byte
of data and thus lead to data corruption in case of write. Isn't that a
problem?
In particular you are not parsing entire bfpt like in [1] and will break
flash operations
[1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/spi-nor.c#L2259
Regards
Vignesh
> Regards
> Rajat
>
>>
>> Regards
>> Vigensh
>>
>>
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + /* Flash Memory Density (in bits). */
>>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
>>> + if (flash->size & BIT(31)) {
>>> + flash->size &= ~BIT(31);
>>> +
>>> + /*
>>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
>>> + * bits is unlikely to exist so this error probably means
>>> + * the BFPT we are reading is corrupted/wrong.
>>> + */
>>> + if (flash->size > 63)
>>> + return -EINVAL;
>>> +
>>> + flash->size = 1ULL << flash->size;
>>> + } else {
>>> + flash->size++;
>>> + }
>>> + flash->size >>= 3; /* Convert to bytes. */
>>> +
>>> + /* Stop here if not JESD216 rev A or later. */
>>> + if (bfpt_header->length < BFPT_DWORD_MAX)
>>> + return 0;
>>> +
>>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
>>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
>>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
>>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
>>> + flash->page_size = 1U << flash->page_size;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
>> Parameters.
>>> + * @flash: pointer to a 'struct spi_flash'
>>> + *
>>> + * The Serial Flash Discoverable Parameters are described by the
>>> +JEDEC JESD216
>>> + * specification. This is a standard which tends to supported by
>>> +almost all
>>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us to
>>> +learn at
>>> + * runtime the main parameters needed to perform basic SPI flash
>> operations.
>>> + *
>>> + * Return: 0 on success, -errno otherwise.
>>> + */
>>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
>>> + const struct sfdp_parameter_header *param_header,
>> *bfpt_header;
>>> + struct sfdp_parameter_header *param_headers = NULL;
>>> + struct sfdp_header header;
>>> + size_t psize;
>>> + int i, err;
>>> +
>>> + /* Get the SFDP header. */
>>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + /* Check the SFDP header version. */
>>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
>>> + header.major != SFDP_JESD216_MAJOR)
>>> + return -EINVAL;
>>> +
>>> + /*
>>> + * Verify that the first and only mandatory parameter header is a
>>> + * Basic Flash Parameter Table header as specified in JESD216.
>>> + */
>>> + bfpt_header = &header.bfpt_header;
>>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
>>> + bfpt_header->major != SFDP_JESD216_MAJOR)
>>> + return -EINVAL;
>>> +
>>> + /*
>>> + * Allocate memory then read all parameter headers with a single
>>> + * Read SFDP command. These parameter headers will actually be
>> parsed
>>> + * twice: a first time to get the latest revision of the basic flash
>>> + * parameter table, then a second time to handle the supported
>> optional
>>> + * tables.
>>> + * Hence we read the parameter headers once for all to reduce the
>>> + * processing time
>>> + */
>>> + if (header.nph) {
>>> + psize = header.nph * sizeof(*param_headers);
>>> +
>>> + param_headers = malloc(psize);
>>> + if (!param_headers)
>>> + return -ENOMEM;
>>> +
>>> + err = spi_flash_read_sfdp(flash, sizeof(header),
>>> + psize, param_headers);
>>> + if (err < 0) {
>>> + dev_err(dev, "failed to read SFDP parameter
>> headers\n");
>>> + goto exit;
>>> + }
>>> + }
>>> +
>>> + /*
>>> + * Check other parameter headers to get the latest revision of
>>> + * the basic flash parameter table.
>>> + */
>>> + for (i = 0; i < header.nph; i++) {
>>> + param_header = ¶m_headers[i];
>>> +
>>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
>> SFDP_BFPT_ID &&
>>> + param_header->major == SFDP_JESD216_MAJOR &&
>>> + (param_header->minor > bfpt_header->minor ||
>>> + (param_header->minor == bfpt_header->minor &&
>>> + param_header->length > bfpt_header->length)))
>>> + bfpt_header = param_header;
>>> + }
>>> +
>>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
>>> + if (err)
>>> + goto exit;
>>> +
>>> +exit:
>>> + free(param_headers);
>>> + return err;
>>> +}
>>> +
>>> static int set_quad_mode(struct spi_flash *flash,
>>> const struct spi_flash_info *info) { @@ -1130,7
>> +1366,7 @@ int
>>> spi_flash_scan(struct spi_flash *flash) {
>>> struct spi_slave *spi = flash->spi;
>>> const struct spi_flash_info *info = NULL;
>>> - int ret;
>>> + int ret, sfdp = 0;
>>>
>>> info = spi_flash_read_id(flash);
>>> if (IS_ERR_OR_NULL(info))
>>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
>>> }
>>> #endif
>>>
>>> + spi->flash = flash;
>>> + flash->addrwd_3_in_use = false;
>>> +
>>> + /* Read Serial Flash Discoverable Parameters and initialize
>>> + * the following parameters of flash:
>>> + * 1. Flash size
>>> + * 2. Page size
>>> + * 3. Address width to be used for commands
>>> + */
>>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
>>> + flash->size = 0;
>>> + sfdp = spi_flash_parse_sfdp(flash);
>>> + if (sfdp < 0)
>>> + debug("Unable to get SFDP information\n");
>>> + }
>>> +
>>> /* Compute the flash size */
>>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
>> 0;
>>> - flash->page_size = info->page_size;
>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>> + flash->page_size = info->page_size;
>>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
>>> + }
>>> /*
>>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
>> pages,
>>> * yet use the 0x4d00 Extended JEDEC code. The rest of the Spansion
>>> @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash *flash)
>>> }
>>> flash->page_size <<= flash->shift;
>>> flash->sector_size = info->sector_size << flash->shift;
>>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>> + flash->size = flash->sector_size * info->n_sectors <<
>>> + flash->shift;
>>> + }
>>> #ifdef CONFIG_SF_DUAL_FLASH
>>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
>>> flash->size <<= 1;
>>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
>>> #endif
>>>
>>> #ifndef CONFIG_SPI_FLASH_BAR
>>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
>>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
>>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
>>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
>>> + (flash->dual_flash == SF_SINGLE_FLASH &&
>>> + flash->size > SPI_FLASH_16MB_BOUN) ||
>>> + (flash->dual_flash > SF_SINGLE_FLASH &&
>>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
>>> puts("SF: Warning - Only lower 16MiB accessible,");
>>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); diff -
>> -git
>>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581 100644
>>> --- a/include/spi.h
>>> +++ b/include/spi.h
>>> @@ -10,6 +10,7 @@
>>> #define _SPI_H_
>>>
>>> #include <common.h>
>>> +#include <spi_flash.h>
>>>
>>> /* SPI mode flags */
>>> #define SPI_CPHA BIT(0) /* clock phase */
>>> @@ -103,6 +104,7 @@ struct spi_slave {
>>> unsigned int bus;
>>> unsigned int cs;
>>> #endif
>>> + struct spi_flash *flash;
>>> uint mode;
>>> unsigned int wordlen;
>>> unsigned int max_read_size;
>>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
>>> 0ec98fb55d..6558a4a1d5 100644
>>> --- a/include/spi_flash.h
>>> +++ b/include/spi_flash.h
>>> @@ -47,6 +47,9 @@ struct spi_slave;
>>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
>> read.
>>> * @write_cmd: Write cmd - page and quad program.
>>> * @dummy_byte: Dummy cycles for read operation.
>>> + * @cmd_len: Total length of command.
>>> + * @addr_width: Number of address bytes.
>>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
>> mode.
>>> * @memory_map: Address of read-only SPI flash access
>>> * @flash_lock: lock a region of the SPI Flash
>>> * @flash_unlock: unlock a region of the SPI Flash
>>> @@ -82,6 +85,9 @@ struct spi_flash {
>>> u8 read_cmd;
>>> u8 write_cmd;
>>> u8 dummy_byte;
>>> + u8 cmd_len;
>>> + u8 addr_width;
>>> + bool addrwd_3_in_use;
>>>
>>> void *memory_map;
>>>
>>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
>>>
>>> +/*
>>> + * Serial Flash Discoverable Parameter Headers */ struct
>>> +sfdp_parameter_header {
>>> + u8 id_lsb;
>>> + u8 minor;
>>> + u8 major;
>>> + u8 length; /* in double words */
>>> + u8 parameter_table_pointer[3]; /* byte address */
>>> + u8 id_msb;
>>> +};
>>> +
>>> +struct sfdp_header {
>>> + u32 signature; /* Ox50444653U <=> "SFDP" */
>>> + u8 minor;
>>> + u8 major;
>>> + u8 nph; /* 0-base number of parameter headers */
>>> + u8 unused;
>>> +
>>> + /* Basic Flash Parameter Table. */
>>> + struct sfdp_parameter_header bfpt_header;
>>> +};
>>> +
>>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
>>> +#define SFDP_PARAM_HEADER_PTP(p) \
>>> + (((p)->parameter_table_pointer[2] << 16) | \
>>> + ((p)->parameter_table_pointer[1] << 8) | \
>>> + ((p)->parameter_table_pointer[0] << 0))
>>> +
>>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
>> */
>>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
>>> +
>>> +#define SFDP_SIGNATURE 0x50444653U
>>> +#define SFDP_JESD216_MAJOR 1
>>> +#define SFDP_JESD216_MINOR 0
>>> +#define SFDP_JESD216A_MINOR 5
>>> +#define SFDP_JESD216B_MINOR 6
>>> +
>>> +/* Basic Flash Parameter Table */
>>> +
>>> +/*
>>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
>>> + * They are indexed from 1 but C arrays are indexed from 0.
>>> + */
>>> +#define BFPT_DWORD(i) ((i) - 1)
>>> +#define BFPT_DWORD_MAX 16
>>> +
>>> +/* The first version of JESB216 defined only 9 DWORDs. */
>>> +#define BFPT_DWORD_MAX_JESD216 9
>>> +
>>> +/* 1st DWORD. */
>>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
>>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
>> GENMASK(18, 17)
>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
>>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
>>> +#define BFPT_DWORD1_DTR BIT(19)
>>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
>>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
>>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
>>> +
>>> +/* 5th DWORD. */
>>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
>>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
>>> +
>>> +/* 11th DWORD. */
>>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
>>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
>> 4)
>>> +
>>> +/* 15th DWORD. */
>>> +
>>> +/*
>>> + * (from JESD216 rev B)
>>> + * Quad Enable Requirements (QER):
>>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
>>> + * reads based on instruction. DQ3/HOLD# functions are hold during
>>> + * instruction phase.
>>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
>>> + * two data bytes where bit 1 of the second byte is one.
>>> + * [...]
>>> + * Writing only one byte to the status register has the side-effect of
>>> + * clearing status register 2, including the QE bit. The 100b code is
>>> + * used if writing one byte to the status register does not modify
>>> + * status register 2.
>>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
>>> + * one data byte where bit 6 is one.
>>> + * [...]
>>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
>>> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
>>> + * [...]
>>> + * The status register 2 is read using instruction 3Fh.
>>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
>>> + * two data bytes where bit 1 of the second byte is one.
>>> + * [...]
>>> + * In contrast to the 001b code, writing one byte to the status
>>> + * register does not modify status register 2.
>>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
>>> + * Read Status instruction 05h. Status register2 is read using
>>> + * instruction 35h. QE is set via Writ Status instruction 01h with
>>> + * two data bytes where bit 1 of the second byte is one.
>>> + * [...]
>>> + */
>>> +#define BFPT_DWORD15_QER_MASK
>> GENMASK(22, 20)
>>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
>> /* Micron */
>>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
>>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
>> Macronix */
>>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
>>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
>>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
>> Spansion */
>>> +
>>> +struct sfdp_bfpt {
>>> + u32 dwords[BFPT_DWORD_MAX];
>>> +};
>>> +
>>> struct dm_spi_flash_ops {
>>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
>>> int (*write)(struct udevice *dev, u32 offset, size_t len,
>>>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 10:20 ` Vignesh R
@ 2018-11-16 10:23 ` Simon Goldschmidt
2018-11-16 12:59 ` Rajat Srivastava
2018-11-16 10:43 ` Jagan Teki
1 sibling, 1 reply; 13+ messages in thread
From: Simon Goldschmidt @ 2018-11-16 10:23 UTC (permalink / raw)
To: u-boot
On Fri, Nov 16, 2018 at 11:19 AM Vignesh R <vigneshr@ti.com> wrote:
>
>
>
> On 16/11/18 3:10 PM, Rajat Srivastava wrote:
> >> Hi Rajat,
> >>
> >> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> >>> Add support for JESD216 rev B standard JEDEC Serial Flash Discoverable
> >>> Parameters (SFDP) tables to dynamically initialize flash size, page
> >>> size and address width of the flash. More parameters can be added as
> >>> per requirement.
> >>> SFDP parsing is made default but already existing method for parsing
> >>> these parameters are not deprecated.
> >>> A flag is created to skip SFDP parsing for a particular flash, if
> >>> required.
> >>>
> >>> SFDP data lets us auto-detect the addressing mode supported by the
> >>> flash which helps us access the flash using 4-byte address.
> >>>
> >>> Add a new argument in spi_flash_addr() function to create commands
> >>> with 3-byte or 4-byte address depending on the SFDP data read. Add
> >>> pointer to struct spi_flash in struct spi_slave so that driver can
> >>> have access to SFDP data.
> >>>
> >>> Introduce new structures and functions to read and parse SFDP data.
> >>> This is loosely based on Linux SFDP framework.
> >>>
> >>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> >>> ---
> >>> Changes in v2:
> >>> - Make SFDP parsing the default method.
> >>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to provide an
> >>> option to skip SFDP parsing for a particular flash.
> >>> ---
> >>
> >> [...]
> >>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> >>> + const struct sfdp_parameter_header
> >> *bfpt_header) {
> >>> + struct sfdp_bfpt bfpt;
> >>> + size_t len;
> >>> + int i, err;
> >>> + u32 addr;
> >>> +
> >>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
> >> */
> >>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> >>> + return -EINVAL;
> >>> +
> >>> + /* Read the Basic Flash Parameter Table. */
> >>> + len = min_t(size_t, sizeof(bfpt),
> >>> + bfpt_header->length * sizeof(u32));
> >>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> >>> + memset(&bfpt, 0, sizeof(bfpt));
> >>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> >>> + if (err < 0)
> >>> + return err;
> >>> +
> >>> + /* Fix endianness of the BFPT DWORDs. */
> >>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
> >>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> >>> +
> >>> + /* Number of address bytes. */
> >>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
> >> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> >>> + flash->addr_width = 3;
> >>> + break;
> >>> +
> >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> >>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> >>> + printf("mode on command\n");
> >>> + /*
> >>> + * By default, 4-byte addressing mode is set.
> >>> + * To enforce 3-byte addressing mode, set addrwd_3_in_use
> >> flag
> >>> + * in struct spi_flash for every command.
> >>> + */
> >>> + flash->addr_width = 4;
> >>> + break;
> >>> +
> >>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> >>> + flash->addr_width = 4;
> >>> + break;
> >>> +
> >>
> >> I see your are updating flash->addr_width to 4 bytes here. But I don't see
> >> any code in this patch that changes flash->read_cmd (and write/erase
> >> cmds) to use 4B addressing opcodes. Also, I dont see any code that bypasses
> >> BAR register configuration when CONFIG_SPI_FLASH_BAR is defined and
> >> SFDP is available.
> >
> > We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is defined?
> >
>
> I suggest skipping BAR configuration completely if flash supports
> address width of 4 as per SFDP. BAR configuration is only needed if
> flash does not support 4 byte addressing and flash size is > 16MB
>
> >> This patch will most certainly break SPI controller drivers(like
> >> cadence_qspi.c) that expect sf layer to provide correct read opcode and
> >> address width information, since there will be mismatch wrt opcode used
> >> and number of address byte sent.
> >> Not sure how this patch was tested, but I guess fsl_qspi modifies read
> >> opcode internal to the driver and gets away with it.
> >> Please modify the patch to update read_cmd/erase_cmd/write_cmd to use
> >> 4 byte addressing opcodes
> >
> > If the flash supports only supports 4-byte opcodes then there will be no mismatch
> > wrt opcode used and number of address bytes sent since all of the opcodes that
> > the flash supports will be 4-bytes opcodes.
> > But if the flash supports both 3-byte as well as 4-byte address widths (which is a
> > separate case) then this code will default the address mode to 4-byte but gives an
> > option to use 3-byte by setting addrwd_3_in_use flag.
> >
>
> Lets assume flash supports both 3 and 4 byte addressing (AFAIK, majority
> of flash parts do that). In this case, this patch is defaulting to 4
> byte address width but flash->read_cmd/write_cmd/erase_cmd are still set
> to 3 byte addressing opcodes.
> So, SPI controller would end up send 3 addressing byte opcode but 4 byte
> of address. The flash would interpret last byte of address as first byte
> of data and thus lead to data corruption in case of write. Isn't that a
> problem?
Additionally, there are flash chips with a 3-/4-byte mode change
instruction. But using 4-byte opcodes should be preferred.
Simon
>
> In particular you are not parsing entire bfpt like in [1] and will break
> flash operations
>
> [1]https://elixir.bootlin.com/linux/latest/source/drivers/mtd/spi-nor/spi-nor.c#L2259
>
> Regards
> Vignesh
>
> > Regards
> > Rajat
> >
> >>
> >> Regards
> >> Vigensh
> >>
> >>
> >>> + default:
> >>> + break;
> >>> + }
> >>> +
> >>> + /* Flash Memory Density (in bits). */
> >>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> >>> + if (flash->size & BIT(31)) {
> >>> + flash->size &= ~BIT(31);
> >>> +
> >>> + /*
> >>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> >>> + * bits is unlikely to exist so this error probably means
> >>> + * the BFPT we are reading is corrupted/wrong.
> >>> + */
> >>> + if (flash->size > 63)
> >>> + return -EINVAL;
> >>> +
> >>> + flash->size = 1ULL << flash->size;
> >>> + } else {
> >>> + flash->size++;
> >>> + }
> >>> + flash->size >>= 3; /* Convert to bytes. */
> >>> +
> >>> + /* Stop here if not JESD216 rev A or later. */
> >>> + if (bfpt_header->length < BFPT_DWORD_MAX)
> >>> + return 0;
> >>> +
> >>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> >>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> >>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> >>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> >>> + flash->page_size = 1U << flash->page_size;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +/*
> >>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
> >> Parameters.
> >>> + * @flash: pointer to a 'struct spi_flash'
> >>> + *
> >>> + * The Serial Flash Discoverable Parameters are described by the
> >>> +JEDEC JESD216
> >>> + * specification. This is a standard which tends to supported by
> >>> +almost all
> >>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us to
> >>> +learn at
> >>> + * runtime the main parameters needed to perform basic SPI flash
> >> operations.
> >>> + *
> >>> + * Return: 0 on success, -errno otherwise.
> >>> + */
> >>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
> >>> + const struct sfdp_parameter_header *param_header,
> >> *bfpt_header;
> >>> + struct sfdp_parameter_header *param_headers = NULL;
> >>> + struct sfdp_header header;
> >>> + size_t psize;
> >>> + int i, err;
> >>> +
> >>> + /* Get the SFDP header. */
> >>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> >>> + if (err < 0)
> >>> + return err;
> >>> +
> >>> + /* Check the SFDP header version. */
> >>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> >>> + header.major != SFDP_JESD216_MAJOR)
> >>> + return -EINVAL;
> >>> +
> >>> + /*
> >>> + * Verify that the first and only mandatory parameter header is a
> >>> + * Basic Flash Parameter Table header as specified in JESD216.
> >>> + */
> >>> + bfpt_header = &header.bfpt_header;
> >>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> >>> + bfpt_header->major != SFDP_JESD216_MAJOR)
> >>> + return -EINVAL;
> >>> +
> >>> + /*
> >>> + * Allocate memory then read all parameter headers with a single
> >>> + * Read SFDP command. These parameter headers will actually be
> >> parsed
> >>> + * twice: a first time to get the latest revision of the basic flash
> >>> + * parameter table, then a second time to handle the supported
> >> optional
> >>> + * tables.
> >>> + * Hence we read the parameter headers once for all to reduce the
> >>> + * processing time
> >>> + */
> >>> + if (header.nph) {
> >>> + psize = header.nph * sizeof(*param_headers);
> >>> +
> >>> + param_headers = malloc(psize);
> >>> + if (!param_headers)
> >>> + return -ENOMEM;
> >>> +
> >>> + err = spi_flash_read_sfdp(flash, sizeof(header),
> >>> + psize, param_headers);
> >>> + if (err < 0) {
> >>> + dev_err(dev, "failed to read SFDP parameter
> >> headers\n");
> >>> + goto exit;
> >>> + }
> >>> + }
> >>> +
> >>> + /*
> >>> + * Check other parameter headers to get the latest revision of
> >>> + * the basic flash parameter table.
> >>> + */
> >>> + for (i = 0; i < header.nph; i++) {
> >>> + param_header = ¶m_headers[i];
> >>> +
> >>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
> >> SFDP_BFPT_ID &&
> >>> + param_header->major == SFDP_JESD216_MAJOR &&
> >>> + (param_header->minor > bfpt_header->minor ||
> >>> + (param_header->minor == bfpt_header->minor &&
> >>> + param_header->length > bfpt_header->length)))
> >>> + bfpt_header = param_header;
> >>> + }
> >>> +
> >>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
> >>> + if (err)
> >>> + goto exit;
> >>> +
> >>> +exit:
> >>> + free(param_headers);
> >>> + return err;
> >>> +}
> >>> +
> >>> static int set_quad_mode(struct spi_flash *flash,
> >>> const struct spi_flash_info *info) { @@ -1130,7
> >> +1366,7 @@ int
> >>> spi_flash_scan(struct spi_flash *flash) {
> >>> struct spi_slave *spi = flash->spi;
> >>> const struct spi_flash_info *info = NULL;
> >>> - int ret;
> >>> + int ret, sfdp = 0;
> >>>
> >>> info = spi_flash_read_id(flash);
> >>> if (IS_ERR_OR_NULL(info))
> >>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> >>> }
> >>> #endif
> >>>
> >>> + spi->flash = flash;
> >>> + flash->addrwd_3_in_use = false;
> >>> +
> >>> + /* Read Serial Flash Discoverable Parameters and initialize
> >>> + * the following parameters of flash:
> >>> + * 1. Flash size
> >>> + * 2. Page size
> >>> + * 3. Address width to be used for commands
> >>> + */
> >>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> >>> + flash->size = 0;
> >>> + sfdp = spi_flash_parse_sfdp(flash);
> >>> + if (sfdp < 0)
> >>> + debug("Unable to get SFDP information\n");
> >>> + }
> >>> +
> >>> /* Compute the flash size */
> >>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
> >> 0;
> >>> - flash->page_size = info->page_size;
> >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> >>> + flash->page_size = info->page_size;
> >>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> >>> + }
> >>> /*
> >>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
> >> pages,
> >>> * yet use the 0x4d00 Extended JEDEC code. The rest of the Spansion
> >>> @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash *flash)
> >>> }
> >>> flash->page_size <<= flash->shift;
> >>> flash->sector_size = info->sector_size << flash->shift;
> >>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> >>> + flash->size = flash->sector_size * info->n_sectors <<
> >>> + flash->shift;
> >>> + }
> >>> #ifdef CONFIG_SF_DUAL_FLASH
> >>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> >>> flash->size <<= 1;
> >>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> >>> #endif
> >>>
> >>> #ifndef CONFIG_SPI_FLASH_BAR
> >>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> >>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> >>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> >>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> >>> + (flash->dual_flash == SF_SINGLE_FLASH &&
> >>> + flash->size > SPI_FLASH_16MB_BOUN) ||
> >>> + (flash->dual_flash > SF_SINGLE_FLASH &&
> >>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> >>> puts("SF: Warning - Only lower 16MiB accessible,");
> >>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); diff -
> >> -git
> >>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581 100644
> >>> --- a/include/spi.h
> >>> +++ b/include/spi.h
> >>> @@ -10,6 +10,7 @@
> >>> #define _SPI_H_
> >>>
> >>> #include <common.h>
> >>> +#include <spi_flash.h>
> >>>
> >>> /* SPI mode flags */
> >>> #define SPI_CPHA BIT(0) /* clock phase */
> >>> @@ -103,6 +104,7 @@ struct spi_slave {
> >>> unsigned int bus;
> >>> unsigned int cs;
> >>> #endif
> >>> + struct spi_flash *flash;
> >>> uint mode;
> >>> unsigned int wordlen;
> >>> unsigned int max_read_size;
> >>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
> >>> 0ec98fb55d..6558a4a1d5 100644
> >>> --- a/include/spi_flash.h
> >>> +++ b/include/spi_flash.h
> >>> @@ -47,6 +47,9 @@ struct spi_slave;
> >>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
> >> read.
> >>> * @write_cmd: Write cmd - page and quad program.
> >>> * @dummy_byte: Dummy cycles for read operation.
> >>> + * @cmd_len: Total length of command.
> >>> + * @addr_width: Number of address bytes.
> >>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
> >> mode.
> >>> * @memory_map: Address of read-only SPI flash access
> >>> * @flash_lock: lock a region of the SPI Flash
> >>> * @flash_unlock: unlock a region of the SPI Flash
> >>> @@ -82,6 +85,9 @@ struct spi_flash {
> >>> u8 read_cmd;
> >>> u8 write_cmd;
> >>> u8 dummy_byte;
> >>> + u8 cmd_len;
> >>> + u8 addr_width;
> >>> + bool addrwd_3_in_use;
> >>>
> >>> void *memory_map;
> >>>
> >>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
> >>>
> >>> +/*
> >>> + * Serial Flash Discoverable Parameter Headers */ struct
> >>> +sfdp_parameter_header {
> >>> + u8 id_lsb;
> >>> + u8 minor;
> >>> + u8 major;
> >>> + u8 length; /* in double words */
> >>> + u8 parameter_table_pointer[3]; /* byte address */
> >>> + u8 id_msb;
> >>> +};
> >>> +
> >>> +struct sfdp_header {
> >>> + u32 signature; /* Ox50444653U <=> "SFDP" */
> >>> + u8 minor;
> >>> + u8 major;
> >>> + u8 nph; /* 0-base number of parameter headers */
> >>> + u8 unused;
> >>> +
> >>> + /* Basic Flash Parameter Table. */
> >>> + struct sfdp_parameter_header bfpt_header;
> >>> +};
> >>> +
> >>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
> >>> +#define SFDP_PARAM_HEADER_PTP(p) \
> >>> + (((p)->parameter_table_pointer[2] << 16) | \
> >>> + ((p)->parameter_table_pointer[1] << 8) | \
> >>> + ((p)->parameter_table_pointer[0] << 0))
> >>> +
> >>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
> >> */
> >>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> >>> +
> >>> +#define SFDP_SIGNATURE 0x50444653U
> >>> +#define SFDP_JESD216_MAJOR 1
> >>> +#define SFDP_JESD216_MINOR 0
> >>> +#define SFDP_JESD216A_MINOR 5
> >>> +#define SFDP_JESD216B_MINOR 6
> >>> +
> >>> +/* Basic Flash Parameter Table */
> >>> +
> >>> +/*
> >>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> >>> + * They are indexed from 1 but C arrays are indexed from 0.
> >>> + */
> >>> +#define BFPT_DWORD(i) ((i) - 1)
> >>> +#define BFPT_DWORD_MAX 16
> >>> +
> >>> +/* The first version of JESB216 defined only 9 DWORDs. */
> >>> +#define BFPT_DWORD_MAX_JESD216 9
> >>> +
> >>> +/* 1st DWORD. */
> >>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> >>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
> >> GENMASK(18, 17)
> >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> >>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> >>> +#define BFPT_DWORD1_DTR BIT(19)
> >>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> >>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> >>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> >>> +
> >>> +/* 5th DWORD. */
> >>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> >>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> >>> +
> >>> +/* 11th DWORD. */
> >>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> >>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
> >> 4)
> >>> +
> >>> +/* 15th DWORD. */
> >>> +
> >>> +/*
> >>> + * (from JESD216 rev B)
> >>> + * Quad Enable Requirements (QER):
> >>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
> >>> + * reads based on instruction. DQ3/HOLD# functions are hold during
> >>> + * instruction phase.
> >>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> >>> + * two data bytes where bit 1 of the second byte is one.
> >>> + * [...]
> >>> + * Writing only one byte to the status register has the side-effect of
> >>> + * clearing status register 2, including the QE bit. The 100b code is
> >>> + * used if writing one byte to the status register does not modify
> >>> + * status register 2.
> >>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> >>> + * one data byte where bit 6 is one.
> >>> + * [...]
> >>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> >>> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> >>> + * [...]
> >>> + * The status register 2 is read using instruction 3Fh.
> >>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> >>> + * two data bytes where bit 1 of the second byte is one.
> >>> + * [...]
> >>> + * In contrast to the 001b code, writing one byte to the status
> >>> + * register does not modify status register 2.
> >>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> >>> + * Read Status instruction 05h. Status register2 is read using
> >>> + * instruction 35h. QE is set via Writ Status instruction 01h with
> >>> + * two data bytes where bit 1 of the second byte is one.
> >>> + * [...]
> >>> + */
> >>> +#define BFPT_DWORD15_QER_MASK
> >> GENMASK(22, 20)
> >>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
> >> /* Micron */
> >>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> >>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> >> Macronix */
> >>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> >>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> >>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> >> Spansion */
> >>> +
> >>> +struct sfdp_bfpt {
> >>> + u32 dwords[BFPT_DWORD_MAX];
> >>> +};
> >>> +
> >>> struct dm_spi_flash_ops {
> >>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> >>> int (*write)(struct udevice *dev, u32 offset, size_t len,
> >>>
>
> --
> Regards
> Vignesh
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 10:20 ` Vignesh R
2018-11-16 10:23 ` Simon Goldschmidt
@ 2018-11-16 10:43 ` Jagan Teki
1 sibling, 0 replies; 13+ messages in thread
From: Jagan Teki @ 2018-11-16 10:43 UTC (permalink / raw)
To: u-boot
On 16/11/18 3:50 PM, Vignesh R wrote:
>
>
> On 16/11/18 3:10 PM, Rajat Srivastava wrote:
>>> Hi Rajat,
>>>
>>> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
>>>> Add support for JESD216 rev B standard JEDEC Serial Flash Discoverable
>>>> Parameters (SFDP) tables to dynamically initialize flash size, page
>>>> size and address width of the flash. More parameters can be added as
>>>> per requirement.
>>>> SFDP parsing is made default but already existing method for parsing
>>>> these parameters are not deprecated.
>>>> A flag is created to skip SFDP parsing for a particular flash, if
>>>> required.
>>>>
>>>> SFDP data lets us auto-detect the addressing mode supported by the
>>>> flash which helps us access the flash using 4-byte address.
>>>>
>>>> Add a new argument in spi_flash_addr() function to create commands
>>>> with 3-byte or 4-byte address depending on the SFDP data read. Add
>>>> pointer to struct spi_flash in struct spi_slave so that driver can
>>>> have access to SFDP data.
>>>>
>>>> Introduce new structures and functions to read and parse SFDP data.
>>>> This is loosely based on Linux SFDP framework.
>>>>
>>>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
>>>> ---
>>>> Changes in v2:
>>>> - Make SFDP parsing the default method.
>>>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to provide an
>>>> option to skip SFDP parsing for a particular flash.
>>>> ---
>>>
>>> [...]
>>>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
>>>> + const struct sfdp_parameter_header
>>> *bfpt_header) {
>>>> + struct sfdp_bfpt bfpt;
>>>> + size_t len;
>>>> + int i, err;
>>>> + u32 addr;
>>>> +
>>>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
>>> */
>>>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
>>>> + return -EINVAL;
>>>> +
>>>> + /* Read the Basic Flash Parameter Table. */
>>>> + len = min_t(size_t, sizeof(bfpt),
>>>> + bfpt_header->length * sizeof(u32));
>>>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
>>>> + memset(&bfpt, 0, sizeof(bfpt));
>>>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
>>>> + if (err < 0)
>>>> + return err;
>>>> +
>>>> + /* Fix endianness of the BFPT DWORDs. */
>>>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
>>>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
>>>> +
>>>> + /* Number of address bytes. */
>>>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
>>> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>>> + flash->addr_width = 3;
>>>> + break;
>>>> +
>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
>>>> + printf("mode on command\n");
>>>> + /*
>>>> + * By default, 4-byte addressing mode is set.
>>>> + * To enforce 3-byte addressing mode, set addrwd_3_in_use
>>> flag
>>>> + * in struct spi_flash for every command.
>>>> + */
>>>> + flash->addr_width = 4;
>>>> + break;
>>>> +
>>>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>>>> + flash->addr_width = 4;
>>>> + break;
>>>> +
>>>
>>> I see your are updating flash->addr_width to 4 bytes here. But I don't see
>>> any code in this patch that changes flash->read_cmd (and write/erase
>>> cmds) to use 4B addressing opcodes. Also, I dont see any code that bypasses
>>> BAR register configuration when CONFIG_SPI_FLASH_BAR is defined and
>>> SFDP is available.
>>
>> We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is defined?
>>
>
> I suggest skipping BAR configuration completely if flash supports
> address width of 4 as per SFDP. BAR configuration is only needed if
> flash does not support 4 byte addressing and flash size is > 16MB
And also some controller do support > 16MB flash, but have in-capable to
handle 4-byte addressing commands, so for those cases BAR would require.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 10:23 ` Simon Goldschmidt
@ 2018-11-16 12:59 ` Rajat Srivastava
2018-11-16 13:09 ` Simon Goldschmidt
2018-11-16 15:25 ` Vignesh R
0 siblings, 2 replies; 13+ messages in thread
From: Rajat Srivastava @ 2018-11-16 12:59 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Sent: Friday, November 16, 2018 3:53 PM
> To: vigneshr at ti.com
> Cc: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Jagan Teki <jagan@openedev.com>
> Subject: Re: [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI
> framework
>
> On Fri, Nov 16, 2018 at 11:19 AM Vignesh R <vigneshr@ti.com> wrote:
> >
> >
> >
> > On 16/11/18 3:10 PM, Rajat Srivastava wrote:
> > >> Hi Rajat,
> > >>
> > >> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> > >>> Add support for JESD216 rev B standard JEDEC Serial Flash
> > >>> Discoverable Parameters (SFDP) tables to dynamically initialize
> > >>> flash size, page size and address width of the flash. More
> > >>> parameters can be added as per requirement.
> > >>> SFDP parsing is made default but already existing method for
> > >>> parsing these parameters are not deprecated.
> > >>> A flag is created to skip SFDP parsing for a particular flash, if
> > >>> required.
> > >>>
> > >>> SFDP data lets us auto-detect the addressing mode supported by the
> > >>> flash which helps us access the flash using 4-byte address.
> > >>>
> > >>> Add a new argument in spi_flash_addr() function to create commands
> > >>> with 3-byte or 4-byte address depending on the SFDP data read. Add
> > >>> pointer to struct spi_flash in struct spi_slave so that driver can
> > >>> have access to SFDP data.
> > >>>
> > >>> Introduce new structures and functions to read and parse SFDP data.
> > >>> This is loosely based on Linux SFDP framework.
> > >>>
> > >>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> > >>> ---
> > >>> Changes in v2:
> > >>> - Make SFDP parsing the default method.
> > >>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
> provide
> > >>> an option to skip SFDP parsing for a particular flash.
> > >>> ---
> > >>
> > >> [...]
> > >>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> > >>> + const struct sfdp_parameter_header
> > >> *bfpt_header) {
> > >>> + struct sfdp_bfpt bfpt;
> > >>> + size_t len;
> > >>> + int i, err;
> > >>> + u32 addr;
> > >>> +
> > >>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
> > >> */
> > >>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> > >>> + return -EINVAL;
> > >>> +
> > >>> + /* Read the Basic Flash Parameter Table. */
> > >>> + len = min_t(size_t, sizeof(bfpt),
> > >>> + bfpt_header->length * sizeof(u32));
> > >>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> > >>> + memset(&bfpt, 0, sizeof(bfpt));
> > >>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> > >>> + if (err < 0)
> > >>> + return err;
> > >>> +
> > >>> + /* Fix endianness of the BFPT DWORDs. */
> > >>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
> > >>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> > >>> +
> > >>> + /* Number of address bytes. */
> > >>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
> > >> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> > >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> > >>> + flash->addr_width = 3;
> > >>> + break;
> > >>> +
> > >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> > >>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> > >>> + printf("mode on command\n");
> > >>> + /*
> > >>> + * By default, 4-byte addressing mode is set.
> > >>> + * To enforce 3-byte addressing mode, set
> > >>> + addrwd_3_in_use
> > >> flag
> > >>> + * in struct spi_flash for every command.
> > >>> + */
> > >>> + flash->addr_width = 4;
> > >>> + break;
> > >>> +
> > >>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > >>> + flash->addr_width = 4;
> > >>> + break;
> > >>> +
> > >>
> > >> I see your are updating flash->addr_width to 4 bytes here. But I
> > >> don't see any code in this patch that changes flash->read_cmd (and
> > >> write/erase
> > >> cmds) to use 4B addressing opcodes. Also, I dont see any code that
> > >> bypasses BAR register configuration when CONFIG_SPI_FLASH_BAR is
> > >> defined and SFDP is available.
> > >
> > > We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do
> you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is
> defined?
> > >
> >
> > I suggest skipping BAR configuration completely if flash supports
> > address width of 4 as per SFDP. BAR configuration is only needed if
> > flash does not support 4 byte addressing and flash size is > 16MB
> >
> > >> This patch will most certainly break SPI controller drivers(like
> > >> cadence_qspi.c) that expect sf layer to provide correct read opcode
> > >> and address width information, since there will be mismatch wrt
> > >> opcode used and number of address byte sent.
> > >> Not sure how this patch was tested, but I guess fsl_qspi modifies
> > >> read opcode internal to the driver and gets away with it.
> > >> Please modify the patch to update read_cmd/erase_cmd/write_cmd to
> > >> use
> > >> 4 byte addressing opcodes
> > >
> > > If the flash supports only supports 4-byte opcodes then there will
> > > be no mismatch wrt opcode used and number of address bytes sent
> > > since all of the opcodes that the flash supports will be 4-bytes opcodes.
> > > But if the flash supports both 3-byte as well as 4-byte address
> > > widths (which is a separate case) then this code will default the
> > > address mode to 4-byte but gives an option to use 3-byte by setting
> addrwd_3_in_use flag.
> > >
> >
> > Lets assume flash supports both 3 and 4 byte addressing (AFAIK,
> > majority of flash parts do that). In this case, this patch is
> > defaulting to 4 byte address width but
> > flash->read_cmd/write_cmd/erase_cmd are still set to 3 byte addressing
> opcodes.
> > So, SPI controller would end up send 3 addressing byte opcode but 4
> > byte of address. The flash would interpret last byte of address as
> > first byte of data and thus lead to data corruption in case of write.
> > Isn't that a problem?
Ok. But I'm curious if a flash supports both 3 and 4 byte address widths (and it does
not have BAR support), then why would one use 3-byte read/write/erase commands?
How would one access >16 MB in such case?
Also, initially I wanted to make SFDP optional so that none of the drivers are affected
and one could enable it using a flag as per requirement. Shall we get back to making
this optional (similar to v1 of this patch)?
Regards
Rajat
>
> Additionally, there are flash chips with a 3-/4-byte mode change instruction.
> But using 4-byte opcodes should be preferred.
>
> Simon
>
> >
> > In particular you are not parsing entire bfpt like in [1] and will
> > break flash operations
> >
> >
> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
> nor
> > %2Fspi-
> nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
> >
> cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0
> >
> %7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
> M428u1bT1bU
> > rXuX2GsSjb0%3D&reserved=0
> >
> > Regards
> > Vignesh
> >
> > > Regards
> > > Rajat
> > >
> > >>
> > >> Regards
> > >> Vigensh
> > >>
> > >>
> > >>> + default:
> > >>> + break;
> > >>> + }
> > >>> +
> > >>> + /* Flash Memory Density (in bits). */
> > >>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> > >>> + if (flash->size & BIT(31)) {
> > >>> + flash->size &= ~BIT(31);
> > >>> +
> > >>> + /*
> > >>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> > >>> + * bits is unlikely to exist so this error probably means
> > >>> + * the BFPT we are reading is corrupted/wrong.
> > >>> + */
> > >>> + if (flash->size > 63)
> > >>> + return -EINVAL;
> > >>> +
> > >>> + flash->size = 1ULL << flash->size;
> > >>> + } else {
> > >>> + flash->size++;
> > >>> + }
> > >>> + flash->size >>= 3; /* Convert to bytes. */
> > >>> +
> > >>> + /* Stop here if not JESD216 rev A or later. */
> > >>> + if (bfpt_header->length < BFPT_DWORD_MAX)
> > >>> + return 0;
> > >>> +
> > >>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> > >>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> > >>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> > >>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> > >>> + flash->page_size = 1U << flash->page_size;
> > >>> +
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> +/*
> > >>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
> > >> Parameters.
> > >>> + * @flash: pointer to a 'struct spi_flash'
> > >>> + *
> > >>> + * The Serial Flash Discoverable Parameters are described by the
> > >>> +JEDEC JESD216
> > >>> + * specification. This is a standard which tends to supported by
> > >>> +almost all
> > >>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us
> > >>> +to learn at
> > >>> + * runtime the main parameters needed to perform basic SPI flash
> > >> operations.
> > >>> + *
> > >>> + * Return: 0 on success, -errno otherwise.
> > >>> + */
> > >>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
> > >>> + const struct sfdp_parameter_header *param_header,
> > >> *bfpt_header;
> > >>> + struct sfdp_parameter_header *param_headers = NULL;
> > >>> + struct sfdp_header header;
> > >>> + size_t psize;
> > >>> + int i, err;
> > >>> +
> > >>> + /* Get the SFDP header. */
> > >>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> > >>> + if (err < 0)
> > >>> + return err;
> > >>> +
> > >>> + /* Check the SFDP header version. */
> > >>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> > >>> + header.major != SFDP_JESD216_MAJOR)
> > >>> + return -EINVAL;
> > >>> +
> > >>> + /*
> > >>> + * Verify that the first and only mandatory parameter header is a
> > >>> + * Basic Flash Parameter Table header as specified in JESD216.
> > >>> + */
> > >>> + bfpt_header = &header.bfpt_header;
> > >>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> > >>> + bfpt_header->major != SFDP_JESD216_MAJOR)
> > >>> + return -EINVAL;
> > >>> +
> > >>> + /*
> > >>> + * Allocate memory then read all parameter headers with a single
> > >>> + * Read SFDP command. These parameter headers will actually be
> > >> parsed
> > >>> + * twice: a first time to get the latest revision of the basic flash
> > >>> + * parameter table, then a second time to handle the supported
> > >> optional
> > >>> + * tables.
> > >>> + * Hence we read the parameter headers once for all to reduce the
> > >>> + * processing time
> > >>> + */
> > >>> + if (header.nph) {
> > >>> + psize = header.nph * sizeof(*param_headers);
> > >>> +
> > >>> + param_headers = malloc(psize);
> > >>> + if (!param_headers)
> > >>> + return -ENOMEM;
> > >>> +
> > >>> + err = spi_flash_read_sfdp(flash, sizeof(header),
> > >>> + psize, param_headers);
> > >>> + if (err < 0) {
> > >>> + dev_err(dev, "failed to read SFDP parameter
> > >> headers\n");
> > >>> + goto exit;
> > >>> + }
> > >>> + }
> > >>> +
> > >>> + /*
> > >>> + * Check other parameter headers to get the latest revision of
> > >>> + * the basic flash parameter table.
> > >>> + */
> > >>> + for (i = 0; i < header.nph; i++) {
> > >>> + param_header = ¶m_headers[i];
> > >>> +
> > >>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
> > >> SFDP_BFPT_ID &&
> > >>> + param_header->major == SFDP_JESD216_MAJOR &&
> > >>> + (param_header->minor > bfpt_header->minor ||
> > >>> + (param_header->minor == bfpt_header->minor &&
> > >>> + param_header->length > bfpt_header->length)))
> > >>> + bfpt_header = param_header;
> > >>> + }
> > >>> +
> > >>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
> > >>> + if (err)
> > >>> + goto exit;
> > >>> +
> > >>> +exit:
> > >>> + free(param_headers);
> > >>> + return err;
> > >>> +}
> > >>> +
> > >>> static int set_quad_mode(struct spi_flash *flash,
> > >>> const struct spi_flash_info *info) { @@
> > >>> -1130,7
> > >> +1366,7 @@ int
> > >>> spi_flash_scan(struct spi_flash *flash) {
> > >>> struct spi_slave *spi = flash->spi;
> > >>> const struct spi_flash_info *info = NULL;
> > >>> - int ret;
> > >>> + int ret, sfdp = 0;
> > >>>
> > >>> info = spi_flash_read_id(flash);
> > >>> if (IS_ERR_OR_NULL(info))
> > >>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> > >>> }
> > >>> #endif
> > >>>
> > >>> + spi->flash = flash;
> > >>> + flash->addrwd_3_in_use = false;
> > >>> +
> > >>> + /* Read Serial Flash Discoverable Parameters and initialize
> > >>> + * the following parameters of flash:
> > >>> + * 1. Flash size
> > >>> + * 2. Page size
> > >>> + * 3. Address width to be used for commands
> > >>> + */
> > >>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> > >>> + flash->size = 0;
> > >>> + sfdp = spi_flash_parse_sfdp(flash);
> > >>> + if (sfdp < 0)
> > >>> + debug("Unable to get SFDP information\n");
> > >>> + }
> > >>> +
> > >>> /* Compute the flash size */
> > >>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
> > >> 0;
> > >>> - flash->page_size = info->page_size;
> > >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > >>> + flash->page_size = info->page_size;
> > >>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> > >>> + }
> > >>> /*
> > >>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
> > >> pages,
> > >>> * yet use the 0x4d00 Extended JEDEC code. The rest of the
> > >>> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash
> *flash)
> > >>> }
> > >>> flash->page_size <<= flash->shift;
> > >>> flash->sector_size = info->sector_size << flash->shift;
> > >>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> > >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > >>> + flash->size = flash->sector_size * info->n_sectors <<
> > >>> + flash->shift;
> > >>> + }
> > >>> #ifdef CONFIG_SF_DUAL_FLASH
> > >>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> > >>> flash->size <<= 1;
> > >>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> > >>> #endif
> > >>>
> > >>> #ifndef CONFIG_SPI_FLASH_BAR
> > >>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> > >>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> > >>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> > >>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> > >>> + (flash->dual_flash == SF_SINGLE_FLASH &&
> > >>> + flash->size > SPI_FLASH_16MB_BOUN) ||
> > >>> + (flash->dual_flash > SF_SINGLE_FLASH &&
> > >>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> > >>> puts("SF: Warning - Only lower 16MiB accessible,");
> > >>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
> > >>> diff -
> > >> -git
> > >>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581
> > >>> 100644
> > >>> --- a/include/spi.h
> > >>> +++ b/include/spi.h
> > >>> @@ -10,6 +10,7 @@
> > >>> #define _SPI_H_
> > >>>
> > >>> #include <common.h>
> > >>> +#include <spi_flash.h>
> > >>>
> > >>> /* SPI mode flags */
> > >>> #define SPI_CPHA BIT(0) /* clock phase */
> > >>> @@ -103,6 +104,7 @@ struct spi_slave {
> > >>> unsigned int bus;
> > >>> unsigned int cs;
> > >>> #endif
> > >>> + struct spi_flash *flash;
> > >>> uint mode;
> > >>> unsigned int wordlen;
> > >>> unsigned int max_read_size;
> > >>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
> > >>> 0ec98fb55d..6558a4a1d5 100644
> > >>> --- a/include/spi_flash.h
> > >>> +++ b/include/spi_flash.h
> > >>> @@ -47,6 +47,9 @@ struct spi_slave;
> > >>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
> > >> read.
> > >>> * @write_cmd: Write cmd - page and quad program.
> > >>> * @dummy_byte: Dummy cycles for read operation.
> > >>> + * @cmd_len: Total length of command.
> > >>> + * @addr_width: Number of address bytes.
> > >>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
> > >> mode.
> > >>> * @memory_map: Address of read-only SPI flash access
> > >>> * @flash_lock: lock a region of the SPI Flash
> > >>> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6
> > >>> +85,9 @@ struct spi_flash {
> > >>> u8 read_cmd;
> > >>> u8 write_cmd;
> > >>> u8 dummy_byte;
> > >>> + u8 cmd_len;
> > >>> + u8 addr_width;
> > >>> + bool addrwd_3_in_use;
> > >>>
> > >>> void *memory_map;
> > >>>
> > >>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
> > >>>
> > >>> +/*
> > >>> + * Serial Flash Discoverable Parameter Headers */ struct
> > >>> +sfdp_parameter_header {
> > >>> + u8 id_lsb;
> > >>> + u8 minor;
> > >>> + u8 major;
> > >>> + u8 length; /* in double words */
> > >>> + u8 parameter_table_pointer[3]; /* byte address */
> > >>> + u8 id_msb;
> > >>> +};
> > >>> +
> > >>> +struct sfdp_header {
> > >>> + u32 signature; /* Ox50444653U <=> "SFDP" */
> > >>> + u8 minor;
> > >>> + u8 major;
> > >>> + u8 nph; /* 0-base number of parameter headers */
> > >>> + u8 unused;
> > >>> +
> > >>> + /* Basic Flash Parameter Table. */
> > >>> + struct sfdp_parameter_header bfpt_header;
> > >>> +};
> > >>> +
> > >>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) |
> > >>> +(p)->id_lsb) #define SFDP_PARAM_HEADER_PTP(p) \
> > >>> + (((p)->parameter_table_pointer[2] << 16) | \
> > >>> + ((p)->parameter_table_pointer[1] << 8) | \
> > >>> + ((p)->parameter_table_pointer[0] << 0))
> > >>> +
> > >>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
> > >> */
> > >>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> > >>> +
> > >>> +#define SFDP_SIGNATURE 0x50444653U
> > >>> +#define SFDP_JESD216_MAJOR 1
> > >>> +#define SFDP_JESD216_MINOR 0
> > >>> +#define SFDP_JESD216A_MINOR 5
> > >>> +#define SFDP_JESD216B_MINOR 6
> > >>> +
> > >>> +/* Basic Flash Parameter Table */
> > >>> +
> > >>> +/*
> > >>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> > >>> + * They are indexed from 1 but C arrays are indexed from 0.
> > >>> + */
> > >>> +#define BFPT_DWORD(i) ((i) - 1)
> > >>> +#define BFPT_DWORD_MAX 16
> > >>> +
> > >>> +/* The first version of JESB216 defined only 9 DWORDs. */
> > >>> +#define BFPT_DWORD_MAX_JESD216 9
> > >>> +
> > >>> +/* 1st DWORD. */
> > >>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
> > >> GENMASK(18, 17)
> > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> > >>> +#define BFPT_DWORD1_DTR BIT(19)
> > >>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> > >>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> > >>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> > >>> +
> > >>> +/* 5th DWORD. */
> > >>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> > >>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> > >>> +
> > >>> +/* 11th DWORD. */
> > >>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> > >>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
> > >> 4)
> > >>> +
> > >>> +/* 15th DWORD. */
> > >>> +
> > >>> +/*
> > >>> + * (from JESD216 rev B)
> > >>> + * Quad Enable Requirements (QER):
> > >>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-
> 4
> > >>> + * reads based on instruction. DQ3/HOLD# functions are hold
> during
> > >>> + * instruction phase.
> > >>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> > >>> + * two data bytes where bit 1 of the second byte is one.
> > >>> + * [...]
> > >>> + * Writing only one byte to the status register has the side-effect
> of
> > >>> + * clearing status register 2, including the QE bit. The 100b code is
> > >>> + * used if writing one byte to the status register does not modify
> > >>> + * status register 2.
> > >>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> > >>> + * one data byte where bit 6 is one.
> > >>> + * [...]
> > >>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> > >>> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> > >>> + * [...]
> > >>> + * The status register 2 is read using instruction 3Fh.
> > >>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> > >>> + * two data bytes where bit 1 of the second byte is one.
> > >>> + * [...]
> > >>> + * In contrast to the 001b code, writing one byte to the status
> > >>> + * register does not modify status register 2.
> > >>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> > >>> + * Read Status instruction 05h. Status register2 is read using
> > >>> + * instruction 35h. QE is set via Writ Status instruction 01h with
> > >>> + * two data bytes where bit 1 of the second byte is one.
> > >>> + * [...]
> > >>> + */
> > >>> +#define BFPT_DWORD15_QER_MASK
> > >> GENMASK(22, 20)
> > >>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
> > >> /* Micron */
> > >>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> > >>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> > >> Macronix */
> > >>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> > >>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> > >>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> > >> Spansion */
> > >>> +
> > >>> +struct sfdp_bfpt {
> > >>> + u32 dwords[BFPT_DWORD_MAX];
> > >>> +};
> > >>> +
> > >>> struct dm_spi_flash_ops {
> > >>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> > >>> int (*write)(struct udevice *dev, u32 offset, size_t len,
> > >>>
> >
> > --
> > Regards
> > Vignesh
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.denx.de%2Flistinfo%2Fu-
> boot&data=02%7C01%7Crajat.srivastava%40n
> >
> xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
> d99c5c
> >
> 301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
> y5eCmDx
> > 0KPmAHXvF8kusaJVUdM%3D&reserved=0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 12:59 ` Rajat Srivastava
@ 2018-11-16 13:09 ` Simon Goldschmidt
2018-11-16 15:25 ` Vignesh R
1 sibling, 0 replies; 13+ messages in thread
From: Simon Goldschmidt @ 2018-11-16 13:09 UTC (permalink / raw)
To: u-boot
On Fri, Nov 16, 2018 at 1:59 PM Rajat Srivastava
<rajat.srivastava@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > Sent: Friday, November 16, 2018 3:53 PM
> > To: vigneshr at ti.com
> > Cc: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u-
> > boot at lists.denx.de>; Jagan Teki <jagan@openedev.com>
> > Subject: Re: [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI
> > framework
> >
> > On Fri, Nov 16, 2018 at 11:19 AM Vignesh R <vigneshr@ti.com> wrote:
> > >
> > >
> > >
> > > On 16/11/18 3:10 PM, Rajat Srivastava wrote:
> > > >> Hi Rajat,
> > > >>
> > > >> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> > > >>> Add support for JESD216 rev B standard JEDEC Serial Flash
> > > >>> Discoverable Parameters (SFDP) tables to dynamically initialize
> > > >>> flash size, page size and address width of the flash. More
> > > >>> parameters can be added as per requirement.
> > > >>> SFDP parsing is made default but already existing method for
> > > >>> parsing these parameters are not deprecated.
> > > >>> A flag is created to skip SFDP parsing for a particular flash, if
> > > >>> required.
> > > >>>
> > > >>> SFDP data lets us auto-detect the addressing mode supported by the
> > > >>> flash which helps us access the flash using 4-byte address.
> > > >>>
> > > >>> Add a new argument in spi_flash_addr() function to create commands
> > > >>> with 3-byte or 4-byte address depending on the SFDP data read. Add
> > > >>> pointer to struct spi_flash in struct spi_slave so that driver can
> > > >>> have access to SFDP data.
> > > >>>
> > > >>> Introduce new structures and functions to read and parse SFDP data.
> > > >>> This is loosely based on Linux SFDP framework.
> > > >>>
> > > >>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> > > >>> ---
> > > >>> Changes in v2:
> > > >>> - Make SFDP parsing the default method.
> > > >>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
> > provide
> > > >>> an option to skip SFDP parsing for a particular flash.
> > > >>> ---
> > > >>
> > > >> [...]
> > > >>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> > > >>> + const struct sfdp_parameter_header
> > > >> *bfpt_header) {
> > > >>> + struct sfdp_bfpt bfpt;
> > > >>> + size_t len;
> > > >>> + int i, err;
> > > >>> + u32 addr;
> > > >>> +
> > > >>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
> > > >> */
> > > >>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> > > >>> + return -EINVAL;
> > > >>> +
> > > >>> + /* Read the Basic Flash Parameter Table. */
> > > >>> + len = min_t(size_t, sizeof(bfpt),
> > > >>> + bfpt_header->length * sizeof(u32));
> > > >>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> > > >>> + memset(&bfpt, 0, sizeof(bfpt));
> > > >>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> > > >>> + if (err < 0)
> > > >>> + return err;
> > > >>> +
> > > >>> + /* Fix endianness of the BFPT DWORDs. */
> > > >>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
> > > >>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> > > >>> +
> > > >>> + /* Number of address bytes. */
> > > >>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
> > > >> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> > > >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> > > >>> + flash->addr_width = 3;
> > > >>> + break;
> > > >>> +
> > > >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> > > >>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> > > >>> + printf("mode on command\n");
> > > >>> + /*
> > > >>> + * By default, 4-byte addressing mode is set.
> > > >>> + * To enforce 3-byte addressing mode, set
> > > >>> + addrwd_3_in_use
> > > >> flag
> > > >>> + * in struct spi_flash for every command.
> > > >>> + */
> > > >>> + flash->addr_width = 4;
> > > >>> + break;
> > > >>> +
> > > >>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > > >>> + flash->addr_width = 4;
> > > >>> + break;
> > > >>> +
> > > >>
> > > >> I see your are updating flash->addr_width to 4 bytes here. But I
> > > >> don't see any code in this patch that changes flash->read_cmd (and
> > > >> write/erase
> > > >> cmds) to use 4B addressing opcodes. Also, I dont see any code that
> > > >> bypasses BAR register configuration when CONFIG_SPI_FLASH_BAR is
> > > >> defined and SFDP is available.
> > > >
> > > > We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do
> > you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is
> > defined?
> > > >
> > >
> > > I suggest skipping BAR configuration completely if flash supports
> > > address width of 4 as per SFDP. BAR configuration is only needed if
> > > flash does not support 4 byte addressing and flash size is > 16MB
> > >
> > > >> This patch will most certainly break SPI controller drivers(like
> > > >> cadence_qspi.c) that expect sf layer to provide correct read opcode
> > > >> and address width information, since there will be mismatch wrt
> > > >> opcode used and number of address byte sent.
> > > >> Not sure how this patch was tested, but I guess fsl_qspi modifies
> > > >> read opcode internal to the driver and gets away with it.
> > > >> Please modify the patch to update read_cmd/erase_cmd/write_cmd to
> > > >> use
> > > >> 4 byte addressing opcodes
> > > >
> > > > If the flash supports only supports 4-byte opcodes then there will
> > > > be no mismatch wrt opcode used and number of address bytes sent
> > > > since all of the opcodes that the flash supports will be 4-bytes opcodes.
> > > > But if the flash supports both 3-byte as well as 4-byte address
> > > > widths (which is a separate case) then this code will default the
> > > > address mode to 4-byte but gives an option to use 3-byte by setting
> > addrwd_3_in_use flag.
> > > >
> > >
> > > Lets assume flash supports both 3 and 4 byte addressing (AFAIK,
> > > majority of flash parts do that). In this case, this patch is
> > > defaulting to 4 byte address width but
> > > flash->read_cmd/write_cmd/erase_cmd are still set to 3 byte addressing
> > opcodes.
> > > So, SPI controller would end up send 3 addressing byte opcode but 4
> > > byte of address. The flash would interpret last byte of address as
> > > first byte of data and thus lead to data corruption in case of write.
> > > Isn't that a problem?
>
> Ok. But I'm curious if a flash supports both 3 and 4 byte address widths (and it does
> not have BAR support), then why would one use 3-byte read/write/erase commands?
> How would one access >16 MB in such case?
>
> Also, initially I wanted to make SFDP optional so that none of the drivers are affected
> and one could enable it using a flag as per requirement. Shall we get back to making
> this optional (similar to v1 of this patch)?
My preference here is to make SFDP the default in future. If driver
support is required and we can't implement support on all drivers
right away, compatibility workaraounds are needed.
However, I don't think making it an opt-in option as in v1 is a good idea...
Simon
>
> Regards
> Rajat
>
> >
> > Additionally, there are flash chips with a 3-/4-byte mode change instruction.
> > But using 4-byte opcodes should be preferred.
> >
> > Simon
> >
> > >
> > > In particular you are not parsing entire bfpt like in [1] and will
> > > break flash operations
> > >
> > >
> > [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
> > nor
> > > %2Fspi-
> > nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
> > >
> > cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0
> > >
> > %7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
> > M428u1bT1bU
> > > rXuX2GsSjb0%3D&reserved=0
> > >
> > > Regards
> > > Vignesh
> > >
> > > > Regards
> > > > Rajat
> > > >
> > > >>
> > > >> Regards
> > > >> Vigensh
> > > >>
> > > >>
> > > >>> + default:
> > > >>> + break;
> > > >>> + }
> > > >>> +
> > > >>> + /* Flash Memory Density (in bits). */
> > > >>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> > > >>> + if (flash->size & BIT(31)) {
> > > >>> + flash->size &= ~BIT(31);
> > > >>> +
> > > >>> + /*
> > > >>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> > > >>> + * bits is unlikely to exist so this error probably means
> > > >>> + * the BFPT we are reading is corrupted/wrong.
> > > >>> + */
> > > >>> + if (flash->size > 63)
> > > >>> + return -EINVAL;
> > > >>> +
> > > >>> + flash->size = 1ULL << flash->size;
> > > >>> + } else {
> > > >>> + flash->size++;
> > > >>> + }
> > > >>> + flash->size >>= 3; /* Convert to bytes. */
> > > >>> +
> > > >>> + /* Stop here if not JESD216 rev A or later. */
> > > >>> + if (bfpt_header->length < BFPT_DWORD_MAX)
> > > >>> + return 0;
> > > >>> +
> > > >>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> > > >>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> > > >>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> > > >>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> > > >>> + flash->page_size = 1U << flash->page_size;
> > > >>> +
> > > >>> + return 0;
> > > >>> +}
> > > >>> +
> > > >>> +/*
> > > >>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
> > > >> Parameters.
> > > >>> + * @flash: pointer to a 'struct spi_flash'
> > > >>> + *
> > > >>> + * The Serial Flash Discoverable Parameters are described by the
> > > >>> +JEDEC JESD216
> > > >>> + * specification. This is a standard which tends to supported by
> > > >>> +almost all
> > > >>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us
> > > >>> +to learn at
> > > >>> + * runtime the main parameters needed to perform basic SPI flash
> > > >> operations.
> > > >>> + *
> > > >>> + * Return: 0 on success, -errno otherwise.
> > > >>> + */
> > > >>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
> > > >>> + const struct sfdp_parameter_header *param_header,
> > > >> *bfpt_header;
> > > >>> + struct sfdp_parameter_header *param_headers = NULL;
> > > >>> + struct sfdp_header header;
> > > >>> + size_t psize;
> > > >>> + int i, err;
> > > >>> +
> > > >>> + /* Get the SFDP header. */
> > > >>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> > > >>> + if (err < 0)
> > > >>> + return err;
> > > >>> +
> > > >>> + /* Check the SFDP header version. */
> > > >>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> > > >>> + header.major != SFDP_JESD216_MAJOR)
> > > >>> + return -EINVAL;
> > > >>> +
> > > >>> + /*
> > > >>> + * Verify that the first and only mandatory parameter header is a
> > > >>> + * Basic Flash Parameter Table header as specified in JESD216.
> > > >>> + */
> > > >>> + bfpt_header = &header.bfpt_header;
> > > >>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> > > >>> + bfpt_header->major != SFDP_JESD216_MAJOR)
> > > >>> + return -EINVAL;
> > > >>> +
> > > >>> + /*
> > > >>> + * Allocate memory then read all parameter headers with a single
> > > >>> + * Read SFDP command. These parameter headers will actually be
> > > >> parsed
> > > >>> + * twice: a first time to get the latest revision of the basic flash
> > > >>> + * parameter table, then a second time to handle the supported
> > > >> optional
> > > >>> + * tables.
> > > >>> + * Hence we read the parameter headers once for all to reduce the
> > > >>> + * processing time
> > > >>> + */
> > > >>> + if (header.nph) {
> > > >>> + psize = header.nph * sizeof(*param_headers);
> > > >>> +
> > > >>> + param_headers = malloc(psize);
> > > >>> + if (!param_headers)
> > > >>> + return -ENOMEM;
> > > >>> +
> > > >>> + err = spi_flash_read_sfdp(flash, sizeof(header),
> > > >>> + psize, param_headers);
> > > >>> + if (err < 0) {
> > > >>> + dev_err(dev, "failed to read SFDP parameter
> > > >> headers\n");
> > > >>> + goto exit;
> > > >>> + }
> > > >>> + }
> > > >>> +
> > > >>> + /*
> > > >>> + * Check other parameter headers to get the latest revision of
> > > >>> + * the basic flash parameter table.
> > > >>> + */
> > > >>> + for (i = 0; i < header.nph; i++) {
> > > >>> + param_header = ¶m_headers[i];
> > > >>> +
> > > >>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
> > > >> SFDP_BFPT_ID &&
> > > >>> + param_header->major == SFDP_JESD216_MAJOR &&
> > > >>> + (param_header->minor > bfpt_header->minor ||
> > > >>> + (param_header->minor == bfpt_header->minor &&
> > > >>> + param_header->length > bfpt_header->length)))
> > > >>> + bfpt_header = param_header;
> > > >>> + }
> > > >>> +
> > > >>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
> > > >>> + if (err)
> > > >>> + goto exit;
> > > >>> +
> > > >>> +exit:
> > > >>> + free(param_headers);
> > > >>> + return err;
> > > >>> +}
> > > >>> +
> > > >>> static int set_quad_mode(struct spi_flash *flash,
> > > >>> const struct spi_flash_info *info) { @@
> > > >>> -1130,7
> > > >> +1366,7 @@ int
> > > >>> spi_flash_scan(struct spi_flash *flash) {
> > > >>> struct spi_slave *spi = flash->spi;
> > > >>> const struct spi_flash_info *info = NULL;
> > > >>> - int ret;
> > > >>> + int ret, sfdp = 0;
> > > >>>
> > > >>> info = spi_flash_read_id(flash);
> > > >>> if (IS_ERR_OR_NULL(info))
> > > >>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> > > >>> }
> > > >>> #endif
> > > >>>
> > > >>> + spi->flash = flash;
> > > >>> + flash->addrwd_3_in_use = false;
> > > >>> +
> > > >>> + /* Read Serial Flash Discoverable Parameters and initialize
> > > >>> + * the following parameters of flash:
> > > >>> + * 1. Flash size
> > > >>> + * 2. Page size
> > > >>> + * 3. Address width to be used for commands
> > > >>> + */
> > > >>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> > > >>> + flash->size = 0;
> > > >>> + sfdp = spi_flash_parse_sfdp(flash);
> > > >>> + if (sfdp < 0)
> > > >>> + debug("Unable to get SFDP information\n");
> > > >>> + }
> > > >>> +
> > > >>> /* Compute the flash size */
> > > >>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
> > > >> 0;
> > > >>> - flash->page_size = info->page_size;
> > > >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > > >>> + flash->page_size = info->page_size;
> > > >>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> > > >>> + }
> > > >>> /*
> > > >>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
> > > >> pages,
> > > >>> * yet use the 0x4d00 Extended JEDEC code. The rest of the
> > > >>> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash
> > *flash)
> > > >>> }
> > > >>> flash->page_size <<= flash->shift;
> > > >>> flash->sector_size = info->sector_size << flash->shift;
> > > >>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> > > >>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> > > >>> + flash->size = flash->sector_size * info->n_sectors <<
> > > >>> + flash->shift;
> > > >>> + }
> > > >>> #ifdef CONFIG_SF_DUAL_FLASH
> > > >>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> > > >>> flash->size <<= 1;
> > > >>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> > > >>> #endif
> > > >>>
> > > >>> #ifndef CONFIG_SPI_FLASH_BAR
> > > >>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> > > >>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> > > >>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> > > >>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> > > >>> + (flash->dual_flash == SF_SINGLE_FLASH &&
> > > >>> + flash->size > SPI_FLASH_16MB_BOUN) ||
> > > >>> + (flash->dual_flash > SF_SINGLE_FLASH &&
> > > >>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> > > >>> puts("SF: Warning - Only lower 16MiB accessible,");
> > > >>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
> > > >>> diff -
> > > >> -git
> > > >>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581
> > > >>> 100644
> > > >>> --- a/include/spi.h
> > > >>> +++ b/include/spi.h
> > > >>> @@ -10,6 +10,7 @@
> > > >>> #define _SPI_H_
> > > >>>
> > > >>> #include <common.h>
> > > >>> +#include <spi_flash.h>
> > > >>>
> > > >>> /* SPI mode flags */
> > > >>> #define SPI_CPHA BIT(0) /* clock phase */
> > > >>> @@ -103,6 +104,7 @@ struct spi_slave {
> > > >>> unsigned int bus;
> > > >>> unsigned int cs;
> > > >>> #endif
> > > >>> + struct spi_flash *flash;
> > > >>> uint mode;
> > > >>> unsigned int wordlen;
> > > >>> unsigned int max_read_size;
> > > >>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
> > > >>> 0ec98fb55d..6558a4a1d5 100644
> > > >>> --- a/include/spi_flash.h
> > > >>> +++ b/include/spi_flash.h
> > > >>> @@ -47,6 +47,9 @@ struct spi_slave;
> > > >>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
> > > >> read.
> > > >>> * @write_cmd: Write cmd - page and quad program.
> > > >>> * @dummy_byte: Dummy cycles for read operation.
> > > >>> + * @cmd_len: Total length of command.
> > > >>> + * @addr_width: Number of address bytes.
> > > >>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
> > > >> mode.
> > > >>> * @memory_map: Address of read-only SPI flash access
> > > >>> * @flash_lock: lock a region of the SPI Flash
> > > >>> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6
> > > >>> +85,9 @@ struct spi_flash {
> > > >>> u8 read_cmd;
> > > >>> u8 write_cmd;
> > > >>> u8 dummy_byte;
> > > >>> + u8 cmd_len;
> > > >>> + u8 addr_width;
> > > >>> + bool addrwd_3_in_use;
> > > >>>
> > > >>> void *memory_map;
> > > >>>
> > > >>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
> > > >>>
> > > >>> +/*
> > > >>> + * Serial Flash Discoverable Parameter Headers */ struct
> > > >>> +sfdp_parameter_header {
> > > >>> + u8 id_lsb;
> > > >>> + u8 minor;
> > > >>> + u8 major;
> > > >>> + u8 length; /* in double words */
> > > >>> + u8 parameter_table_pointer[3]; /* byte address */
> > > >>> + u8 id_msb;
> > > >>> +};
> > > >>> +
> > > >>> +struct sfdp_header {
> > > >>> + u32 signature; /* Ox50444653U <=> "SFDP" */
> > > >>> + u8 minor;
> > > >>> + u8 major;
> > > >>> + u8 nph; /* 0-base number of parameter headers */
> > > >>> + u8 unused;
> > > >>> +
> > > >>> + /* Basic Flash Parameter Table. */
> > > >>> + struct sfdp_parameter_header bfpt_header;
> > > >>> +};
> > > >>> +
> > > >>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) |
> > > >>> +(p)->id_lsb) #define SFDP_PARAM_HEADER_PTP(p) \
> > > >>> + (((p)->parameter_table_pointer[2] << 16) | \
> > > >>> + ((p)->parameter_table_pointer[1] << 8) | \
> > > >>> + ((p)->parameter_table_pointer[0] << 0))
> > > >>> +
> > > >>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
> > > >> */
> > > >>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> > > >>> +
> > > >>> +#define SFDP_SIGNATURE 0x50444653U
> > > >>> +#define SFDP_JESD216_MAJOR 1
> > > >>> +#define SFDP_JESD216_MINOR 0
> > > >>> +#define SFDP_JESD216A_MINOR 5
> > > >>> +#define SFDP_JESD216B_MINOR 6
> > > >>> +
> > > >>> +/* Basic Flash Parameter Table */
> > > >>> +
> > > >>> +/*
> > > >>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> > > >>> + * They are indexed from 1 but C arrays are indexed from 0.
> > > >>> + */
> > > >>> +#define BFPT_DWORD(i) ((i) - 1)
> > > >>> +#define BFPT_DWORD_MAX 16
> > > >>> +
> > > >>> +/* The first version of JESB216 defined only 9 DWORDs. */
> > > >>> +#define BFPT_DWORD_MAX_JESD216 9
> > > >>> +
> > > >>> +/* 1st DWORD. */
> > > >>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> > > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
> > > >> GENMASK(18, 17)
> > > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> > > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> > > >>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> > > >>> +#define BFPT_DWORD1_DTR BIT(19)
> > > >>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> > > >>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> > > >>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> > > >>> +
> > > >>> +/* 5th DWORD. */
> > > >>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> > > >>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> > > >>> +
> > > >>> +/* 11th DWORD. */
> > > >>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> > > >>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
> > > >> 4)
> > > >>> +
> > > >>> +/* 15th DWORD. */
> > > >>> +
> > > >>> +/*
> > > >>> + * (from JESD216 rev B)
> > > >>> + * Quad Enable Requirements (QER):
> > > >>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-
> > 4
> > > >>> + * reads based on instruction. DQ3/HOLD# functions are hold
> > during
> > > >>> + * instruction phase.
> > > >>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> > > >>> + * two data bytes where bit 1 of the second byte is one.
> > > >>> + * [...]
> > > >>> + * Writing only one byte to the status register has the side-effect
> > of
> > > >>> + * clearing status register 2, including the QE bit. The 100b code is
> > > >>> + * used if writing one byte to the status register does not modify
> > > >>> + * status register 2.
> > > >>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> > > >>> + * one data byte where bit 6 is one.
> > > >>> + * [...]
> > > >>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> > > >>> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> > > >>> + * [...]
> > > >>> + * The status register 2 is read using instruction 3Fh.
> > > >>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> > > >>> + * two data bytes where bit 1 of the second byte is one.
> > > >>> + * [...]
> > > >>> + * In contrast to the 001b code, writing one byte to the status
> > > >>> + * register does not modify status register 2.
> > > >>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> > > >>> + * Read Status instruction 05h. Status register2 is read using
> > > >>> + * instruction 35h. QE is set via Writ Status instruction 01h with
> > > >>> + * two data bytes where bit 1 of the second byte is one.
> > > >>> + * [...]
> > > >>> + */
> > > >>> +#define BFPT_DWORD15_QER_MASK
> > > >> GENMASK(22, 20)
> > > >>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
> > > >> /* Micron */
> > > >>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> > > >>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> > > >> Macronix */
> > > >>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> > > >>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> > > >>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> > > >> Spansion */
> > > >>> +
> > > >>> +struct sfdp_bfpt {
> > > >>> + u32 dwords[BFPT_DWORD_MAX];
> > > >>> +};
> > > >>> +
> > > >>> struct dm_spi_flash_ops {
> > > >>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> > > >>> int (*write)(struct udevice *dev, u32 offset, size_t len,
> > > >>>
> > >
> > > --
> > > Regards
> > > Vignesh
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > > ts.denx.de%2Flistinfo%2Fu-
> > boot&data=02%7C01%7Crajat.srivastava%40n
> > >
> > xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
> > d99c5c
> > >
> > 301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
> > y5eCmDx
> > > 0KPmAHXvF8kusaJVUdM%3D&reserved=0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 12:59 ` Rajat Srivastava
2018-11-16 13:09 ` Simon Goldschmidt
@ 2018-11-16 15:25 ` Vignesh R
2018-12-19 9:01 ` Rajat Srivastava
1 sibling, 1 reply; 13+ messages in thread
From: Vignesh R @ 2018-11-16 15:25 UTC (permalink / raw)
To: u-boot
On 16-Nov-18 6:29 PM, Rajat Srivastava wrote:
>> On Fri, Nov 16, 2018 at 11:19 AM Vignesh R <vigneshr@ti.com> wrote:
>>> On 16/11/18 3:10 PM, Rajat Srivastava wrote:
>>>>> Hi Rajat,
>>>>>
>>>>> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
>>>>>> Add support for JESD216 rev B standard JEDEC Serial Flash
>>>>>> Discoverable Parameters (SFDP) tables to dynamically initialize
>>>>>> flash size, page size and address width of the flash. More
>>>>>> parameters can be added as per requirement.
>>>>>> SFDP parsing is made default but already existing method for
>>>>>> parsing these parameters are not deprecated.
>>>>>> A flag is created to skip SFDP parsing for a particular flash, if
>>>>>> required.
>>>>>>
>>>>>> SFDP data lets us auto-detect the addressing mode supported by the
>>>>>> flash which helps us access the flash using 4-byte address.
>>>>>>
>>>>>> Add a new argument in spi_flash_addr() function to create commands
>>>>>> with 3-byte or 4-byte address depending on the SFDP data read. Add
>>>>>> pointer to struct spi_flash in struct spi_slave so that driver can
>>>>>> have access to SFDP data.
>>>>>>
>>>>>> Introduce new structures and functions to read and parse SFDP data.
>>>>>> This is loosely based on Linux SFDP framework.
>>>>>>
>>>>>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - Make SFDP parsing the default method.
>>>>>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
>> provide
>>>>>> an option to skip SFDP parsing for a particular flash.
>>>>>> ---
>>>>>
>>>>> [...]
>>>>>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
>>>>>> + const struct sfdp_parameter_header
>>>>> *bfpt_header) {
>>>>>> + struct sfdp_bfpt bfpt;
>>>>>> + size_t len;
>>>>>> + int i, err;
>>>>>> + u32 addr;
>>>>>> +
>>>>>> + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs.
>>>>> */
>>>>>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /* Read the Basic Flash Parameter Table. */
>>>>>> + len = min_t(size_t, sizeof(bfpt),
>>>>>> + bfpt_header->length * sizeof(u32));
>>>>>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
>>>>>> + memset(&bfpt, 0, sizeof(bfpt));
>>>>>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
>>>>>> + if (err < 0)
>>>>>> + return err;
>>>>>> +
>>>>>> + /* Fix endianness of the BFPT DWORDs. */
>>>>>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
>>>>>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
>>>>>> +
>>>>>> + /* Number of address bytes. */
>>>>>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
>>>>> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>>>>> + flash->addr_width = 3;
>>>>>> + break;
>>>>>> +
>>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>>>>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
>>>>>> + printf("mode on command\n");
>>>>>> + /*
>>>>>> + * By default, 4-byte addressing mode is set.
>>>>>> + * To enforce 3-byte addressing mode, set
>>>>>> + addrwd_3_in_use
>>>>> flag
>>>>>> + * in struct spi_flash for every command.
>>>>>> + */
>>>>>> + flash->addr_width = 4;
>>>>>> + break;
>>>>>> +
>>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
>>>>>> + flash->addr_width = 4;
>>>>>> + break;
>>>>>> +
>>>>>
>>>>> I see your are updating flash->addr_width to 4 bytes here. But I
>>>>> don't see any code in this patch that changes flash->read_cmd (and
>>>>> write/erase
>>>>> cmds) to use 4B addressing opcodes. Also, I dont see any code that
>>>>> bypasses BAR register configuration when CONFIG_SPI_FLASH_BAR is
>>>>> defined and SFDP is available.
>>>>
>>>> We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What do
>> you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR is
>> defined?
>>>>
>>>
>>> I suggest skipping BAR configuration completely if flash supports
>>> address width of 4 as per SFDP. BAR configuration is only needed if
>>> flash does not support 4 byte addressing and flash size is > 16MB
>>>
>>>>> This patch will most certainly break SPI controller drivers(like
>>>>> cadence_qspi.c) that expect sf layer to provide correct read opcode
>>>>> and address width information, since there will be mismatch wrt
>>>>> opcode used and number of address byte sent.
>>>>> Not sure how this patch was tested, but I guess fsl_qspi modifies
>>>>> read opcode internal to the driver and gets away with it.
>>>>> Please modify the patch to update read_cmd/erase_cmd/write_cmd to
>>>>> use
>>>>> 4 byte addressing opcodes
>>>>
>>>> If the flash supports only supports 4-byte opcodes then there will
>>>> be no mismatch wrt opcode used and number of address bytes sent
>>>> since all of the opcodes that the flash supports will be 4-bytes opcodes.
>>>> But if the flash supports both 3-byte as well as 4-byte address
>>>> widths (which is a separate case) then this code will default the
>>>> address mode to 4-byte but gives an option to use 3-byte by setting
>> addrwd_3_in_use flag.
>>>>
>>>
>>> Lets assume flash supports both 3 and 4 byte addressing (AFAIK,
>>> majority of flash parts do that). In this case, this patch is
>>> defaulting to 4 byte address width but
>>> flash->read_cmd/write_cmd/erase_cmd are still set to 3 byte addressing
>> opcodes.
>>> So, SPI controller would end up send 3 addressing byte opcode but 4
>>> byte of address. The flash would interpret last byte of address as
>>> first byte of data and thus lead to data corruption in case of write.
>>> Isn't that a problem?
>
> Ok. But I'm curious if a flash supports both 3 and 4 byte address widths (and it does
> not have BAR support), then why would one use 3-byte read/write/erase commands?
> How would one access >16 MB in such case?
>
Unfortunately thats a problem with U-Boot spi_flash layer today. Many
newer flashes like Micron's mt35x series dont support BAR registers.
Accessing above 16MB is broken for such flashes today.
> Also, initially I wanted to make SFDP optional so that none of the drivers are affected
> and one could enable it using a flag as per requirement. Shall we get back to making
> this optional (similar to v1 of this patch)?
>
How does that help? With opt-in approach you mark some flash devices as
SFDP capable, but you still don't update opcodes, thus breaking boards
that have such flash devices. So, porting full bfpt parsing and always
using opcode to 4 byte addressing opcodes when available will be needed.
Thus you would end up with SFDP as default instead of opt-in.
> Regards
> Rajat
>
>>
>> Additionally, there are flash chips with a 3-/4-byte mode change instruction.
>> But using 4-byte opcodes should be preferred.
>>
>> Simon
>>
>>>
>>> In particular you are not parsing entire bfpt like in [1] and will
>>> break flash operations
>>>
>>>
>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>>> elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
>> nor
>>> %2Fspi-
>> nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
>>>
>> cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c301635
>> %7C0
>>>
>> %7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
>> M428u1bT1bU
>>> rXuX2GsSjb0%3D&reserved=0
>>>
>>> Regards
>>> Vignesh
>>>
>>>> Regards
>>>> Rajat
>>>>
>>>>>
>>>>> Regards
>>>>> Vigensh
>>>>>
>>>>>
>>>>>> + default:
>>>>>> + break;
>>>>>> + }
>>>>>> +
>>>>>> + /* Flash Memory Density (in bits). */
>>>>>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
>>>>>> + if (flash->size & BIT(31)) {
>>>>>> + flash->size &= ~BIT(31);
>>>>>> +
>>>>>> + /*
>>>>>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
>>>>>> + * bits is unlikely to exist so this error probably means
>>>>>> + * the BFPT we are reading is corrupted/wrong.
>>>>>> + */
>>>>>> + if (flash->size > 63)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + flash->size = 1ULL << flash->size;
>>>>>> + } else {
>>>>>> + flash->size++;
>>>>>> + }
>>>>>> + flash->size >>= 3; /* Convert to bytes. */
>>>>>> +
>>>>>> + /* Stop here if not JESD216 rev A or later. */
>>>>>> + if (bfpt_header->length < BFPT_DWORD_MAX)
>>>>>> + return 0;
>>>>>> +
>>>>>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
>>>>>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
>>>>>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
>>>>>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
>>>>>> + flash->page_size = 1U << flash->page_size;
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
>>>>> Parameters.
>>>>>> + * @flash: pointer to a 'struct spi_flash'
>>>>>> + *
>>>>>> + * The Serial Flash Discoverable Parameters are described by the
>>>>>> +JEDEC JESD216
>>>>>> + * specification. This is a standard which tends to supported by
>>>>>> +almost all
>>>>>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us
>>>>>> +to learn at
>>>>>> + * runtime the main parameters needed to perform basic SPI flash
>>>>> operations.
>>>>>> + *
>>>>>> + * Return: 0 on success, -errno otherwise.
>>>>>> + */
>>>>>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
>>>>>> + const struct sfdp_parameter_header *param_header,
>>>>> *bfpt_header;
>>>>>> + struct sfdp_parameter_header *param_headers = NULL;
>>>>>> + struct sfdp_header header;
>>>>>> + size_t psize;
>>>>>> + int i, err;
>>>>>> +
>>>>>> + /* Get the SFDP header. */
>>>>>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
>>>>>> + if (err < 0)
>>>>>> + return err;
>>>>>> +
>>>>>> + /* Check the SFDP header version. */
>>>>>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
>>>>>> + header.major != SFDP_JESD216_MAJOR)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /*
>>>>>> + * Verify that the first and only mandatory parameter header is a
>>>>>> + * Basic Flash Parameter Table header as specified in JESD216.
>>>>>> + */
>>>>>> + bfpt_header = &header.bfpt_header;
>>>>>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
>>>>>> + bfpt_header->major != SFDP_JESD216_MAJOR)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + /*
>>>>>> + * Allocate memory then read all parameter headers with a single
>>>>>> + * Read SFDP command. These parameter headers will actually be
>>>>> parsed
>>>>>> + * twice: a first time to get the latest revision of the basic flash
>>>>>> + * parameter table, then a second time to handle the supported
>>>>> optional
>>>>>> + * tables.
>>>>>> + * Hence we read the parameter headers once for all to reduce the
>>>>>> + * processing time
>>>>>> + */
>>>>>> + if (header.nph) {
>>>>>> + psize = header.nph * sizeof(*param_headers);
>>>>>> +
>>>>>> + param_headers = malloc(psize);
>>>>>> + if (!param_headers)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + err = spi_flash_read_sfdp(flash, sizeof(header),
>>>>>> + psize, param_headers);
>>>>>> + if (err < 0) {
>>>>>> + dev_err(dev, "failed to read SFDP parameter
>>>>> headers\n");
>>>>>> + goto exit;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + /*
>>>>>> + * Check other parameter headers to get the latest revision of
>>>>>> + * the basic flash parameter table.
>>>>>> + */
>>>>>> + for (i = 0; i < header.nph; i++) {
>>>>>> + param_header = ¶m_headers[i];
>>>>>> +
>>>>>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
>>>>> SFDP_BFPT_ID &&
>>>>>> + param_header->major == SFDP_JESD216_MAJOR &&
>>>>>> + (param_header->minor > bfpt_header->minor ||
>>>>>> + (param_header->minor == bfpt_header->minor &&
>>>>>> + param_header->length > bfpt_header->length)))
>>>>>> + bfpt_header = param_header;
>>>>>> + }
>>>>>> +
>>>>>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
>>>>>> + if (err)
>>>>>> + goto exit;
>>>>>> +
>>>>>> +exit:
>>>>>> + free(param_headers);
>>>>>> + return err;
>>>>>> +}
>>>>>> +
>>>>>> static int set_quad_mode(struct spi_flash *flash,
>>>>>> const struct spi_flash_info *info) { @@
>>>>>> -1130,7
>>>>> +1366,7 @@ int
>>>>>> spi_flash_scan(struct spi_flash *flash) {
>>>>>> struct spi_slave *spi = flash->spi;
>>>>>> const struct spi_flash_info *info = NULL;
>>>>>> - int ret;
>>>>>> + int ret, sfdp = 0;
>>>>>>
>>>>>> info = spi_flash_read_id(flash);
>>>>>> if (IS_ERR_OR_NULL(info))
>>>>>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>> }
>>>>>> #endif
>>>>>>
>>>>>> + spi->flash = flash;
>>>>>> + flash->addrwd_3_in_use = false;
>>>>>> +
>>>>>> + /* Read Serial Flash Discoverable Parameters and initialize
>>>>>> + * the following parameters of flash:
>>>>>> + * 1. Flash size
>>>>>> + * 2. Page size
>>>>>> + * 3. Address width to be used for commands
>>>>>> + */
>>>>>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
>>>>>> + flash->size = 0;
>>>>>> + sfdp = spi_flash_parse_sfdp(flash);
>>>>>> + if (sfdp < 0)
>>>>>> + debug("Unable to get SFDP information\n");
>>>>>> + }
>>>>>> +
>>>>>> /* Compute the flash size */
>>>>>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
>>>>> 0;
>>>>>> - flash->page_size = info->page_size;
>>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>>>>> + flash->page_size = info->page_size;
>>>>>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
>>>>>> + }
>>>>>> /*
>>>>>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
>>>>> pages,
>>>>>> * yet use the 0x4d00 Extended JEDEC code. The rest of the
>>>>>> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct spi_flash
>> *flash)
>>>>>> }
>>>>>> flash->page_size <<= flash->shift;
>>>>>> flash->sector_size = info->sector_size << flash->shift;
>>>>>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
>>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>>>>> + flash->size = flash->sector_size * info->n_sectors <<
>>>>>> + flash->shift;
>>>>>> + }
>>>>>> #ifdef CONFIG_SF_DUAL_FLASH
>>>>>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
>>>>>> flash->size <<= 1;
>>>>>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>> #endif
>>>>>>
>>>>>> #ifndef CONFIG_SPI_FLASH_BAR
>>>>>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
>>>>>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
>>>>>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
>>>>>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
>>>>>> + (flash->dual_flash == SF_SINGLE_FLASH &&
>>>>>> + flash->size > SPI_FLASH_16MB_BOUN) ||
>>>>>> + (flash->dual_flash > SF_SINGLE_FLASH &&
>>>>>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
>>>>>> puts("SF: Warning - Only lower 16MiB accessible,");
>>>>>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
>>>>>> diff -
>>>>> -git
>>>>>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581
>>>>>> 100644
>>>>>> --- a/include/spi.h
>>>>>> +++ b/include/spi.h
>>>>>> @@ -10,6 +10,7 @@
>>>>>> #define _SPI_H_
>>>>>>
>>>>>> #include <common.h>
>>>>>> +#include <spi_flash.h>
>>>>>>
>>>>>> /* SPI mode flags */
>>>>>> #define SPI_CPHA BIT(0) /* clock phase */
>>>>>> @@ -103,6 +104,7 @@ struct spi_slave {
>>>>>> unsigned int bus;
>>>>>> unsigned int cs;
>>>>>> #endif
>>>>>> + struct spi_flash *flash;
>>>>>> uint mode;
>>>>>> unsigned int wordlen;
>>>>>> unsigned int max_read_size;
>>>>>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
>>>>>> 0ec98fb55d..6558a4a1d5 100644
>>>>>> --- a/include/spi_flash.h
>>>>>> +++ b/include/spi_flash.h
>>>>>> @@ -47,6 +47,9 @@ struct spi_slave;
>>>>>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
>>>>> read.
>>>>>> * @write_cmd: Write cmd - page and quad program.
>>>>>> * @dummy_byte: Dummy cycles for read operation.
>>>>>> + * @cmd_len: Total length of command.
>>>>>> + * @addr_width: Number of address bytes.
>>>>>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
>>>>> mode.
>>>>>> * @memory_map: Address of read-only SPI flash access
>>>>>> * @flash_lock: lock a region of the SPI Flash
>>>>>> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6
>>>>>> +85,9 @@ struct spi_flash {
>>>>>> u8 read_cmd;
>>>>>> u8 write_cmd;
>>>>>> u8 dummy_byte;
>>>>>> + u8 cmd_len;
>>>>>> + u8 addr_width;
>>>>>> + bool addrwd_3_in_use;
>>>>>>
>>>>>> void *memory_map;
>>>>>>
>>>>>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
>>>>>>
>>>>>> +/*
>>>>>> + * Serial Flash Discoverable Parameter Headers */ struct
>>>>>> +sfdp_parameter_header {
>>>>>> + u8 id_lsb;
>>>>>> + u8 minor;
>>>>>> + u8 major;
>>>>>> + u8 length; /* in double words */
>>>>>> + u8 parameter_table_pointer[3]; /* byte address */
>>>>>> + u8 id_msb;
>>>>>> +};
>>>>>> +
>>>>>> +struct sfdp_header {
>>>>>> + u32 signature; /* Ox50444653U <=> "SFDP" */
>>>>>> + u8 minor;
>>>>>> + u8 major;
>>>>>> + u8 nph; /* 0-base number of parameter headers */
>>>>>> + u8 unused;
>>>>>> +
>>>>>> + /* Basic Flash Parameter Table. */
>>>>>> + struct sfdp_parameter_header bfpt_header;
>>>>>> +};
>>>>>> +
>>>>>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) |
>>>>>> +(p)->id_lsb) #define SFDP_PARAM_HEADER_PTP(p) \
>>>>>> + (((p)->parameter_table_pointer[2] << 16) | \
>>>>>> + ((p)->parameter_table_pointer[1] << 8) | \
>>>>>> + ((p)->parameter_table_pointer[0] << 0))
>>>>>> +
>>>>>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
>>>>> */
>>>>>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
>>>>>> +
>>>>>> +#define SFDP_SIGNATURE 0x50444653U
>>>>>> +#define SFDP_JESD216_MAJOR 1
>>>>>> +#define SFDP_JESD216_MINOR 0
>>>>>> +#define SFDP_JESD216A_MINOR 5
>>>>>> +#define SFDP_JESD216B_MINOR 6
>>>>>> +
>>>>>> +/* Basic Flash Parameter Table */
>>>>>> +
>>>>>> +/*
>>>>>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
>>>>>> + * They are indexed from 1 but C arrays are indexed from 0.
>>>>>> + */
>>>>>> +#define BFPT_DWORD(i) ((i) - 1)
>>>>>> +#define BFPT_DWORD_MAX 16
>>>>>> +
>>>>>> +/* The first version of JESB216 defined only 9 DWORDs. */
>>>>>> +#define BFPT_DWORD_MAX_JESD216 9
>>>>>> +
>>>>>> +/* 1st DWORD. */
>>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
>>>>> GENMASK(18, 17)
>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
>>>>>> +#define BFPT_DWORD1_DTR BIT(19)
>>>>>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
>>>>>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
>>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
>>>>>> +
>>>>>> +/* 5th DWORD. */
>>>>>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
>>>>>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
>>>>>> +
>>>>>> +/* 11th DWORD. */
>>>>>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
>>>>>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
>>>>> 4)
>>>>>> +
>>>>>> +/* 15th DWORD. */
>>>>>> +
>>>>>> +/*
>>>>>> + * (from JESD216 rev B)
>>>>>> + * Quad Enable Requirements (QER):
>>>>>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-
>> 4
>>>>>> + * reads based on instruction. DQ3/HOLD# functions are hold
>> during
>>>>>> + * instruction phase.
>>>>>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>> + * [...]
>>>>>> + * Writing only one byte to the status register has the side-effect
>> of
>>>>>> + * clearing status register 2, including the QE bit. The 100b code is
>>>>>> + * used if writing one byte to the status register does not modify
>>>>>> + * status register 2.
>>>>>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
>>>>>> + * one data byte where bit 6 is one.
>>>>>> + * [...]
>>>>>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
>>>>>> + * register 2 instruction 3Eh with one data byte where bit 7 is one.
>>>>>> + * [...]
>>>>>> + * The status register 2 is read using instruction 3Fh.
>>>>>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>> + * [...]
>>>>>> + * In contrast to the 001b code, writing one byte to the status
>>>>>> + * register does not modify status register 2.
>>>>>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
>>>>>> + * Read Status instruction 05h. Status register2 is read using
>>>>>> + * instruction 35h. QE is set via Writ Status instruction 01h with
>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>> + * [...]
>>>>>> + */
>>>>>> +#define BFPT_DWORD15_QER_MASK
>>>>> GENMASK(22, 20)
>>>>>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
>>>>> /* Micron */
>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
>>>>>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
>>>>> Macronix */
>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
>>>>> Spansion */
>>>>>> +
>>>>>> +struct sfdp_bfpt {
>>>>>> + u32 dwords[BFPT_DWORD_MAX];
>>>>>> +};
>>>>>> +
>>>>>> struct dm_spi_flash_ops {
>>>>>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
>>>>>> int (*write)(struct udevice *dev, u32 offset, size_t len,
>>>>>>
>>>
>>> --
>>> Regards
>>> Vignesh
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>> ts.denx.de%2Flistinfo%2Fu-
>> boot&data=02%7C01%7Crajat.srivastava%40n
>>>
>> xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
>> d99c5c
>>>
>> 301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
>> y5eCmDx
>>> 0KPmAHXvF8kusaJVUdM%3D&reserved=0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-11-16 15:25 ` Vignesh R
@ 2018-12-19 9:01 ` Rajat Srivastava
2018-12-19 13:04 ` Vignesh R
0 siblings, 1 reply; 13+ messages in thread
From: Rajat Srivastava @ 2018-12-19 9:01 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Vignesh R <vigneshr@ti.com>
> Sent: Friday, November 16, 2018 8:56 PM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Cc: Jagan Teki <jagan@openedev.com>
> Subject: Re: [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI
> framework
>
> On 16-Nov-18 6:29 PM, Rajat Srivastava wrote:
> >> On Fri, Nov 16, 2018 at 11:19 AM Vignesh R <vigneshr@ti.com> wrote:
> >>> On 16/11/18 3:10 PM, Rajat Srivastava wrote:
> >>>>> Hi Rajat,
> >>>>>
> >>>>> On 13/11/18 5:30 PM, Rajat Srivastava wrote:
> >>>>>> Add support for JESD216 rev B standard JEDEC Serial Flash
> >>>>>> Discoverable Parameters (SFDP) tables to dynamically initialize
> >>>>>> flash size, page size and address width of the flash. More
> >>>>>> parameters can be added as per requirement.
> >>>>>> SFDP parsing is made default but already existing method for
> >>>>>> parsing these parameters are not deprecated.
> >>>>>> A flag is created to skip SFDP parsing for a particular flash, if
> >>>>>> required.
> >>>>>>
> >>>>>> SFDP data lets us auto-detect the addressing mode supported by
> >>>>>> the flash which helps us access the flash using 4-byte address.
> >>>>>>
> >>>>>> Add a new argument in spi_flash_addr() function to create
> >>>>>> commands with 3-byte or 4-byte address depending on the SFDP
> data
> >>>>>> read. Add pointer to struct spi_flash in struct spi_slave so that
> >>>>>> driver can have access to SFDP data.
> >>>>>>
> >>>>>> Introduce new structures and functions to read and parse SFDP data.
> >>>>>> This is loosely based on Linux SFDP framework.
> >>>>>>
> >>>>>> Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
> >>>>>> ---
> >>>>>> Changes in v2:
> >>>>>> - Make SFDP parsing the default method.
> >>>>>> - Change SPI_FLASH_USE_SFDP flag to SPI_FLASH_USE_SFDP to
> >> provide
> >>>>>> an option to skip SFDP parsing for a particular flash.
> >>>>>> ---
> >>>>>
> >>>>> [...]
> >>>>>> +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> >>>>>> + const struct sfdp_parameter_header
> >>>>> *bfpt_header) {
> >>>>>> + struct sfdp_bfpt bfpt;
> >>>>>> + size_t len;
> >>>>>> + int i, err;
> >>>>>> + u32 addr;
> >>>>>> +
> >>>>>> + /* JESD216 Basic Flash Parameter Table length is at least 9
> DWORDs.
> >>>>> */
> >>>>>> + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> >>>>>> + return -EINVAL;
> >>>>>> +
> >>>>>> + /* Read the Basic Flash Parameter Table. */
> >>>>>> + len = min_t(size_t, sizeof(bfpt),
> >>>>>> + bfpt_header->length * sizeof(u32));
> >>>>>> + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> >>>>>> + memset(&bfpt, 0, sizeof(bfpt));
> >>>>>> + err = spi_flash_read_sfdp(flash, addr, len, &bfpt);
> >>>>>> + if (err < 0)
> >>>>>> + return err;
> >>>>>> +
> >>>>>> + /* Fix endianness of the BFPT DWORDs. */
> >>>>>> + for (i = 0; i < BFPT_DWORD_MAX; i++)
> >>>>>> + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> >>>>>> +
> >>>>>> + /* Number of address bytes. */
> >>>>>> + switch (bfpt.dwords[BFPT_DWORD(1)] &
> >>>>> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> >>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> >>>>>> + flash->addr_width = 3;
> >>>>>> + break;
> >>>>>> +
> >>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> >>>>>> + printf("SF: Flash defaults to 3-Byte mode; enters 4-Byte ");
> >>>>>> + printf("mode on command\n");
> >>>>>> + /*
> >>>>>> + * By default, 4-byte addressing mode is set.
> >>>>>> + * To enforce 3-byte addressing mode, set
> >>>>>> + addrwd_3_in_use
> >>>>> flag
> >>>>>> + * in struct spi_flash for every command.
> >>>>>> + */
> >>>>>> + flash->addr_width = 4;
> >>>>>> + break;
> >>>>>> +
> >>>>>> + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> >>>>>> + flash->addr_width = 4;
> >>>>>> + break;
> >>>>>> +
> >>>>>
> >>>>> I see your are updating flash->addr_width to 4 bytes here. But I
> >>>>> don't see any code in this patch that changes flash->read_cmd (and
> >>>>> write/erase
> >>>>> cmds) to use 4B addressing opcodes. Also, I dont see any code that
> >>>>> bypasses BAR register configuration when CONFIG_SPI_FLASH_BAR is
> >>>>> defined and SFDP is available.
> >>>>
> >>>> We don't have any flash that supports CONFIG_SPI_FLASH_BAR. What
> do
> >> you suggest, shall we skip SFDP parsing in case CONFIG_SPI_FLASH_BAR
> >> is defined?
> >>>>
> >>>
> >>> I suggest skipping BAR configuration completely if flash supports
> >>> address width of 4 as per SFDP. BAR configuration is only needed if
> >>> flash does not support 4 byte addressing and flash size is > 16MB
> >>>
> >>>>> This patch will most certainly break SPI controller drivers(like
> >>>>> cadence_qspi.c) that expect sf layer to provide correct read
> >>>>> opcode and address width information, since there will be mismatch
> >>>>> wrt opcode used and number of address byte sent.
> >>>>> Not sure how this patch was tested, but I guess fsl_qspi modifies
> >>>>> read opcode internal to the driver and gets away with it.
> >>>>> Please modify the patch to update read_cmd/erase_cmd/write_cmd
> to
> >>>>> use
> >>>>> 4 byte addressing opcodes
> >>>>
> >>>> If the flash supports only supports 4-byte opcodes then there will
> >>>> be no mismatch wrt opcode used and number of address bytes sent
> >>>> since all of the opcodes that the flash supports will be 4-bytes opcodes.
> >>>> But if the flash supports both 3-byte as well as 4-byte address
> >>>> widths (which is a separate case) then this code will default the
> >>>> address mode to 4-byte but gives an option to use 3-byte by setting
> >> addrwd_3_in_use flag.
> >>>>
> >>>
> >>> Lets assume flash supports both 3 and 4 byte addressing (AFAIK,
> >>> majority of flash parts do that). In this case, this patch is
> >>> defaulting to 4 byte address width but
> >>> flash->read_cmd/write_cmd/erase_cmd are still set to 3 byte
> >>> flash->addressing
> >> opcodes.
> >>> So, SPI controller would end up send 3 addressing byte opcode but 4
> >>> byte of address. The flash would interpret last byte of address as
> >>> first byte of data and thus lead to data corruption in case of write.
> >>> Isn't that a problem?
> >
> > Ok. But I'm curious if a flash supports both 3 and 4 byte address
> > widths (and it does not have BAR support), then why would one use 3-byte
> read/write/erase commands?
> > How would one access >16 MB in such case?
> >
>
> Unfortunately thats a problem with U-Boot spi_flash layer today. Many
> newer flashes like Micron's mt35x series dont support BAR registers.
> Accessing above 16MB is broken for such flashes today.
>
> > Also, initially I wanted to make SFDP optional so that none of the drivers
> are affected
> > and one could enable it using a flag as per requirement. Shall we get back
> to making
> > this optional (similar to v1 of this patch)?
> >
>
> How does that help? With opt-in approach you mark some flash devices as
> SFDP capable, but you still don't update opcodes, thus breaking boards
> that have such flash devices. So, porting full bfpt parsing and always
> using opcode to 4 byte addressing opcodes when available will be needed.
> Thus you would end up with SFDP as default instead of opt-in.
Hi Vignesh
Porting full bftp parsing requires time. I am seeing a conflict with a patch of yours
https://patchwork.ozlabs.org/cover/1012146/
It seem above RFC patch will bring SFDP to U-boot. Do you still want me
to put effort in porting SFDP in current version?
Regards
Rajat
>
> > Regards
> > Rajat
> >
> >>
> >> Additionally, there are flash chips with a 3-/4-byte mode change
> instruction.
> >> But using 4-byte opcodes should be preferred.
> >>
> >> Simon
> >>
> >>>
> >>> In particular you are not parsing entire bfpt like in [1] and will
> >>> break flash operations
> >>>
> >>>
> >>
> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> >>> elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
> >> nor
> >>> %2Fspi-
> >> nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
> >>>
> >>
> cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5
> >> %7C0
> >>>
> >>
> %7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
> >> M428u1bT1bU
> >>> rXuX2GsSjb0%3D&reserved=0
> >>>
> >>> Regards
> >>> Vignesh
> >>>
> >>>> Regards
> >>>> Rajat
> >>>>
> >>>>>
> >>>>> Regards
> >>>>> Vigensh
> >>>>>
> >>>>>
> >>>>>> + default:
> >>>>>> + break;
> >>>>>> + }
> >>>>>> +
> >>>>>> + /* Flash Memory Density (in bits). */
> >>>>>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> >>>>>> + if (flash->size & BIT(31)) {
> >>>>>> + flash->size &= ~BIT(31);
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
> >>>>>> + * bits is unlikely to exist so this error probably means
> >>>>>> + * the BFPT we are reading is corrupted/wrong.
> >>>>>> + */
> >>>>>> + if (flash->size > 63)
> >>>>>> + return -EINVAL;
> >>>>>> +
> >>>>>> + flash->size = 1ULL << flash->size;
> >>>>>> + } else {
> >>>>>> + flash->size++;
> >>>>>> + }
> >>>>>> + flash->size >>= 3; /* Convert to bytes. */
> >>>>>> +
> >>>>>> + /* Stop here if not JESD216 rev A or later. */
> >>>>>> + if (bfpt_header->length < BFPT_DWORD_MAX)
> >>>>>> + return 0;
> >>>>>> +
> >>>>>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> >>>>>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> >>>>>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> >>>>>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> >>>>>> + flash->page_size = 1U << flash->page_size;
> >>>>>> +
> >>>>>> + return 0;
> >>>>>> +}
> >>>>>> +
> >>>>>> +/*
> >>>>>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
> >>>>> Parameters.
> >>>>>> + * @flash: pointer to a 'struct spi_flash'
> >>>>>> + *
> >>>>>> + * The Serial Flash Discoverable Parameters are described by the
> >>>>>> +JEDEC JESD216
> >>>>>> + * specification. This is a standard which tends to supported by
> >>>>>> +almost all
> >>>>>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us
> >>>>>> +to learn at
> >>>>>> + * runtime the main parameters needed to perform basic SPI flash
> >>>>> operations.
> >>>>>> + *
> >>>>>> + * Return: 0 on success, -errno otherwise.
> >>>>>> + */
> >>>>>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
> >>>>>> + const struct sfdp_parameter_header *param_header,
> >>>>> *bfpt_header;
> >>>>>> + struct sfdp_parameter_header *param_headers = NULL;
> >>>>>> + struct sfdp_header header;
> >>>>>> + size_t psize;
> >>>>>> + int i, err;
> >>>>>> +
> >>>>>> + /* Get the SFDP header. */
> >>>>>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
> >>>>>> + if (err < 0)
> >>>>>> + return err;
> >>>>>> +
> >>>>>> + /* Check the SFDP header version. */
> >>>>>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> >>>>>> + header.major != SFDP_JESD216_MAJOR)
> >>>>>> + return -EINVAL;
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * Verify that the first and only mandatory parameter header is a
> >>>>>> + * Basic Flash Parameter Table header as specified in JESD216.
> >>>>>> + */
> >>>>>> + bfpt_header = &header.bfpt_header;
> >>>>>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> >>>>>> + bfpt_header->major != SFDP_JESD216_MAJOR)
> >>>>>> + return -EINVAL;
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * Allocate memory then read all parameter headers with a single
> >>>>>> + * Read SFDP command. These parameter headers will actually be
> >>>>> parsed
> >>>>>> + * twice: a first time to get the latest revision of the basic flash
> >>>>>> + * parameter table, then a second time to handle the supported
> >>>>> optional
> >>>>>> + * tables.
> >>>>>> + * Hence we read the parameter headers once for all to reduce
> the
> >>>>>> + * processing time
> >>>>>> + */
> >>>>>> + if (header.nph) {
> >>>>>> + psize = header.nph * sizeof(*param_headers);
> >>>>>> +
> >>>>>> + param_headers = malloc(psize);
> >>>>>> + if (!param_headers)
> >>>>>> + return -ENOMEM;
> >>>>>> +
> >>>>>> + err = spi_flash_read_sfdp(flash, sizeof(header),
> >>>>>> + psize, param_headers);
> >>>>>> + if (err < 0) {
> >>>>>> + dev_err(dev, "failed to read SFDP parameter
> >>>>> headers\n");
> >>>>>> + goto exit;
> >>>>>> + }
> >>>>>> + }
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * Check other parameter headers to get the latest revision of
> >>>>>> + * the basic flash parameter table.
> >>>>>> + */
> >>>>>> + for (i = 0; i < header.nph; i++) {
> >>>>>> + param_header = ¶m_headers[i];
> >>>>>> +
> >>>>>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
> >>>>> SFDP_BFPT_ID &&
> >>>>>> + param_header->major == SFDP_JESD216_MAJOR &&
> >>>>>> + (param_header->minor > bfpt_header->minor ||
> >>>>>> + (param_header->minor == bfpt_header->minor &&
> >>>>>> + param_header->length > bfpt_header->length)))
> >>>>>> + bfpt_header = param_header;
> >>>>>> + }
> >>>>>> +
> >>>>>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
> >>>>>> + if (err)
> >>>>>> + goto exit;
> >>>>>> +
> >>>>>> +exit:
> >>>>>> + free(param_headers);
> >>>>>> + return err;
> >>>>>> +}
> >>>>>> +
> >>>>>> static int set_quad_mode(struct spi_flash *flash,
> >>>>>> const struct spi_flash_info *info) { @@
> >>>>>> -1130,7
> >>>>> +1366,7 @@ int
> >>>>>> spi_flash_scan(struct spi_flash *flash) {
> >>>>>> struct spi_slave *spi = flash->spi;
> >>>>>> const struct spi_flash_info *info = NULL;
> >>>>>> - int ret;
> >>>>>> + int ret, sfdp = 0;
> >>>>>>
> >>>>>> info = spi_flash_read_id(flash);
> >>>>>> if (IS_ERR_OR_NULL(info))
> >>>>>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
> >>>>>> }
> >>>>>> #endif
> >>>>>>
> >>>>>> + spi->flash = flash;
> >>>>>> + flash->addrwd_3_in_use = false;
> >>>>>> +
> >>>>>> + /* Read Serial Flash Discoverable Parameters and initialize
> >>>>>> + * the following parameters of flash:
> >>>>>> + * 1. Flash size
> >>>>>> + * 2. Page size
> >>>>>> + * 3. Address width to be used for commands
> >>>>>> + */
> >>>>>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
> >>>>>> + flash->size = 0;
> >>>>>> + sfdp = spi_flash_parse_sfdp(flash);
> >>>>>> + if (sfdp < 0)
> >>>>>> + debug("Unable to get SFDP information\n");
> >>>>>> + }
> >>>>>> +
> >>>>>> /* Compute the flash size */
> >>>>>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
> >>>>> 0;
> >>>>>> - flash->page_size = info->page_size;
> >>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> >>>>>> + flash->page_size = info->page_size;
> >>>>>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> >>>>>> + }
> >>>>>> /*
> >>>>>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
> >>>>> pages,
> >>>>>> * yet use the 0x4d00 Extended JEDEC code. The rest of the
> >>>>>> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct
> spi_flash
> >> *flash)
> >>>>>> }
> >>>>>> flash->page_size <<= flash->shift;
> >>>>>> flash->sector_size = info->sector_size << flash->shift;
> >>>>>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
> >>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
> >>>>>> + flash->size = flash->sector_size * info->n_sectors <<
> >>>>>> + flash->shift;
> >>>>>> + }
> >>>>>> #ifdef CONFIG_SF_DUAL_FLASH
> >>>>>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
> >>>>>> flash->size <<= 1;
> >>>>>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
> >>>>>> #endif
> >>>>>>
> >>>>>> #ifndef CONFIG_SPI_FLASH_BAR
> >>>>>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
> >>>>>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
> >>>>>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
> >>>>>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
> >>>>>> + (flash->dual_flash == SF_SINGLE_FLASH &&
> >>>>>> + flash->size > SPI_FLASH_16MB_BOUN) ||
> >>>>>> + (flash->dual_flash > SF_SINGLE_FLASH &&
> >>>>>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
> >>>>>> puts("SF: Warning - Only lower 16MiB accessible,");
> >>>>>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
> >>>>>> diff -
> >>>>> -git
> >>>>>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581
> >>>>>> 100644
> >>>>>> --- a/include/spi.h
> >>>>>> +++ b/include/spi.h
> >>>>>> @@ -10,6 +10,7 @@
> >>>>>> #define _SPI_H_
> >>>>>>
> >>>>>> #include <common.h>
> >>>>>> +#include <spi_flash.h>
> >>>>>>
> >>>>>> /* SPI mode flags */
> >>>>>> #define SPI_CPHA BIT(0) /* clock phase */
> >>>>>> @@ -103,6 +104,7 @@ struct spi_slave {
> >>>>>> unsigned int bus;
> >>>>>> unsigned int cs;
> >>>>>> #endif
> >>>>>> + struct spi_flash *flash;
> >>>>>> uint mode;
> >>>>>> unsigned int wordlen;
> >>>>>> unsigned int max_read_size;
> >>>>>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
> >>>>>> 0ec98fb55d..6558a4a1d5 100644
> >>>>>> --- a/include/spi_flash.h
> >>>>>> +++ b/include/spi_flash.h
> >>>>>> @@ -47,6 +47,9 @@ struct spi_slave;
> >>>>>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
> >>>>> read.
> >>>>>> * @write_cmd: Write cmd - page and quad program.
> >>>>>> * @dummy_byte: Dummy cycles for read operation.
> >>>>>> + * @cmd_len: Total length of command.
> >>>>>> + * @addr_width: Number of address bytes.
> >>>>>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
> >>>>> mode.
> >>>>>> * @memory_map: Address of read-only SPI flash access
> >>>>>> * @flash_lock: lock a region of the SPI Flash
> >>>>>> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6
> >>>>>> +85,9 @@ struct spi_flash {
> >>>>>> u8 read_cmd;
> >>>>>> u8 write_cmd;
> >>>>>> u8 dummy_byte;
> >>>>>> + u8 cmd_len;
> >>>>>> + u8 addr_width;
> >>>>>> + bool addrwd_3_in_use;
> >>>>>>
> >>>>>> void *memory_map;
> >>>>>>
> >>>>>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
> >>>>>>
> >>>>>> +/*
> >>>>>> + * Serial Flash Discoverable Parameter Headers */ struct
> >>>>>> +sfdp_parameter_header {
> >>>>>> + u8 id_lsb;
> >>>>>> + u8 minor;
> >>>>>> + u8 major;
> >>>>>> + u8 length; /* in double words */
> >>>>>> + u8 parameter_table_pointer[3]; /* byte address */
> >>>>>> + u8 id_msb;
> >>>>>> +};
> >>>>>> +
> >>>>>> +struct sfdp_header {
> >>>>>> + u32 signature; /* Ox50444653U <=> "SFDP" */
> >>>>>> + u8 minor;
> >>>>>> + u8 major;
> >>>>>> + u8 nph; /* 0-base number of parameter headers */
> >>>>>> + u8 unused;
> >>>>>> +
> >>>>>> + /* Basic Flash Parameter Table. */
> >>>>>> + struct sfdp_parameter_header bfpt_header;
> >>>>>> +};
> >>>>>> +
> >>>>>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) |
> >>>>>> +(p)->id_lsb) #define SFDP_PARAM_HEADER_PTP(p) \
> >>>>>> + (((p)->parameter_table_pointer[2] << 16) | \
> >>>>>> + ((p)->parameter_table_pointer[1] << 8) | \
> >>>>>> + ((p)->parameter_table_pointer[0] << 0))
> >>>>>> +
> >>>>>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter
> Table
> >>>>> */
> >>>>>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> >>>>>> +
> >>>>>> +#define SFDP_SIGNATURE 0x50444653U
> >>>>>> +#define SFDP_JESD216_MAJOR 1
> >>>>>> +#define SFDP_JESD216_MINOR 0
> >>>>>> +#define SFDP_JESD216A_MINOR 5
> >>>>>> +#define SFDP_JESD216B_MINOR 6
> >>>>>> +
> >>>>>> +/* Basic Flash Parameter Table */
> >>>>>> +
> >>>>>> +/*
> >>>>>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16
> DWORDs.
> >>>>>> + * They are indexed from 1 but C arrays are indexed from 0.
> >>>>>> + */
> >>>>>> +#define BFPT_DWORD(i) ((i) - 1)
> >>>>>> +#define BFPT_DWORD_MAX 16
> >>>>>> +
> >>>>>> +/* The first version of JESB216 defined only 9 DWORDs. */
> >>>>>> +#define BFPT_DWORD_MAX_JESD216 9
> >>>>>> +
> >>>>>> +/* 1st DWORD. */
> >>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> >>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
> >>>>> GENMASK(18, 17)
> >>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> >>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> >>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> >>>>>> +#define BFPT_DWORD1_DTR BIT(19)
> >>>>>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> >>>>>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> >>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> >>>>>> +
> >>>>>> +/* 5th DWORD. */
> >>>>>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> >>>>>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> >>>>>> +
> >>>>>> +/* 11th DWORD. */
> >>>>>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> >>>>>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
> >>>>> 4)
> >>>>>> +
> >>>>>> +/* 15th DWORD. */
> >>>>>> +
> >>>>>> +/*
> >>>>>> + * (from JESD216 rev B)
> >>>>>> + * Quad Enable Requirements (QER):
> >>>>>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-
> 4-
> >> 4
> >>>>>> + * reads based on instruction. DQ3/HOLD# functions are hold
> >> during
> >>>>>> + * instruction phase.
> >>>>>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status
> with
> >>>>>> + * two data bytes where bit 1 of the second byte is one.
> >>>>>> + * [...]
> >>>>>> + * Writing only one byte to the status register has the side-
> effect
> >> of
> >>>>>> + * clearing status register 2, including the QE bit. The 100b code
> is
> >>>>>> + * used if writing one byte to the status register does not
> modify
> >>>>>> + * status register 2.
> >>>>>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status
> with
> >>>>>> + * one data byte where bit 6 is one.
> >>>>>> + * [...]
> >>>>>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> >>>>>> + * register 2 instruction 3Eh with one data byte where bit 7 is
> one.
> >>>>>> + * [...]
> >>>>>> + * The status register 2 is read using instruction 3Fh.
> >>>>>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status
> with
> >>>>>> + * two data bytes where bit 1 of the second byte is one.
> >>>>>> + * [...]
> >>>>>> + * In contrast to the 001b code, writing one byte to the status
> >>>>>> + * register does not modify status register 2.
> >>>>>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read
> using
> >>>>>> + * Read Status instruction 05h. Status register2 is read using
> >>>>>> + * instruction 35h. QE is set via Writ Status instruction 01h with
> >>>>>> + * two data bytes where bit 1 of the second byte is one.
> >>>>>> + * [...]
> >>>>>> + */
> >>>>>> +#define BFPT_DWORD15_QER_MASK
> >>>>> GENMASK(22, 20)
> >>>>>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
> >>>>> /* Micron */
> >>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL <<
> 20)
> >>>>>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> >>>>> Macronix */
> >>>>>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> >>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL <<
> 20)
> >>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> >>>>> Spansion */
> >>>>>> +
> >>>>>> +struct sfdp_bfpt {
> >>>>>> + u32 dwords[BFPT_DWORD_MAX];
> >>>>>> +};
> >>>>>> +
> >>>>>> struct dm_spi_flash_ops {
> >>>>>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
> >>>>>> int (*write)(struct udevice *dev, u32 offset, size_t len,
> >>>>>>
> >>>
> >>> --
> >>> Regards
> >>> Vignesh
> >>> _______________________________________________
> >>> U-Boot mailing list
> >>> U-Boot at lists.denx.de
> >>>
> >>
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> >>> ts.denx.de%2Flistinfo%2Fu-
> >> boot&data=02%7C01%7Crajat.srivastava%40n
> >>>
> >>
> xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
> >> d99c5c
> >>>
> >>
> 301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
> >> y5eCmDx
> >>> 0KPmAHXvF8kusaJVUdM%3D&reserved=0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework
2018-12-19 9:01 ` Rajat Srivastava
@ 2018-12-19 13:04 ` Vignesh R
0 siblings, 0 replies; 13+ messages in thread
From: Vignesh R @ 2018-12-19 13:04 UTC (permalink / raw)
To: u-boot
Hi,
>>
>> Unfortunately thats a problem with U-Boot spi_flash layer today. Many
>> newer flashes like Micron's mt35x series dont support BAR registers.
>> Accessing above 16MB is broken for such flashes today.
>>
>>> Also, initially I wanted to make SFDP optional so that none of the drivers
>> are affected
>>> and one could enable it using a flag as per requirement. Shall we get back
>> to making
>>> this optional (similar to v1 of this patch)?
>>>
>>
>> How does that help? With opt-in approach you mark some flash devices as
>> SFDP capable, but you still don't update opcodes, thus breaking boards
>> that have such flash devices. So, porting full bfpt parsing and always
>> using opcode to 4 byte addressing opcodes when available will be needed.
>> Thus you would end up with SFDP as default instead of opt-in.
>
> Hi Vignesh
>
> Porting full bftp parsing requires time. I am seeing a conflict with a patch of yours
> https://patchwork.ozlabs.org/cover/1012146/
>
Yes, I ended up porting all of Linux SPI NOR features including SFDP and
4 byte addressing.
> It seem above RFC patch will bring SFDP to U-boot. Do you still want me
> to put effort in porting SFDP in current version?
>
No need to post this series as my patches including this change. I would
appreciate any review/test of my series.
You can find Patch v1 here: https://patchwork.ozlabs.org/cover/1012146/
(Please drop patch 9/16 while testing). Thanks!
Regards
Vignesh
> Regards
> Rajat
>
>>
>>> Regards
>>> Rajat
>>>
>>>>
>>>> Additionally, there are flash chips with a 3-/4-byte mode change
>> instruction.
>>>> But using 4-byte opcodes should be preferred.
>>>>
>>>> Simon
>>>>
>>>>>
>>>>> In particular you are not parsing entire bfpt like in [1] and will
>>>>> break flash operations
>>>>>
>>>>>
>>>>
>> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>>>>> elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
>>>> nor
>>>>> %2Fspi-
>>>> nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
>>>>>
>>>>
>> cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c30163
>> 5
>>>> %7C0
>>>>>
>>>>
>> %7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
>>>> M428u1bT1bU
>>>>> rXuX2GsSjb0%3D&reserved=0
>>>>>
>>>>> Regards
>>>>> Vignesh
>>>>>
>>>>>> Regards
>>>>>> Rajat
>>>>>>
>>>>>>>
>>>>>>> Regards
>>>>>>> Vigensh
>>>>>>>
>>>>>>>
>>>>>>>> + default:
>>>>>>>> + break;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + /* Flash Memory Density (in bits). */
>>>>>>>> + flash->size = bfpt.dwords[BFPT_DWORD(2)];
>>>>>>>> + if (flash->size & BIT(31)) {
>>>>>>>> + flash->size &= ~BIT(31);
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Prevent overflows on flash->size. Anyway, a NOR of 2^64
>>>>>>>> + * bits is unlikely to exist so this error probably means
>>>>>>>> + * the BFPT we are reading is corrupted/wrong.
>>>>>>>> + */
>>>>>>>> + if (flash->size > 63)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + flash->size = 1ULL << flash->size;
>>>>>>>> + } else {
>>>>>>>> + flash->size++;
>>>>>>>> + }
>>>>>>>> + flash->size >>= 3; /* Convert to bytes. */
>>>>>>>> +
>>>>>>>> + /* Stop here if not JESD216 rev A or later. */
>>>>>>>> + if (bfpt_header->length < BFPT_DWORD_MAX)
>>>>>>>> + return 0;
>>>>>>>> +
>>>>>>>> + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
>>>>>>>> + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
>>>>>>>> + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
>>>>>>>> + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
>>>>>>>> + flash->page_size = 1U << flash->page_size;
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable
>>>>>>> Parameters.
>>>>>>>> + * @flash: pointer to a 'struct spi_flash'
>>>>>>>> + *
>>>>>>>> + * The Serial Flash Discoverable Parameters are described by the
>>>>>>>> +JEDEC JESD216
>>>>>>>> + * specification. This is a standard which tends to supported by
>>>>>>>> +almost all
>>>>>>>> + * (Q)SPI memory manufacturers. Those hard-coded tables allow us
>>>>>>>> +to learn at
>>>>>>>> + * runtime the main parameters needed to perform basic SPI flash
>>>>>>> operations.
>>>>>>>> + *
>>>>>>>> + * Return: 0 on success, -errno otherwise.
>>>>>>>> + */
>>>>>>>> +static int spi_flash_parse_sfdp(struct spi_flash *flash) {
>>>>>>>> + const struct sfdp_parameter_header *param_header,
>>>>>>> *bfpt_header;
>>>>>>>> + struct sfdp_parameter_header *param_headers = NULL;
>>>>>>>> + struct sfdp_header header;
>>>>>>>> + size_t psize;
>>>>>>>> + int i, err;
>>>>>>>> +
>>>>>>>> + /* Get the SFDP header. */
>>>>>>>> + err = spi_flash_read_sfdp(flash, 0, sizeof(header), &header);
>>>>>>>> + if (err < 0)
>>>>>>>> + return err;
>>>>>>>> +
>>>>>>>> + /* Check the SFDP header version. */
>>>>>>>> + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
>>>>>>>> + header.major != SFDP_JESD216_MAJOR)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Verify that the first and only mandatory parameter header is a
>>>>>>>> + * Basic Flash Parameter Table header as specified in JESD216.
>>>>>>>> + */
>>>>>>>> + bfpt_header = &header.bfpt_header;
>>>>>>>> + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
>>>>>>>> + bfpt_header->major != SFDP_JESD216_MAJOR)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Allocate memory then read all parameter headers with a single
>>>>>>>> + * Read SFDP command. These parameter headers will actually be
>>>>>>> parsed
>>>>>>>> + * twice: a first time to get the latest revision of the basic flash
>>>>>>>> + * parameter table, then a second time to handle the supported
>>>>>>> optional
>>>>>>>> + * tables.
>>>>>>>> + * Hence we read the parameter headers once for all to reduce
>> the
>>>>>>>> + * processing time
>>>>>>>> + */
>>>>>>>> + if (header.nph) {
>>>>>>>> + psize = header.nph * sizeof(*param_headers);
>>>>>>>> +
>>>>>>>> + param_headers = malloc(psize);
>>>>>>>> + if (!param_headers)
>>>>>>>> + return -ENOMEM;
>>>>>>>> +
>>>>>>>> + err = spi_flash_read_sfdp(flash, sizeof(header),
>>>>>>>> + psize, param_headers);
>>>>>>>> + if (err < 0) {
>>>>>>>> + dev_err(dev, "failed to read SFDP parameter
>>>>>>> headers\n");
>>>>>>>> + goto exit;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + /*
>>>>>>>> + * Check other parameter headers to get the latest revision of
>>>>>>>> + * the basic flash parameter table.
>>>>>>>> + */
>>>>>>>> + for (i = 0; i < header.nph; i++) {
>>>>>>>> + param_header = ¶m_headers[i];
>>>>>>>> +
>>>>>>>> + if (SFDP_PARAM_HEADER_ID(param_header) ==
>>>>>>> SFDP_BFPT_ID &&
>>>>>>>> + param_header->major == SFDP_JESD216_MAJOR &&
>>>>>>>> + (param_header->minor > bfpt_header->minor ||
>>>>>>>> + (param_header->minor == bfpt_header->minor &&
>>>>>>>> + param_header->length > bfpt_header->length)))
>>>>>>>> + bfpt_header = param_header;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + err = spi_flash_parse_bfpt(flash, bfpt_header);
>>>>>>>> + if (err)
>>>>>>>> + goto exit;
>>>>>>>> +
>>>>>>>> +exit:
>>>>>>>> + free(param_headers);
>>>>>>>> + return err;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> static int set_quad_mode(struct spi_flash *flash,
>>>>>>>> const struct spi_flash_info *info) { @@
>>>>>>>> -1130,7
>>>>>>> +1366,7 @@ int
>>>>>>>> spi_flash_scan(struct spi_flash *flash) {
>>>>>>>> struct spi_slave *spi = flash->spi;
>>>>>>>> const struct spi_flash_info *info = NULL;
>>>>>>>> - int ret;
>>>>>>>> + int ret, sfdp = 0;
>>>>>>>>
>>>>>>>> info = spi_flash_read_id(flash);
>>>>>>>> if (IS_ERR_OR_NULL(info))
>>>>>>>> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>> }
>>>>>>>> #endif
>>>>>>>>
>>>>>>>> + spi->flash = flash;
>>>>>>>> + flash->addrwd_3_in_use = false;
>>>>>>>> +
>>>>>>>> + /* Read Serial Flash Discoverable Parameters and initialize
>>>>>>>> + * the following parameters of flash:
>>>>>>>> + * 1. Flash size
>>>>>>>> + * 2. Page size
>>>>>>>> + * 3. Address width to be used for commands
>>>>>>>> + */
>>>>>>>> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) {
>>>>>>>> + flash->size = 0;
>>>>>>>> + sfdp = spi_flash_parse_sfdp(flash);
>>>>>>>> + if (sfdp < 0)
>>>>>>>> + debug("Unable to get SFDP information\n");
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> /* Compute the flash size */
>>>>>>>> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 :
>>>>>>> 0;
>>>>>>>> - flash->page_size = info->page_size;
>>>>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>>>>>>> + flash->page_size = info->page_size;
>>>>>>>> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
>>>>>>>> + }
>>>>>>>> /*
>>>>>>>> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b
>>>>>>> pages,
>>>>>>>> * yet use the 0x4d00 Extended JEDEC code. The rest of the
>>>>>>>> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct
>> spi_flash
>>>> *flash)
>>>>>>>> }
>>>>>>>> flash->page_size <<= flash->shift;
>>>>>>>> flash->sector_size = info->sector_size << flash->shift;
>>>>>>>> - flash->size = flash->sector_size * info->n_sectors << flash->shift;
>>>>>>>> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) {
>>>>>>>> + flash->size = flash->sector_size * info->n_sectors <<
>>>>>>>> + flash->shift;
>>>>>>>> + }
>>>>>>>> #ifdef CONFIG_SF_DUAL_FLASH
>>>>>>>> if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
>>>>>>>> flash->size <<= 1;
>>>>>>>> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash)
>>>>>>>> #endif
>>>>>>>>
>>>>>>>> #ifndef CONFIG_SPI_FLASH_BAR
>>>>>>>> - if (((flash->dual_flash == SF_SINGLE_FLASH) &&
>>>>>>>> - (flash->size > SPI_FLASH_16MB_BOUN)) ||
>>>>>>>> - ((flash->dual_flash > SF_SINGLE_FLASH) &&
>>>>>>>> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) &&
>>>>>>>> + (flash->dual_flash == SF_SINGLE_FLASH &&
>>>>>>>> + flash->size > SPI_FLASH_16MB_BOUN) ||
>>>>>>>> + (flash->dual_flash > SF_SINGLE_FLASH &&
>>>>>>>> (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
>>>>>>>> puts("SF: Warning - Only lower 16MiB accessible,");
>>>>>>>> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
>>>>>>>> diff -
>>>>>>> -git
>>>>>>>> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581
>>>>>>>> 100644
>>>>>>>> --- a/include/spi.h
>>>>>>>> +++ b/include/spi.h
>>>>>>>> @@ -10,6 +10,7 @@
>>>>>>>> #define _SPI_H_
>>>>>>>>
>>>>>>>> #include <common.h>
>>>>>>>> +#include <spi_flash.h>
>>>>>>>>
>>>>>>>> /* SPI mode flags */
>>>>>>>> #define SPI_CPHA BIT(0) /* clock phase */
>>>>>>>> @@ -103,6 +104,7 @@ struct spi_slave {
>>>>>>>> unsigned int bus;
>>>>>>>> unsigned int cs;
>>>>>>>> #endif
>>>>>>>> + struct spi_flash *flash;
>>>>>>>> uint mode;
>>>>>>>> unsigned int wordlen;
>>>>>>>> unsigned int max_read_size;
>>>>>>>> diff --git a/include/spi_flash.h b/include/spi_flash.h index
>>>>>>>> 0ec98fb55d..6558a4a1d5 100644
>>>>>>>> --- a/include/spi_flash.h
>>>>>>>> +++ b/include/spi_flash.h
>>>>>>>> @@ -47,6 +47,9 @@ struct spi_slave;
>>>>>>>> * @read_cmd: Read cmd - Array Fast, Extn read and quad
>>>>>>> read.
>>>>>>>> * @write_cmd: Write cmd - page and quad program.
>>>>>>>> * @dummy_byte: Dummy cycles for read operation.
>>>>>>>> + * @cmd_len: Total length of command.
>>>>>>>> + * @addr_width: Number of address bytes.
>>>>>>>> + * @addrwd_3_in_use: Flag to send command in 3-byte address
>>>>>>> mode.
>>>>>>>> * @memory_map: Address of read-only SPI flash access
>>>>>>>> * @flash_lock: lock a region of the SPI Flash
>>>>>>>> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6
>>>>>>>> +85,9 @@ struct spi_flash {
>>>>>>>> u8 read_cmd;
>>>>>>>> u8 write_cmd;
>>>>>>>> u8 dummy_byte;
>>>>>>>> + u8 cmd_len;
>>>>>>>> + u8 addr_width;
>>>>>>>> + bool addrwd_3_in_use;
>>>>>>>>
>>>>>>>> void *memory_map;
>>>>>>>>
>>>>>>>> @@ -107,6 +113,120 @@ struct spi_flash { #endif };
>>>>>>>>
>>>>>>>> +/*
>>>>>>>> + * Serial Flash Discoverable Parameter Headers */ struct
>>>>>>>> +sfdp_parameter_header {
>>>>>>>> + u8 id_lsb;
>>>>>>>> + u8 minor;
>>>>>>>> + u8 major;
>>>>>>>> + u8 length; /* in double words */
>>>>>>>> + u8 parameter_table_pointer[3]; /* byte address */
>>>>>>>> + u8 id_msb;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +struct sfdp_header {
>>>>>>>> + u32 signature; /* Ox50444653U <=> "SFDP" */
>>>>>>>> + u8 minor;
>>>>>>>> + u8 major;
>>>>>>>> + u8 nph; /* 0-base number of parameter headers */
>>>>>>>> + u8 unused;
>>>>>>>> +
>>>>>>>> + /* Basic Flash Parameter Table. */
>>>>>>>> + struct sfdp_parameter_header bfpt_header;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) |
>>>>>>>> +(p)->id_lsb) #define SFDP_PARAM_HEADER_PTP(p) \
>>>>>>>> + (((p)->parameter_table_pointer[2] << 16) | \
>>>>>>>> + ((p)->parameter_table_pointer[1] << 8) | \
>>>>>>>> + ((p)->parameter_table_pointer[0] << 0))
>>>>>>>> +
>>>>>>>> +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter
>> Table
>>>>>>> */
>>>>>>>> +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
>>>>>>>> +
>>>>>>>> +#define SFDP_SIGNATURE 0x50444653U
>>>>>>>> +#define SFDP_JESD216_MAJOR 1
>>>>>>>> +#define SFDP_JESD216_MINOR 0
>>>>>>>> +#define SFDP_JESD216A_MINOR 5
>>>>>>>> +#define SFDP_JESD216B_MINOR 6
>>>>>>>> +
>>>>>>>> +/* Basic Flash Parameter Table */
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> + * JESD216 rev B defines a Basic Flash Parameter Table of 16
>> DWORDs.
>>>>>>>> + * They are indexed from 1 but C arrays are indexed from 0.
>>>>>>>> + */
>>>>>>>> +#define BFPT_DWORD(i) ((i) - 1)
>>>>>>>> +#define BFPT_DWORD_MAX 16
>>>>>>>> +
>>>>>>>> +/* The first version of JESB216 defined only 9 DWORDs. */
>>>>>>>> +#define BFPT_DWORD_MAX_JESD216 9
>>>>>>>> +
>>>>>>>> +/* 1st DWORD. */
>>>>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
>>>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_MASK
>>>>>>> GENMASK(18, 17)
>>>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
>>>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
>>>>>>>> +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
>>>>>>>> +#define BFPT_DWORD1_DTR BIT(19)
>>>>>>>> +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
>>>>>>>> +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
>>>>>>>> +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
>>>>>>>> +
>>>>>>>> +/* 5th DWORD. */
>>>>>>>> +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
>>>>>>>> +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
>>>>>>>> +
>>>>>>>> +/* 11th DWORD. */
>>>>>>>> +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
>>>>>>>> +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7,
>>>>>>> 4)
>>>>>>>> +
>>>>>>>> +/* 15th DWORD. */
>>>>>>>> +
>>>>>>>> +/*
>>>>>>>> + * (from JESD216 rev B)
>>>>>>>> + * Quad Enable Requirements (QER):
>>>>>>>> + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-
>> 4-
>>>> 4
>>>>>>>> + * reads based on instruction. DQ3/HOLD# functions are hold
>>>> during
>>>>>>>> + * instruction phase.
>>>>>>>> + * - 001b: QE is bit 1 of status register 2. It is set via Write Status
>> with
>>>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>>>> + * [...]
>>>>>>>> + * Writing only one byte to the status register has the side-
>> effect
>>>> of
>>>>>>>> + * clearing status register 2, including the QE bit. The 100b code
>> is
>>>>>>>> + * used if writing one byte to the status register does not
>> modify
>>>>>>>> + * status register 2.
>>>>>>>> + * - 010b: QE is bit 6 of status register 1. It is set via Write Status
>> with
>>>>>>>> + * one data byte where bit 6 is one.
>>>>>>>> + * [...]
>>>>>>>> + * - 011b: QE is bit 7 of status register 2. It is set via Write status
>>>>>>>> + * register 2 instruction 3Eh with one data byte where bit 7 is
>> one.
>>>>>>>> + * [...]
>>>>>>>> + * The status register 2 is read using instruction 3Fh.
>>>>>>>> + * - 100b: QE is bit 1 of status register 2. It is set via Write Status
>> with
>>>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>>>> + * [...]
>>>>>>>> + * In contrast to the 001b code, writing one byte to the status
>>>>>>>> + * register does not modify status register 2.
>>>>>>>> + * - 101b: QE is bit 1 of status register 2. Status register 1 is read
>> using
>>>>>>>> + * Read Status instruction 05h. Status register2 is read using
>>>>>>>> + * instruction 35h. QE is set via Writ Status instruction 01h with
>>>>>>>> + * two data bytes where bit 1 of the second byte is one.
>>>>>>>> + * [...]
>>>>>>>> + */
>>>>>>>> +#define BFPT_DWORD15_QER_MASK
>>>>>>> GENMASK(22, 20)
>>>>>>>> +#define BFPT_DWORD15_QER_NONE (0x0UL << 20)
>>>>>>> /* Micron */
>>>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL <<
>> 20)
>>>>>>>> +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
>>>>>>> Macronix */
>>>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
>>>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL <<
>> 20)
>>>>>>>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
>>>>>>> Spansion */
>>>>>>>> +
>>>>>>>> +struct sfdp_bfpt {
>>>>>>>> + u32 dwords[BFPT_DWORD_MAX];
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> struct dm_spi_flash_ops {
>>>>>>>> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf);
>>>>>>>> int (*write)(struct udevice *dev, u32 offset, size_t len,
>>>>>>>>
>>>>>
>>>>> --
>>>>> Regards
>>>>> Vignesh
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>>
>>>>
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
>>>>> ts.denx.de%2Flistinfo%2Fu-
>>>> boot&data=02%7C01%7Crajat.srivastava%40n
>>>>>
>>>>
>> xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
>>>> d99c5c
>>>>>
>>>>
>> 301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
>>>> y5eCmDx
>>>>> 0KPmAHXvF8kusaJVUdM%3D&reserved=0
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-12-19 13:04 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 12:00 [U-Boot] [PATCH v2 0/2] Read SFDP parameters and access flash above 16MB Rajat Srivastava
2018-11-13 12:00 ` [U-Boot] [PATCH v2 1/2] mtd/spi: Add JEDEC SFDP support in SPI framework Rajat Srivastava
2018-11-15 9:22 ` Vignesh R
2018-11-16 9:40 ` Rajat Srivastava
2018-11-16 10:20 ` Vignesh R
2018-11-16 10:23 ` Simon Goldschmidt
2018-11-16 12:59 ` Rajat Srivastava
2018-11-16 13:09 ` Simon Goldschmidt
2018-11-16 15:25 ` Vignesh R
2018-12-19 9:01 ` Rajat Srivastava
2018-12-19 13:04 ` Vignesh R
2018-11-16 10:43 ` Jagan Teki
2018-11-13 12:00 ` [U-Boot] [PATCH v2 2/2] fsl_qspi: Access flash above 16MB using SFDP Rajat Srivastava
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.