[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Rajat Srivastava]

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.

Enable reading and parsing of SFDP parameters for Spansion's
s25fs512 flash to auto-detect its size, its page size and the
addressing mode it supports.

Rajat Srivastava (3):
  mtd/spi: Add JEDEC SFDP support in SPI framework
  fsl_qspi: Access flash above 16MB using SFDP
  spi_flash_ids: Enable SFDP parsing for s25fs512 flash

 drivers/mtd/spi/sf_internal.h   |   4 +
 drivers/mtd/spi/spi_flash.c     | 297 +++++++++++++++++++++++++++++++++++++---
 drivers/mtd/spi/spi_flash_ids.c |   3 +-
 drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
 include/spi.h                   |   2 +
 include/spi_flash.h             | 120 ++++++++++++++++
 6 files changed, 501 insertions(+), 28 deletions(-)

-- 
2.14.1

[U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI framework

[Rajat Srivastava]

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.
Already existing method for parsing these parameters
are not deprecated, which can be done once most flash
start using SFDP data.

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>
---
 drivers/mtd/spi/sf_internal.h |   4 +
 drivers/mtd/spi/spi_flash.c   | 297 +++++++++++++++++++++++++++++++++++++++---
 include/spi.h                 |   2 +
 include/spi_flash.h           | 120 +++++++++++++++++
 4 files changed, 403 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 26f5c7c995..1bb4431d84 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_USE_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..5d2728cc00 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 = &param_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)
 {
@@ -1196,9 +1432,26 @@ 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_USE_SFDP) {
+		flash->size = 0;
+		spi_flash_parse_sfdp(flash);
+	}
+
 	/* 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_USE_SFDP)) {
+		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 +1466,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_USE_SFDP)) {
+		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 +1568,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_USE_SFDP) &&
+	    (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

[U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI framework

[Vignesh R]

On 17/10/18 4:58 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.
> Already existing method for parsing these parameters
> are not deprecated, which can be done once most flash
> start using SFDP data.
> 
> 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>
> ---
>  drivers/mtd/spi/sf_internal.h |   4 +
>  drivers/mtd/spi/spi_flash.c   | 297 +++++++++++++++++++++++++++++++++++++++---
>  include/spi.h                 |   2 +
>  include/spi_flash.h           | 120 +++++++++++++++++
>  4 files changed, 403 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 26f5c7c995..1bb4431d84 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_USE_SFDP	BIT(8)  /* parse SFDP to get flash info */

I suggest to SFDP not be a opt-in but default option like Linux kernel,
since that's SFDP is a standard. If flash does not support SFDP that can
be detected by looking for SFPD signature and code can fallback
gracefully. If a flash has a broken SFDP table then such parts can use
"SPI_FLASH_BROKEN_SFDP" flag or something. That way we don't need to add
above flag to almost all flash entries here.

Regards
Vignesh

>  #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..5d2728cc00 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 = &param_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)
>  {
> @@ -1196,9 +1432,26 @@ 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_USE_SFDP) {
> +		flash->size = 0;
> +		spi_flash_parse_sfdp(flash);
> +	}
> +
>  	/* 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_USE_SFDP)) {
> +		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 +1466,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_USE_SFDP)) {
> +		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 +1568,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_USE_SFDP) &&
> +	    (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] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI framework

[Rajat Srivastava]

> -----Original Message-----
> From: Vignesh R <vigneshr@ti.com>
> Sent: Wednesday, October 31, 2018 1:47 PM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> Cc: jagan at openedev.com
> Subject: Re: [U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI
> framework
> 
> 
> 
> On 17/10/18 4:58 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.
> > Already existing method for parsing these parameters are not
> > deprecated, which can be done once most flash start using SFDP data.
> >
> > 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>
> > ---
> >  drivers/mtd/spi/sf_internal.h |   4 +
> >  drivers/mtd/spi/spi_flash.c   | 297
> +++++++++++++++++++++++++++++++++++++++---
> >  include/spi.h                 |   2 +
> >  include/spi_flash.h           | 120 +++++++++++++++++
> >  4 files changed, 403 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/sf_internal.h
> > b/drivers/mtd/spi/sf_internal.h index 26f5c7c995..1bb4431d84 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_USE_SFDP	BIT(8)  /* parse SFDP to get flash info
> */
> 
> I suggest to SFDP not be a opt-in but default option like Linux kernel, since
> that's SFDP is a standard. If flash does not support SFDP that can be detected
> by looking for SFPD signature and code can fallback gracefully. If a flash has a
> broken SFDP table then such parts can use "SPI_FLASH_BROKEN_SFDP" flag
> or something. That way we don't need to add above flag to almost all flash
> entries here.

You are correct. But, currently no drivers have implementation to send READ SFDP
command from their respective drivers. Once most have migrated to SFDP standard
then we can change this flag (making it the default mode).

Is it possible that a flash that supports SFDP parameter reading can send some incorrect
signature? In that case I'll have to add an alternate path for fail scenario.

> 
> Regards
> Vignesh
> 
> >  #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..5d2728cc00 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 = &param_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)  { @@ -1196,9
> +1432,26 @@ 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_USE_SFDP) {
> > +		flash->size = 0;
> > +		spi_flash_parse_sfdp(flash);
> > +	}
> > +
> >  	/* 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_USE_SFDP)) {
> > +		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 +1466,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_USE_SFDP)) {
> > +		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 +1568,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_USE_SFDP) &&
> > +	    (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] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI framework

[Simon Goldschmidt]

On Wed, Oct 31, 2018 at 10:01 AM Rajat Srivastava
<rajat.srivastava@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Vignesh R <vigneshr@ti.com>
> > Sent: Wednesday, October 31, 2018 1:47 PM
> > To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> > Cc: jagan at openedev.com
> > Subject: Re: [U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI
> > framework
> >
> >
> >
> > On 17/10/18 4:58 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.
> > > Already existing method for parsing these parameters are not
> > > deprecated, which can be done once most flash start using SFDP data.
> > >
> > > 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>
> > > ---
> > >  drivers/mtd/spi/sf_internal.h |   4 +
> > >  drivers/mtd/spi/spi_flash.c   | 297
> > +++++++++++++++++++++++++++++++++++++++---
> > >  include/spi.h                 |   2 +
> > >  include/spi_flash.h           | 120 +++++++++++++++++
> > >  4 files changed, 403 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/mtd/spi/sf_internal.h
> > > b/drivers/mtd/spi/sf_internal.h index 26f5c7c995..1bb4431d84 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_USE_SFDP BIT(8)  /* parse SFDP to get flash info
> > */
> >
> > I suggest to SFDP not be a opt-in but default option like Linux kernel, since
> > that's SFDP is a standard. If flash does not support SFDP that can be detected
> > by looking for SFPD signature and code can fallback gracefully. If a flash has a
> > broken SFDP table then such parts can use "SPI_FLASH_BROKEN_SFDP" flag
> > or something. That way we don't need to add above flag to almost all flash
> > entries here.
>
> You are correct. But, currently no drivers have implementation to send READ SFDP
> command from their respective drivers. Once most have migrated to SFDP standard
> then we can change this flag (making it the default mode).

But can't we start now with making it default? What happens if the
driver doesn't support it? Can't we implement a fallback mode now so
we can make it default? That would encourage adoption, wouldn't it?

Simon

> Is it possible that a flash that supports SFDP parameter reading can send some incorrect
> signature? In that case I'll have to add an alternate path for fail scenario.

You're checking the signature already. But you're not checking the
return value of 'spi_flash_parse_sfdp'. See below.

>
> >
> > Regards
> > Vignesh
> >
> > >  #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..5d2728cc00 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 = &param_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)  { @@ -1196,9
> > +1432,26 @@ 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_USE_SFDP) {
> > > +           flash->size = 0;
> > > +           spi_flash_parse_sfdp(flash);

Check return value here and ensure the next block checking
'info->flags & SPI_FLASH_USE_SFDP' is executed?

Simon

> > > +   }
> > > +
> > >     /* 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_USE_SFDP)) {
> > > +           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 +1466,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_USE_SFDP)) {
> > > +           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 +1568,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_USE_SFDP) &&
> > > +       (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

[U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI framework

[Rajat Srivastava]

> -----Original Message-----
> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Sent: Wednesday, October 31, 2018 2:51 PM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>
> Cc: vigneshr at ti.com; U-Boot Mailing List <u-boot@lists.denx.de>; Jagan Teki
> <jagan@openedev.com>
> Subject: Re: [U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in SPI
> framework
> 
> On Wed, Oct 31, 2018 at 10:01 AM Rajat Srivastava
> <rajat.srivastava@nxp.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Vignesh R <vigneshr@ti.com>
> > > Sent: Wednesday, October 31, 2018 1:47 PM
> > > To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> > > Cc: jagan at openedev.com
> > > Subject: Re: [U-Boot] [PATCH 1/3] mtd/spi: Add JEDEC SFDP support in
> SPI
> > > framework
> > >
> > >
> > >
> > > On 17/10/18 4:58 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.
> > > > Already existing method for parsing these parameters are not
> > > > deprecated, which can be done once most flash start using SFDP data.
> > > >
> > > > 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>
> > > > ---
> > > >  drivers/mtd/spi/sf_internal.h |   4 +
> > > >  drivers/mtd/spi/spi_flash.c   | 297
> > > +++++++++++++++++++++++++++++++++++++++---
> > > >  include/spi.h                 |   2 +
> > > >  include/spi_flash.h           | 120 +++++++++++++++++
> > > >  4 files changed, 403 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/mtd/spi/sf_internal.h
> > > > b/drivers/mtd/spi/sf_internal.h index 26f5c7c995..1bb4431d84 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_USE_SFDP BIT(8)  /* parse SFDP to get flash info
> > > */
> > >
> > > I suggest to SFDP not be a opt-in but default option like Linux kernel,
> since
> > > that's SFDP is a standard. If flash does not support SFDP that can be
> detected
> > > by looking for SFPD signature and code can fallback gracefully. If a flash
> has a
> > > broken SFDP table then such parts can use "SPI_FLASH_BROKEN_SFDP"
> flag
> > > or something. That way we don't need to add above flag to almost all
> flash
> > > entries here.
> >
> > You are correct. But, currently no drivers have implementation to send
> READ SFDP
> > command from their respective drivers. Once most have migrated to SFDP
> standard
> > then we can change this flag (making it the default mode).
> 
> But can't we start now with making it default? What happens if the
> driver doesn't support it? Can't we implement a fallback mode now so
> we can make it default? That would encourage adoption, wouldn't it?

Ok. I'll make SFDP reading the default method and implement the fallback path 
for SPI framework, in v2 patch.

Rajat

> 
> Simon
> 
> > Is it possible that a flash that supports SFDP parameter reading can send
> some incorrect
> > signature? In that case I'll have to add an alternate path for fail scenario.
> 
> You're checking the signature already. But you're not checking the
> return value of 'spi_flash_parse_sfdp'. See below.
> 
> >
> > >
> > > Regards
> > > Vignesh
> > >
> > > >  #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..5d2728cc00 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 = &param_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)  { @@ -1196,9
> > > +1432,26 @@ 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_USE_SFDP) {
> > > > +           flash->size = 0;
> > > > +           spi_flash_parse_sfdp(flash);
> 
> Check return value here and ensure the next block checking
> 'info->flags & SPI_FLASH_USE_SFDP' is executed?

Fallback path will be implemented here.

Rajat

> 
> Simon
> 
> > > > +   }
> > > > +
> > > >     /* 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_USE_SFDP)) {
> > > > +           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 +1466,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_USE_SFDP)) {
> > > > +           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 +1568,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_USE_SFDP) &&
> > > > +       (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%2Flist
> s.denx.de%2Flistinfo%2Fu-
> boot&amp;data=02%7C01%7Crajat.srivastava%40nxp.com%7C59ecda360c19
> 437aa84608d63f124063%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636765744965696179&amp;sdata=4IMDM2b9Zle2WKqDNNx45%2BkbvD5Q
> l7odSfccyS8jIK8%3D&amp;reserved=0

[U-Boot] [PATCH 2/3] fsl_qspi: Access flash above 16MB using SFDP

[Rajat Srivastava]

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>
---
 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, &regs->lut[lut_base + 1],
 		     OPRND0(1) | PAD0(LUT_PAD1) | INSTR0(LUT_WRITE));
 
+	/* Read SFDP information */
+	lut_base = SEQID_RDSFDP * 4;
+	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->lut[lut_base + 2], 0);
+	qspi_write32(priv->flags, &regs->lut[lut_base + 3], 0);
+
 	/* Lock the LUT */
 	qspi_write32(priv->flags, &regs->lutkey, LUT_KEY_VALUE);
 	qspi_write32(priv->flags, &regs->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, &regs->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, &regs->mcr);
+	qspi_write32(priv->flags, &regs->mcr,
+		     QSPI_MCR_CLR_RXF_MASK | QSPI_MCR_CLR_TXF_MASK |
+		     QSPI_MCR_RESERVED_MASK | QSPI_MCR_END_CFD_LE);
+	qspi_write32(priv->flags, &regs->rbct, QSPI_RBCT_RXBRD_USEIPS);
+
+	to_or_from = priv->sf_addr + priv->cur_amba_base;
+
+	while (len > 0) {
+		WATCHDOG_RESET();
+
+		qspi_write32(priv->flags, &regs->sfar, to_or_from);
+
+		size = (len > RX_BUFFER_SIZE) ?
+			RX_BUFFER_SIZE : len;
+
+		qspi_write32(priv->flags, &regs->ipcr,
+			     (seqid << QSPI_IPCR_SEQID_SHIFT) |
+			     size);
+		while (qspi_read32(priv->flags, &regs->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, &regs->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, &regs->mcr,
+			     qspi_read32(priv->flags, &regs->mcr) |
+			     QSPI_MCR_CLR_RXF_MASK);
+	}
+
+	qspi_write32(priv->flags, &regs->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

[U-Boot] [PATCH 3/3] spi_flash_ids: Enable SFDP parsing for s25fs512 flash

[Rajat Srivastava]

Signed-off-by: Rajat Srivastava <rajat.srivastava@nxp.com>
---
 drivers/mtd/spi/spi_flash_ids.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
index ad0a0c8150..db4a93c7f5 100644
--- a/drivers/mtd/spi/spi_flash_ids.c
+++ b/drivers/mtd/spi/spi_flash_ids.c
@@ -113,7 +113,8 @@ const struct spi_flash_info spi_flash_ids[] = {
 	{"s25fl256s_256k", INFO(0x010219, 0x4d00, 256 * 1024,   128, RD_FULL | WR_QPP) },
 	{"s25fs256s_64k",  INFO6(0x010219, 0x4d0181, 64 * 1024, 512, RD_FULL | WR_QPP | SECT_4K) },
 	{"s25fl256s_64k",  INFO(0x010219, 0x4d01,  64 * 1024,   512, RD_FULL | WR_QPP) },
-	{"s25fs512s",      INFO6(0x010220, 0x4d0081, 256 * 1024, 256, RD_FULL | WR_QPP | SECT_4K) },
+	{"s25fs512s",      INFO6(0x010220, 0x4d0081, 256 * 1024, 256, RD_FULL |
+				 WR_QPP | SECT_4K | SPI_FLASH_USE_SFDP) },
 	{"s25fl512s_256k", INFO(0x010220, 0x4d00, 256 * 1024,   256, RD_FULL | WR_QPP) },
 	{"s25fl512s_64k",  INFO(0x010220, 0x4d01,  64 * 1024,  1024, RD_FULL | WR_QPP) },
 	{"s25fl512s_512k", INFO(0x010220, 0x4f00, 256 * 1024,   256, RD_FULL | WR_QPP) },
-- 
2.14.1

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Simon Goldschmidt]

On 17.10.2018 13:27, Rajat Srivastava wrote:
> 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.
>
> Enable reading and parsing of SFDP parameters for Spansion's
> s25fs512 flash to auto-detect its size, its page size and the
> addressing mode it supports.

Why do you need driver-specific code to read the sfdp parameters? 
Wouldn't it be much cleaner to solve this from spi_flash.c only?

If so, just adding the 'spi_flash_parse_sfdp()' function on top of 
Stefan's patch should work?

I tried testing this patch, but just like for Stefan, but it just didn't 
work for me and I failed to see some simple steps to adjust my spi 
driver to make it work.

Simon

>
> Rajat Srivastava (3):
>    mtd/spi: Add JEDEC SFDP support in SPI framework
>    fsl_qspi: Access flash above 16MB using SFDP
>    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
>
>   drivers/mtd/spi/sf_internal.h   |   4 +
>   drivers/mtd/spi/spi_flash.c     | 297 +++++++++++++++++++++++++++++++++++++---
>   drivers/mtd/spi/spi_flash_ids.c |   3 +-
>   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
>   include/spi.h                   |   2 +
>   include/spi_flash.h             | 120 ++++++++++++++++
>   6 files changed, 501 insertions(+), 28 deletions(-)
>

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Simon Goldschmidt]

On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt
<simon.k.r.goldschmidt@gmail.com> wrote:
>
> On 17.10.2018 13:27, Rajat Srivastava wrote:
> > 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.
> >
> > Enable reading and parsing of SFDP parameters for Spansion's
> > s25fs512 flash to auto-detect its size, its page size and the
> > addressing mode it supports.
>
> Why do you need driver-specific code to read the sfdp parameters?
> Wouldn't it be much cleaner to solve this from spi_flash.c only?
>
> If so, just adding the 'spi_flash_parse_sfdp()' function on top of
> Stefan's patch should work?
>
> I tried testing this patch, but just like for Stefan, but it just didn't
> work for me and I failed to see some simple steps to adjust my spi
> driver to make it work.

And taking this further, would it make sense to add a config option
that dumps the whole flash chip table and only uses SFDP? Then we
could save a lot of space in SPL for boards where reading SFDP is
enough.

Simon

>
> Simon
>
> >
> > Rajat Srivastava (3):
> >    mtd/spi: Add JEDEC SFDP support in SPI framework
> >    fsl_qspi: Access flash above 16MB using SFDP
> >    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
> >
> >   drivers/mtd/spi/sf_internal.h   |   4 +
> >   drivers/mtd/spi/spi_flash.c     | 297 +++++++++++++++++++++++++++++++++++++---
> >   drivers/mtd/spi/spi_flash_ids.c |   3 +-
> >   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
> >   include/spi.h                   |   2 +
> >   include/spi_flash.h             | 120 ++++++++++++++++
> >   6 files changed, 501 insertions(+), 28 deletions(-)
> >
>

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Rajat Srivastava]

> -----Original Message-----
> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Sent: Wednesday, October 31, 2018 1:09 PM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>
> Cc: Jagan Teki <jagan@openedev.com>
> Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash
> above 16MB
> 
> On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com> wrote:
> >
> > On 17.10.2018 13:27, Rajat Srivastava wrote:
> > > 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.
> > >
> > > Enable reading and parsing of SFDP parameters for Spansion's
> > > s25fs512 flash to auto-detect its size, its page size and the
> > > addressing mode it supports.
> >
> > Why do you need driver-specific code to read the sfdp parameters?
> > Wouldn't it be much cleaner to solve this from spi_flash.c only?
> >
> > If so, just adding the 'spi_flash_parse_sfdp()' function on top of
> > Stefan's patch should work?
> >
> > I tried testing this patch, but just like for Stefan, but it just didn't
> > work for me and I failed to see some simple steps to adjust my spi
> > driver to make it work.
> 
> And taking this further, would it make sense to add a config option
> that dumps the whole flash chip table and only uses SFDP? Then we
> could save a lot of space in SPL for boards where reading SFDP is
> enough.

Does the whole flash chip table mean the table that consists CFI information 
as well as SFDP information? If that's so, could you please elaborate where 
would we dump and save this table? And how will this help in saving space in SPL?

Rajat

> 
> Simon
> 
> >
> > Simon
> >
> > >
> > > Rajat Srivastava (3):
> > >    mtd/spi: Add JEDEC SFDP support in SPI framework
> > >    fsl_qspi: Access flash above 16MB using SFDP
> > >    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
> > >
> > >   drivers/mtd/spi/sf_internal.h   |   4 +
> > >   drivers/mtd/spi/spi_flash.c     | 297
> +++++++++++++++++++++++++++++++++++++---
> > >   drivers/mtd/spi/spi_flash_ids.c |   3 +-
> > >   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
> > >   include/spi.h                   |   2 +
> > >   include/spi_flash.h             | 120 ++++++++++++++++
> > >   6 files changed, 501 insertions(+), 28 deletions(-)
> > >
> >

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Simon Goldschmidt]

On Wed, Oct 31, 2018 at 10:38 AM Rajat Srivastava
<rajat.srivastava@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > Sent: Wednesday, October 31, 2018 1:09 PM
> > To: Rajat Srivastava <rajat.srivastava@nxp.com>; U-Boot Mailing List <u-
> > boot at lists.denx.de>
> > Cc: Jagan Teki <jagan@openedev.com>
> > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash
> > above 16MB
> >
> > On Tue, Oct 30, 2018 at 9:35 PM Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com> wrote:
> > >
> > > On 17.10.2018 13:27, Rajat Srivastava wrote:
> > > > 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.
> > > >
> > > > Enable reading and parsing of SFDP parameters for Spansion's
> > > > s25fs512 flash to auto-detect its size, its page size and the
> > > > addressing mode it supports.
> > >
> > > Why do you need driver-specific code to read the sfdp parameters?
> > > Wouldn't it be much cleaner to solve this from spi_flash.c only?
> > >
> > > If so, just adding the 'spi_flash_parse_sfdp()' function on top of
> > > Stefan's patch should work?
> > >
> > > I tried testing this patch, but just like for Stefan, but it just didn't
> > > work for me and I failed to see some simple steps to adjust my spi
> > > driver to make it work.
> >
> > And taking this further, would it make sense to add a config option
> > that dumps the whole flash chip table and only uses SFDP? Then we
> > could save a lot of space in SPL for boards where reading SFDP is
> > enough.
>
> Does the whole flash chip table mean the table that consists CFI information
> as well as SFDP information? If that's so, could you please elaborate where
> would we dump and save this table? And how will this help in saving space in SPL?

I meant the 'spi_flash_ids' table in spi_flash_ids.c. This table is >
1 kByte for my configuration and it contains strings that add up to
roughly another kByte. So if we could remove this, we could save 2
kByte of unused rodata in SPL (for my SPL, rodata and rwdata are in
the same RAM).

I haven't looked into this in detail, so I don't know if this is
really possible. Just from looking at the table, it seems SFDP should
provide all the information contained therein?

Simon

>
> Rajat
>
> >
> > Simon
> >
> > >
> > > Simon
> > >
> > > >
> > > > Rajat Srivastava (3):
> > > >    mtd/spi: Add JEDEC SFDP support in SPI framework
> > > >    fsl_qspi: Access flash above 16MB using SFDP
> > > >    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
> > > >
> > > >   drivers/mtd/spi/sf_internal.h   |   4 +
> > > >   drivers/mtd/spi/spi_flash.c     | 297
> > +++++++++++++++++++++++++++++++++++++---
> > > >   drivers/mtd/spi/spi_flash_ids.c |   3 +-
> > > >   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
> > > >   include/spi.h                   |   2 +
> > > >   include/spi_flash.h             | 120 ++++++++++++++++
> > > >   6 files changed, 501 insertions(+), 28 deletions(-)
> > > >
> > >

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Rajat Srivastava]

> -----Original Message-----
> From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Sent: Wednesday, October 31, 2018 2:06 AM
> To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> Cc: jagan at openedev.com
> Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash
> above 16MB
> 
> On 17.10.2018 13:27, Rajat Srivastava wrote:
> > 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.
> >
> > Enable reading and parsing of SFDP parameters for Spansion's
> > s25fs512 flash to auto-detect its size, its page size and the
> > addressing mode it supports.
> 
> Why do you need driver-specific code to read the sfdp parameters?
> Wouldn't it be much cleaner to solve this from spi_flash.c only?

That would have been cleaner only if it was possible. To read SFDP parameters of
any flash, a READ_SFDP (0x5A) command needs to be sent to flash.

The mtd framework does not allow sending any command to any flash directly
from mtd layer. Any transaction initiated from mtd framework will call 
spi_xfer() function of the respective SPI driver which will further send the actual 
command to flash.

Linux has also implemented reading SFDP parameters functionality in a 
similar way.

> If so, just adding the 'spi_flash_parse_sfdp()' function on top of
> Stefan's patch should work?
> 
> I tried testing this patch, but just like for Stefan, but it just didn't
> work for me and I failed to see some simple steps to adjust my spi
> driver to make it work.

You can see these patches for SFDP implementation in SPI driver:
   https://patchwork.ozlabs.org/patch/985329/ 
   https://patchwork.ozlabs.org/patch/985328/ 

You, basically, need to send:
   - 0x5A command (READ_SFDP command)
   - 3-byte address (address will be provided by framework)
   - 8 dummy cycles

> 
> Simon
> 
> >
> > Rajat Srivastava (3):
> >    mtd/spi: Add JEDEC SFDP support in SPI framework
> >    fsl_qspi: Access flash above 16MB using SFDP
> >    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
> >
> >   drivers/mtd/spi/sf_internal.h   |   4 +
> >   drivers/mtd/spi/spi_flash.c     | 297
> +++++++++++++++++++++++++++++++++++++---
> >   drivers/mtd/spi/spi_flash_ids.c |   3 +-
> >   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
> >   include/spi.h                   |   2 +
> >   include/spi_flash.h             | 120 ++++++++++++++++
> >   6 files changed, 501 insertions(+), 28 deletions(-)
> >

[U-Boot] [PATCH 0/3] Read SFDP parameters and access flash above 16MB

[Simon Goldschmidt]

On Wed, Oct 31, 2018 at 9:41 AM Rajat Srivastava
<rajat.srivastava@nxp.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > Sent: Wednesday, October 31, 2018 2:06 AM
> > To: Rajat Srivastava <rajat.srivastava@nxp.com>; u-boot at lists.denx.de
> > Cc: jagan at openedev.com
> > Subject: Re: [U-Boot] [PATCH 0/3] Read SFDP parameters and access flash
> > above 16MB
> >
> > On 17.10.2018 13:27, Rajat Srivastava wrote:
> > > 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.
> > >
> > > Enable reading and parsing of SFDP parameters for Spansion's
> > > s25fs512 flash to auto-detect its size, its page size and the
> > > addressing mode it supports.
> >
> > Why do you need driver-specific code to read the sfdp parameters?
> > Wouldn't it be much cleaner to solve this from spi_flash.c only?
>
> That would have been cleaner only if it was possible. To read SFDP parameters of
> any flash, a READ_SFDP (0x5A) command needs to be sent to flash.
>
> The mtd framework does not allow sending any command to any flash directly
> from mtd layer. Any transaction initiated from mtd framework will call
> spi_xfer() function of the respective SPI driver which will further send the actual
> command to flash.
>
> Linux has also implemented reading SFDP parameters functionality in a
> similar way.

I don't know the Linux way. I only looked at the driver my platform
uses (cadence_qspi) and I can't match this to your driver. The cadence
driver more or less sends 'dout' as command, so using
'spi_flash_read_common()' in spi_flash.c should work.
Is this different in your driver?

> > If so, just adding the 'spi_flash_parse_sfdp()' function on top of
> > Stefan's patch should work?
> >
> > I tried testing this patch, but just like for Stefan, but it just didn't
> > work for me and I failed to see some simple steps to adjust my spi
> > driver to make it work.
>
> You can see these patches for SFDP implementation in SPI driver:
>    https://patchwork.ozlabs.org/patch/985329/
>    https://patchwork.ozlabs.org/patch/985328/

Well, of course I saw those patches. As written above, I had a hard
time adapting your fsl_qspi changes to the cadence_qspi driver and I
think it would be better if this wasn't needed.

Simon

>
> You, basically, need to send:
>    - 0x5A command (READ_SFDP command)
>    - 3-byte address (address will be provided by framework)
>    - 8 dummy cycles
>
> >
> > Simon
> >
> > >
> > > Rajat Srivastava (3):
> > >    mtd/spi: Add JEDEC SFDP support in SPI framework
> > >    fsl_qspi: Access flash above 16MB using SFDP
> > >    spi_flash_ids: Enable SFDP parsing for s25fs512 flash
> > >
> > >   drivers/mtd/spi/sf_internal.h   |   4 +
> > >   drivers/mtd/spi/spi_flash.c     | 297
> > +++++++++++++++++++++++++++++++++++++---
> > >   drivers/mtd/spi/spi_flash_ids.c |   3 +-
> > >   drivers/spi/fsl_qspi.c          | 103 +++++++++++++-
> > >   include/spi.h                   |   2 +
> > >   include/spi_flash.h             | 120 ++++++++++++++++
> > >   6 files changed, 501 insertions(+), 28 deletions(-)
> > >
>