[U-Boot] [PATCH v2 0/7] BAR/wait_ready logic updates

[U-Boot] [PATCH v2 0/7] BAR/wait_ready logic updates

[Jagan Teki]

BAR and spi_flash_cmd_wait_ready are updated to make more
module to add new status checks.

Changes for v2:
- Update bank_sel for non-bar case.
- split the spi_flash_cmd_wait_ready logic into SR and FSR patches.

Clone spi-nor branch u-boot-spi.git repo for changes
http://git.denx.de/?p=u-boot/u-boot-spi.git;a=tree;h=refs/heads/spi-nor;hb=refs/heads/spi-nor

Jagan Teki (7):
  spi: zynq_spi: Remove unneeded headers
  sf: Return proper bank_sel, if flash->bank_curr == bank_sel
  sf: Make BAR discovery, as spi_flash_read_bar
  sf: Optimize BAR write code
  sf: Make flash->flags use for generic usage
  sf: Update status reg check in spi_flash_cmd_wait_ready
  sf: Add FSR support to spi_flash_cmd_wait_ready

 drivers/mtd/spi/sf_internal.h |   5 ++
 drivers/mtd/spi/sf_ops.c      | 151 +++++++++++++++++++++---------------------
 drivers/mtd/spi/sf_probe.c    |  64 ++++++++++--------
 drivers/spi/zynq_spi.c        |   6 +-
 include/spi_flash.h           |   6 +-
 5 files changed, 121 insertions(+), 111 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH v2 1/7] spi: zynq_spi: Remove unneeded headers

[Jagan Teki]

- Removed unneeded inclusion of header files
- Add "Xilinx" on license text

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
---
 drivers/spi/zynq_spi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/spi/zynq_spi.c b/drivers/spi/zynq_spi.c
index 7ae1f0e..b9cf335 100644
--- a/drivers/spi/zynq_spi.c
+++ b/drivers/spi/zynq_spi.c
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2013 Inc.
+ * (C) Copyright 2013 Xilinx, Inc.
  * (C) Copyright 2015 Jagan Teki <jteki@openedev.com>
  *
  * Xilinx Zynq PS SPI controller driver (master mode only)
@@ -7,15 +7,11 @@
  * SPDX-License-Identifier:     GPL-2.0+
  */
 
-#include <config.h>
 #include <common.h>
 #include <dm.h>
-#include <errno.h>
 #include <malloc.h>
 #include <spi.h>
-#include <fdtdec.h>
 #include <asm/io.h>
-#include <asm/arch/hardware.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-- 
1.9.1

[U-Boot] [PATCH v2 2/7] sf: Return proper bank_sel, if flash->bank_curr == bank_sel

[Jagan Teki]

If computed bank_sel is same as flash->bank_curr which is
computed at probe time, then return the bank_sel instead of zero.

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
---
 drivers/mtd/spi/sf_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 38592f5..5cb4ef6 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -101,7 +101,7 @@ static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
 
 	if (flash->bank_curr == bank_sel) {
 		debug("SF: not require to enable bank%d\n", bank_sel);
-		return 0;
+		return bank_sel;
 	}
 
 	cmd = flash->bank_write_cmd;
-- 
1.9.1

[U-Boot] [PATCH v2 3/7] sf: Make BAR discovery, as spi_flash_read_bar

[Jagan Teki]

Add spi_flash_read_bar function for reading bar and discovering
bar commands at probe time.

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
---
 drivers/mtd/spi/sf_probe.c | 54 ++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index e0283dc..0483bed 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -99,6 +99,37 @@ static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
 	}
 }
 
+#ifdef CONFIG_SPI_FLASH_BAR
+static int spi_flash_read_bank(struct spi_flash *flash, u8 idcode0)
+{
+	u8 curr_bank = 0;
+	int ret;
+
+	if (flash->size <= SPI_FLASH_16MB_BOUN)
+		goto bank_end;
+
+	switch (idcode0) {
+	case SPI_FLASH_CFI_MFR_SPANSION:
+		flash->bank_read_cmd = CMD_BANKADDR_BRRD;
+		flash->bank_write_cmd = CMD_BANKADDR_BRWR;
+	default:
+		flash->bank_read_cmd = CMD_EXTNADDR_RDEAR;
+		flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
+	}
+
+	ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
+				    &curr_bank, 1);
+	if (ret) {
+		debug("SF: fail to read bank addr register\n");
+		return ret;
+	}
+
+bank_end:
+	flash->bank_curr = curr_bank;
+	return 0;
+}
+#endif
+
 static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 				     struct spi_flash *flash)
 {
@@ -106,6 +137,7 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 	u8 cmd;
 	u16 jedec = idcode[1] << 8 | idcode[2];
 	u16 ext_jedec = idcode[3] << 8 | idcode[4];
+	int ret;
 
 	/* Validate params from spi_flash_params table */
 	params = spi_flash_params_table;
@@ -235,25 +267,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
-	u8 curr_bank = 0;
-	if (flash->size > SPI_FLASH_16MB_BOUN) {
-		int ret;
-
-		flash->bank_read_cmd = (idcode[0] == 0x01) ?
-					CMD_BANKADDR_BRRD : CMD_EXTNADDR_RDEAR;
-		flash->bank_write_cmd = (idcode[0] == 0x01) ?
-					CMD_BANKADDR_BRWR : CMD_EXTNADDR_WREAR;
-
-		ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
-					    &curr_bank, 1);
-		if (ret) {
-			debug("SF: fail to read bank addr register\n");
-			return ret;
-		}
-		flash->bank_curr = curr_bank;
-	} else {
-		flash->bank_curr = curr_bank;
-	}
+	ret = spi_flash_read_bank(flash, idcode[0]);
+	if (ret < 0)
+		return ret;
 #endif
 
 	/* Flash powers up read-only, so clear BP# bits */
-- 
1.9.1

[U-Boot] [PATCH v2 4/7] sf: Optimize BAR write code

[Jagan Teki]

Optimized spi-flash bar writing code and also removed
unnecessary bank_sel in read_ops.

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
---
 drivers/mtd/spi/sf_ops.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 5cb4ef6..0f34450 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -94,15 +94,14 @@ int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
 #endif
 
 #ifdef CONFIG_SPI_FLASH_BAR
-static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
+static int spi_flash_write_bank(struct spi_flash *flash, u32 offset)
 {
-	u8 cmd;
+	u8 cmd, bank_sel;
 	int ret;
 
-	if (flash->bank_curr == bank_sel) {
-		debug("SF: not require to enable bank%d\n", bank_sel);
-		return bank_sel;
-	}
+	bank_sel = offset / (SPI_FLASH_16MB_BOUN << flash->shift);
+	if (bank_sel == flash->bank_curr)
+		goto bar_end;
 
 	cmd = flash->bank_write_cmd;
 	ret = spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1);
@@ -110,25 +109,10 @@ static int spi_flash_cmd_bankaddr_write(struct spi_flash *flash, u8 bank_sel)
 		debug("SF: fail to write bank register\n");
 		return ret;
 	}
-	flash->bank_curr = bank_sel;
-
-	return 0;
-}
-
-static int spi_flash_bank(struct spi_flash *flash, u32 offset)
-{
-	u8 bank_sel;
-	int ret;
-
-	bank_sel = offset / (SPI_FLASH_16MB_BOUN << flash->shift);
-
-	ret = spi_flash_cmd_bankaddr_write(flash, bank_sel);
-	if (ret) {
-		debug("SF: fail to set bank%d\n", bank_sel);
-		return ret;
-	}
 
-	return bank_sel;
+bar_end:
+	flash->bank_curr = bank_sel;
+	return flash->bank_curr;
 }
 #endif
 
@@ -284,7 +268,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 			spi_flash_dual_flash(flash, &erase_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		ret = spi_flash_bank(flash, erase_addr);
+		ret = spi_flash_write_bank(flash, erase_addr);
 		if (ret < 0)
 			return ret;
 #endif
@@ -326,7 +310,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 			spi_flash_dual_flash(flash, &write_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		ret = spi_flash_bank(flash, write_addr);
+		ret = spi_flash_write_bank(flash, write_addr);
 		if (ret < 0)
 			return ret;
 #endif
@@ -416,9 +400,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 			spi_flash_dual_flash(flash, &read_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		bank_sel = spi_flash_bank(flash, read_addr);
-		if (bank_sel < 0)
+		ret = spi_flash_write_bank(flash, read_addr);
+		if (ret < 0)
 			return ret;
+		bank_sel = flash->bank_curr;
 #endif
 		remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
 				(bank_sel + 1)) - offset;
-- 
1.9.1

[U-Boot] [PATCH v2 5/7] sf: Make flash->flags use for generic usage

[Jagan Teki]

Use the flash->flags for generic usage, not only for dm-spi-flash,
this will be used for future flag additions.

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
---
 drivers/mtd/spi/sf_internal.h | 4 ++++
 drivers/mtd/spi/sf_probe.c    | 6 ++----
 include/spi_flash.h           | 4 ++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 9fb5557..e97c716 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -47,6 +47,10 @@ enum {
 
 #define SST_WR		(SST_BP | SST_WP)
 
+enum spi_nor_option_flags {
+	SNOR_F_SST_WR		= (1 << 0),
+};
+
 #define SPI_FLASH_3B_ADDR_LEN		3
 #define SPI_FLASH_CMD_LEN		(1 + SPI_FLASH_3B_ADDR_LEN)
 #define SPI_FLASH_16MB_BOUN		0x1000000
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 0483bed..1de2bbb 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -164,15 +164,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 	flash->name = params->name;
 	flash->memory_map = spi->memory_map;
 	flash->dual_flash = flash->spi->option;
-#ifdef CONFIG_DM_SPI_FLASH
-	flash->flags = params->flags;
-#endif
 
 	/* Assign spi_flash ops */
 #ifndef CONFIG_DM_SPI_FLASH
 	flash->write = spi_flash_cmd_write_ops;
 #if defined(CONFIG_SPI_FLASH_SST)
 	if (params->flags & SST_WR) {
+		flash->flags |= SNOR_F_SST_WR;
 		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
 			flash->write = sst_write_bp;
 		else
@@ -467,7 +465,7 @@ int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
 	struct spi_flash *flash = dev_get_uclass_priv(dev);
 
 #if defined(CONFIG_SPI_FLASH_SST)
-	if (flash->flags & SST_WR) {
+	if (flash->flags & SNOR_F_SST_WR) {
 		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
 			return sst_write_bp(flash, offset, len, buf);
 		else
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 3b2d555..8d85468 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -38,10 +38,10 @@ struct spi_slave;
  *
  * @spi:		SPI slave
  * @dev:		SPI flash device
- * @flags:		Indication of spi flash flags
  * @name:		Name of SPI flash
  * @dual_flash:		Indicates dual flash memories - dual stacked, parallel
  * @shift:		Flash shift useful in dual parallel
+ * @flags:		Indication of spi flash flags
  * @size:		Total flash size
  * @page_size:		Write (page) size
  * @sector_size:	Sector size
@@ -67,11 +67,11 @@ struct spi_flash {
 	struct spi_slave *spi;
 #ifdef CONFIG_DM_SPI_FLASH
 	struct udevice *dev;
-	u16 flags;
 #endif
 	const char *name;
 	u8 dual_flash;
 	u8 shift;
+	u16 flags;
 
 	u32 size;
 	u32 page_size;
-- 
1.9.1

[U-Boot] [PATCH v2 6/7] sf: Update status reg check in spi_flash_cmd_wait_ready

[Jagan Teki]

Current flash wait_ready logic is not modular to add new
register status check, hence few of the logic is used from
Linux spi-nor framework.

Below are the sf speed runs with 'sf update' on whole flash, 16MiB.

=> sf update 0x100 0x0 0x1000000
device 0 whole chip
16777216 bytes written, 0 bytes skipped in 59.564s, speed 289262 B/s

=> sf update 0x100 0x0 0x1000000
device 0 whole chip
16777216 bytes written, 0 bytes skipped in 62.549s, speed 275036 B/s

=> sf update 0x100 0x0 0x1000000
device 0 whole chip
16777216 bytes written, 0 bytes skipped in 61.276s, speed 284359 B/s

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Tom Warren <twarren@nvidia.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Tested-by: Jagan Teki <jteki@openedev.com>
---
 drivers/mtd/spi/sf_ops.c | 76 ++++++++++++------------------------------------
 1 file changed, 18 insertions(+), 58 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 0f34450..7d7c264 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -138,72 +138,32 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
 }
 #endif
 
-static int spi_flash_poll_status(struct spi_slave *spi, unsigned long timeout,
-				 u8 cmd, u8 poll_bit)
+/*
+ * Service routine to read status register until ready, or timeout occurs.
+ * Returns non-zero if error.
+ */
+int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long deadline)
 {
-	unsigned long timebase;
-	unsigned long flags = SPI_XFER_BEGIN;
-	int ret;
-	u8 status;
-	u8 check_status = 0x0;
+	int timeout, ret, sr;
 
-	if (cmd == CMD_FLAG_STATUS)
-		check_status = poll_bit;
+	timeout = get_timer(0);
 
-#ifdef CONFIG_SF_DUAL_FLASH
-	if (spi->flags & SPI_XFER_U_PAGE)
-		flags |= SPI_XFER_U_PAGE;
-#endif
-	ret = spi_xfer(spi, 8, &cmd, NULL, flags);
-	if (ret) {
-		debug("SF: fail to read %s status register\n",
-		      cmd == CMD_READ_STATUS ? "read" : "flag");
-		return ret;
-	}
-
-	timebase = get_timer(0);
-	do {
-		WATCHDOG_RESET();
-
-		ret = spi_xfer(spi, 8, NULL, &status, 0);
-		if (ret)
-			return -1;
+	while (get_timer(timeout) < deadline) {
+		ret = spi_flash_cmd_read_status(flash, &sr);
+		if (ret < 0)
+			return ret;
 
-		if ((status & poll_bit) == check_status)
+		if (sr < 0)
 			break;
+		else if (!(sr & STATUS_WIP))
+			return 0;
 
-	} while (get_timer(timebase) < timeout);
-
-	spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
-
-	if ((status & poll_bit) == check_status)
-		return 0;
-
-	/* Timed out */
-	debug("SF: time out!\n");
-	return -1;
-}
-
-int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
-{
-	struct spi_slave *spi = flash->spi;
-	int ret;
-	u8 poll_bit = STATUS_WIP;
-	u8 cmd = CMD_READ_STATUS;
-
-	ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
-	if (ret < 0)
-		return ret;
-
-	if (flash->poll_cmd == CMD_FLAG_STATUS) {
-		poll_bit = STATUS_PEC;
-		cmd = CMD_FLAG_STATUS;
-		ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
-		if (ret < 0)
-			return ret;
+		cond_resched();
 	}
 
-	return 0;
+	printf("SF: Timeout!\n");
+
+	return -ETIMEDOUT;
 }
 
 int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
-- 
1.9.1

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Jagan Teki]

This patch adds flag status register reading support to
spi_flash_cmd_wait_ready.

Signed-off-by: Jagan Teki <jteki@openedev.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Marek Vasut <marex@denx.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Tom Warren <twarren@nvidia.com>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Hou Zhiqiang <B48286@freescale.com>
Tested-by: Jagan Teki <jteki@openedev.com>
---
 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 66 +++++++++++++++++++++++++++++++++++++++----
 drivers/mtd/spi/sf_probe.c    |  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index e97c716..4ecfd0c 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -49,6 +49,7 @@ enum {
 
 enum spi_nor_option_flags {
 	SNOR_F_SST_WR		= (1 << 0),
+	SNOR_F_USE_FSR		= (1 << 1),
 };
 
 #define SPI_FLASH_3B_ADDR_LEN		3
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 7d7c264..a5487ad 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs)
 	return 0;
 }
 
+static int read_fsr(struct spi_flash *flash, u8 *fsr)
+{
+	int ret;
+	u8 cmd;
+
+	cmd = CMD_FLAG_STATUS;
+	ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
+	if (ret < 0) {
+		debug("SF: fail to read flag status register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
 {
 	u8 cmd;
@@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
 }
 #endif
 
+static inline int spi_flash_sr_ready(struct spi_flash *flash)
+{
+	u8 sr;
+	int ret;
+
+	ret = spi_flash_cmd_read_status(flash, &sr);
+	if (ret < 0)
+		return ret;
+
+	if (sr < 0)
+		return sr;
+	else
+		return !(sr & STATUS_WIP);
+}
+
+static inline int spi_flash_fsr_ready(struct spi_flash *flash)
+{
+	u8 fsr;
+	int ret;
+
+	ret = read_fsr(flash, &fsr);
+	if (ret < 0)
+		return ret;
+
+	if (fsr < 0)
+		return fsr;
+	else
+		return fsr & STATUS_PEC;
+}
+
+static int spi_flash_ready(struct spi_flash *flash)
+{
+	int sr, fsr;
+	sr = spi_flash_sr_ready(flash);
+	if (sr < 0)
+		return sr;
+	fsr = flash->flags & SNOR_F_USE_FSR ? spi_flash_fsr_ready(flash) : 1;
+	if (fsr < 0)
+		return fsr;
+	return sr && fsr;
+}
+
 /*
  * Service routine to read status register until ready, or timeout occurs.
  * Returns non-zero if error.
  */
 int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long deadline)
 {
-	int timeout, ret, sr;
+	int timeout, ret;
 
 	timeout = get_timer(0);
 
 	while (get_timer(timeout) < deadline) {
-		ret = spi_flash_cmd_read_status(flash, &sr);
+		ret = spi_flash_ready(flash);
 		if (ret < 0)
 			return ret;
-
-		if (sr < 0)
-			break;
-		else if (!(sr & STATUS_WIP))
+		if (ret)
 			return 0;
 
 		cond_resched();
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 1de2bbb..fb79b02 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -256,11 +256,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 		flash->dummy_byte = 1;
 	}
 
-	/* Poll cmd selection */
-	flash->poll_cmd = CMD_READ_STATUS;
 #ifdef CONFIG_SPI_FLASH_STMICRO
 	if (params->flags & E_FSR)
-		flash->poll_cmd = CMD_FLAG_STATUS;
+		flash->flags |= SNOR_F_USE_FSR;
 #endif
 
 	/* Configure the BAR - discover bank cmds and read current bank */
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 8d85468..4312d3d 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -49,7 +49,6 @@ struct spi_slave;
  * @bank_read_cmd:	Bank read cmd
  * @bank_write_cmd:	Bank write cmd
  * @bank_curr:		Current flash bank
- * @poll_cmd:		Poll cmd - for flash erase/program
  * @erase_cmd:		Erase cmd 4K, 32K, 64K
  * @read_cmd:		Read cmd - Array Fast, Extn read and quad read.
  * @write_cmd:		Write cmd - page and quad program.
@@ -82,7 +81,6 @@ struct spi_flash {
 	u8 bank_write_cmd;
 	u8 bank_curr;
 #endif
-	u8 poll_cmd;
 	u8 erase_cmd;
 	u8 read_cmd;
 	u8 write_cmd;
-- 
1.9.1

[U-Boot] [PATCH v2 0/7] BAR/wait_ready logic updates

[Bin Meng]

Hi Jagan,

On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
> BAR and spi_flash_cmd_wait_ready are updated to make more
> module to add new status checks.
>
> Changes for v2:
> - Update bank_sel for non-bar case.
> - split the spi_flash_cmd_wait_ready logic into SR and FSR patches.
>
> Clone spi-nor branch u-boot-spi.git repo for changes
> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=tree;h=refs/heads/spi-nor;hb=refs/heads/spi-nor
>
> Jagan Teki (7):
>   spi: zynq_spi: Remove unneeded headers
>   sf: Return proper bank_sel, if flash->bank_curr == bank_sel
>   sf: Make BAR discovery, as spi_flash_read_bar
>   sf: Optimize BAR write code
>   sf: Make flash->flags use for generic usage
>   sf: Update status reg check in spi_flash_cmd_wait_ready
>   sf: Add FSR support to spi_flash_cmd_wait_ready
>
>  drivers/mtd/spi/sf_internal.h |   5 ++
>  drivers/mtd/spi/sf_ops.c      | 151 +++++++++++++++++++++---------------------
>  drivers/mtd/spi/sf_probe.c    |  64 ++++++++++--------
>  drivers/spi/zynq_spi.c        |   6 +-
>  include/spi_flash.h           |   6 +-
>  5 files changed, 121 insertions(+), 111 deletions(-)
>
> --

Unfortunately with this series the SF flash was broken on Crown Bay.
Please take a look.

Regards,
Bin

[U-Boot] [PATCH v2 0/7] BAR/wait_ready logic updates

[Jagan Teki]

On 18 August 2015 at 10:12, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
>> BAR and spi_flash_cmd_wait_ready are updated to make more
>> module to add new status checks.
>>
>> Changes for v2:
>> - Update bank_sel for non-bar case.
>> - split the spi_flash_cmd_wait_ready logic into SR and FSR patches.
>>
>> Clone spi-nor branch u-boot-spi.git repo for changes
>> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=tree;h=refs/heads/spi-nor;hb=refs/heads/spi-nor
>>
>> Jagan Teki (7):
>>   spi: zynq_spi: Remove unneeded headers
>>   sf: Return proper bank_sel, if flash->bank_curr == bank_sel
>>   sf: Make BAR discovery, as spi_flash_read_bar
>>   sf: Optimize BAR write code
>>   sf: Make flash->flags use for generic usage
>>   sf: Update status reg check in spi_flash_cmd_wait_ready
>>   sf: Add FSR support to spi_flash_cmd_wait_ready
>>
>>  drivers/mtd/spi/sf_internal.h |   5 ++
>>  drivers/mtd/spi/sf_ops.c      | 151 +++++++++++++++++++++---------------------
>>  drivers/mtd/spi/sf_probe.c    |  64 ++++++++++--------
>>  drivers/spi/zynq_spi.c        |   6 +-
>>  include/spi_flash.h           |   6 +-
>>  5 files changed, 121 insertions(+), 111 deletions(-)
>>
>> --
>
> Unfortunately with this series the SF flash was broken on Crown Bay.
> Please take a look.

Please specify which sort of error it is from, "Timeout" form
wait_ready or something.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 0/7] BAR/wait_ready logic updates

[Bin Meng]

Hi Jagan,

On Tue, Aug 18, 2015 at 1:22 PM, Jagan Teki <jteki@openedev.com> wrote:
> On 18 August 2015 at 10:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Jagan,
>>
>> On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
>>> BAR and spi_flash_cmd_wait_ready are updated to make more
>>> module to add new status checks.
>>>
>>> Changes for v2:
>>> - Update bank_sel for non-bar case.
>>> - split the spi_flash_cmd_wait_ready logic into SR and FSR patches.
>>>
>>> Clone spi-nor branch u-boot-spi.git repo for changes
>>> http://git.denx.de/?p=u-boot/u-boot-spi.git;a=tree;h=refs/heads/spi-nor;hb=refs/heads/spi-nor
>>>
>>> Jagan Teki (7):
>>>   spi: zynq_spi: Remove unneeded headers
>>>   sf: Return proper bank_sel, if flash->bank_curr == bank_sel
>>>   sf: Make BAR discovery, as spi_flash_read_bar
>>>   sf: Optimize BAR write code
>>>   sf: Make flash->flags use for generic usage
>>>   sf: Update status reg check in spi_flash_cmd_wait_ready
>>>   sf: Add FSR support to spi_flash_cmd_wait_ready
>>>
>>>  drivers/mtd/spi/sf_internal.h |   5 ++
>>>  drivers/mtd/spi/sf_ops.c      | 151 +++++++++++++++++++++---------------------
>>>  drivers/mtd/spi/sf_probe.c    |  64 ++++++++++--------
>>>  drivers/spi/zynq_spi.c        |   6 +-
>>>  include/spi_flash.h           |   6 +-
>>>  5 files changed, 121 insertions(+), 111 deletions(-)
>>>
>>> --
>>
>> Unfortunately with this series the SF flash was broken on Crown Bay.
>> Please take a look.
>
> Please specify which sort of error it is from, "Timeout" form
> wait_ready or something.
>

I will need apply the patch in this series one by one and find which one breaks.

Regards,
Bin

[U-Boot] [PATCH v2 3/7] sf: Make BAR discovery, as spi_flash_read_bar

[Bin Meng]

Hi Jagan,

On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
> Add spi_flash_read_bar function for reading bar and discovering
> bar commands at probe time.
>
> Signed-off-by: Jagan Teki <jteki@openedev.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> ---
>  drivers/mtd/spi/sf_probe.c | 54 ++++++++++++++++++++++++++++++----------------
>  1 file changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index e0283dc..0483bed 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -99,6 +99,37 @@ static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
>         }
>  }
>
> +#ifdef CONFIG_SPI_FLASH_BAR
> +static int spi_flash_read_bank(struct spi_flash *flash, u8 idcode0)
> +{
> +       u8 curr_bank = 0;
> +       int ret;
> +
> +       if (flash->size <= SPI_FLASH_16MB_BOUN)
> +               goto bank_end;
> +
> +       switch (idcode0) {
> +       case SPI_FLASH_CFI_MFR_SPANSION:
> +               flash->bank_read_cmd = CMD_BANKADDR_BRRD;
> +               flash->bank_write_cmd = CMD_BANKADDR_BRWR;
> +       default:
> +               flash->bank_read_cmd = CMD_EXTNADDR_RDEAR;
> +               flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
> +       }
> +
> +       ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
> +                                   &curr_bank, 1);
> +       if (ret) {
> +               debug("SF: fail to read bank addr register\n");
> +               return ret;
> +       }
> +
> +bank_end:
> +       flash->bank_curr = curr_bank;
> +       return 0;
> +}
> +#endif
> +
>  static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>                                      struct spi_flash *flash)
>  {
> @@ -106,6 +137,7 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>         u8 cmd;
>         u16 jedec = idcode[1] << 8 | idcode[2];
>         u16 ext_jedec = idcode[3] << 8 | idcode[4];
> +       int ret;
>
>         /* Validate params from spi_flash_params table */
>         params = spi_flash_params_table;
> @@ -235,25 +267,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>
>         /* Configure the BAR - discover bank cmds and read current bank */
>  #ifdef CONFIG_SPI_FLASH_BAR
> -       u8 curr_bank = 0;
> -       if (flash->size > SPI_FLASH_16MB_BOUN) {
> -               int ret;
> -
> -               flash->bank_read_cmd = (idcode[0] == 0x01) ?
> -                                       CMD_BANKADDR_BRRD : CMD_EXTNADDR_RDEAR;
> -               flash->bank_write_cmd = (idcode[0] == 0x01) ?
> -                                       CMD_BANKADDR_BRWR : CMD_EXTNADDR_WREAR;
> -
> -               ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
> -                                           &curr_bank, 1);
> -               if (ret) {
> -                       debug("SF: fail to read bank addr register\n");
> -                       return ret;
> -               }
> -               flash->bank_curr = curr_bank;
> -       } else {
> -               flash->bank_curr = curr_bank;
> -       }
> +       ret = spi_flash_read_bank(flash, idcode[0]);
> +       if (ret < 0)
> +               return ret;
>  #endif
>
>         /* Flash powers up read-only, so clear BP# bits */
> --

This commit is causing build warnings:

drivers/mtd/spi/sf_probe.c: In function ?spi_flash_validate_params?:
drivers/mtd/spi/sf_probe.c:140:6: warning: unused variable ?ret?
[-Wunused-variable]

Regards,
Bin

[U-Boot] [PATCH v2 5/7] sf: Make flash->flags use for generic usage

[Bin Meng]

Hi Jagan,

On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
> Use the flash->flags for generic usage, not only for dm-spi-flash,
> this will be used for future flag additions.
>
> Signed-off-by: Jagan Teki <jteki@openedev.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> ---
>  drivers/mtd/spi/sf_internal.h | 4 ++++
>  drivers/mtd/spi/sf_probe.c    | 6 ++----
>  include/spi_flash.h           | 4 ++--
>  3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 9fb5557..e97c716 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -47,6 +47,10 @@ enum {
>
>  #define SST_WR         (SST_BP | SST_WP)
>
> +enum spi_nor_option_flags {
> +       SNOR_F_SST_WR           = (1 << 0),
> +};
> +
>  #define SPI_FLASH_3B_ADDR_LEN          3
>  #define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_3B_ADDR_LEN)
>  #define SPI_FLASH_16MB_BOUN            0x1000000
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 0483bed..1de2bbb 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -164,15 +164,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>         flash->name = params->name;
>         flash->memory_map = spi->memory_map;
>         flash->dual_flash = flash->spi->option;
> -#ifdef CONFIG_DM_SPI_FLASH
> -       flash->flags = params->flags;
> -#endif
>
>         /* Assign spi_flash ops */
>  #ifndef CONFIG_DM_SPI_FLASH
>         flash->write = spi_flash_cmd_write_ops;
>  #if defined(CONFIG_SPI_FLASH_SST)
>         if (params->flags & SST_WR) {
> +               flash->flags |= SNOR_F_SST_WR;
>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>                         flash->write = sst_write_bp;
>                 else
> @@ -467,7 +465,7 @@ int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
>         struct spi_flash *flash = dev_get_uclass_priv(dev);
>
>  #if defined(CONFIG_SPI_FLASH_SST)
> -       if (flash->flags & SST_WR) {
> +       if (flash->flags & SNOR_F_SST_WR) {
>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>                         return sst_write_bp(flash, offset, len, buf);
>                 else
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 3b2d555..8d85468 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -38,10 +38,10 @@ struct spi_slave;
>   *
>   * @spi:               SPI slave
>   * @dev:               SPI flash device
> - * @flags:             Indication of spi flash flags
>   * @name:              Name of SPI flash
>   * @dual_flash:                Indicates dual flash memories - dual stacked, parallel
>   * @shift:             Flash shift useful in dual parallel
> + * @flags:             Indication of spi flash flags
>   * @size:              Total flash size
>   * @page_size:         Write (page) size
>   * @sector_size:       Sector size
> @@ -67,11 +67,11 @@ struct spi_flash {
>         struct spi_slave *spi;
>  #ifdef CONFIG_DM_SPI_FLASH
>         struct udevice *dev;
> -       u16 flags;
>  #endif
>         const char *name;
>         u8 dual_flash;
>         u8 shift;
> +       u16 flags;
>
>         u32 size;
>         u32 page_size;
> --

This is the commit that breaks the SF flash on Intel Crown Bay. I've
tested like this:

Power-on the board, type "setenv testcase whatever" and "saveenv", do
a "reset", and check if the "testcase" variable is there.

Unfortunately with this commit, sometimes my board does not even boot
after "reset". I checked that if must be the SF driver writes the
wrong places into the flash which corrupts the codes.

Regards,
Bin

[U-Boot] [PATCH v2 3/7] sf: Make BAR discovery, as spi_flash_read_bar

[Jagan Teki]

On 18 August 2015 at 13:16, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
>> Add spi_flash_read_bar function for reading bar and discovering
>> bar commands at probe time.
>>
>> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> ---
>>  drivers/mtd/spi/sf_probe.c | 54 ++++++++++++++++++++++++++++++----------------
>>  1 file changed, 35 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> index e0283dc..0483bed 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -99,6 +99,37 @@ static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
>>         }
>>  }
>>
>> +#ifdef CONFIG_SPI_FLASH_BAR
>> +static int spi_flash_read_bank(struct spi_flash *flash, u8 idcode0)
>> +{
>> +       u8 curr_bank = 0;
>> +       int ret;
>> +
>> +       if (flash->size <= SPI_FLASH_16MB_BOUN)
>> +               goto bank_end;
>> +
>> +       switch (idcode0) {
>> +       case SPI_FLASH_CFI_MFR_SPANSION:
>> +               flash->bank_read_cmd = CMD_BANKADDR_BRRD;
>> +               flash->bank_write_cmd = CMD_BANKADDR_BRWR;
>> +       default:
>> +               flash->bank_read_cmd = CMD_EXTNADDR_RDEAR;
>> +               flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
>> +       }
>> +
>> +       ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
>> +                                   &curr_bank, 1);
>> +       if (ret) {
>> +               debug("SF: fail to read bank addr register\n");
>> +               return ret;
>> +       }
>> +
>> +bank_end:
>> +       flash->bank_curr = curr_bank;
>> +       return 0;
>> +}
>> +#endif
>> +
>>  static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>>                                      struct spi_flash *flash)
>>  {
>> @@ -106,6 +137,7 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>>         u8 cmd;
>>         u16 jedec = idcode[1] << 8 | idcode[2];
>>         u16 ext_jedec = idcode[3] << 8 | idcode[4];
>> +       int ret;
>>
>>         /* Validate params from spi_flash_params table */
>>         params = spi_flash_params_table;
>> @@ -235,25 +267,9 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>>
>>         /* Configure the BAR - discover bank cmds and read current bank */
>>  #ifdef CONFIG_SPI_FLASH_BAR
>> -       u8 curr_bank = 0;
>> -       if (flash->size > SPI_FLASH_16MB_BOUN) {
>> -               int ret;
>> -
>> -               flash->bank_read_cmd = (idcode[0] == 0x01) ?
>> -                                       CMD_BANKADDR_BRRD : CMD_EXTNADDR_RDEAR;
>> -               flash->bank_write_cmd = (idcode[0] == 0x01) ?
>> -                                       CMD_BANKADDR_BRWR : CMD_EXTNADDR_WREAR;
>> -
>> -               ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
>> -                                           &curr_bank, 1);
>> -               if (ret) {
>> -                       debug("SF: fail to read bank addr register\n");
>> -                       return ret;
>> -               }
>> -               flash->bank_curr = curr_bank;
>> -       } else {
>> -               flash->bank_curr = curr_bank;
>> -       }
>> +       ret = spi_flash_read_bank(flash, idcode[0]);
>> +       if (ret < 0)
>> +               return ret;
>>  #endif
>>
>>         /* Flash powers up read-only, so clear BP# bits */
>> --
>
> This commit is causing build warnings:
>
> drivers/mtd/spi/sf_probe.c: In function ?spi_flash_validate_params?:
> drivers/mtd/spi/sf_probe.c:140:6: warning: unused variable ?ret?
> [-Wunused-variable]

Yes, this I knew - will fix the same.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 5/7] sf: Make flash->flags use for generic usage

[Jagan Teki]

Hi Bin,

On 18 August 2015 at 13:23, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Aug 17, 2015 at 6:32 PM, Jagan Teki <jteki@openedev.com> wrote:
>> Use the flash->flags for generic usage, not only for dm-spi-flash,
>> this will be used for future flag additions.
>>
>> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>  drivers/mtd/spi/sf_internal.h | 4 ++++
>>  drivers/mtd/spi/sf_probe.c    | 6 ++----
>>  include/spi_flash.h           | 4 ++--
>>  3 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 9fb5557..e97c716 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -47,6 +47,10 @@ enum {
>>
>>  #define SST_WR         (SST_BP | SST_WP)
>>
>> +enum spi_nor_option_flags {
>> +       SNOR_F_SST_WR           = (1 << 0),
>> +};
>> +
>>  #define SPI_FLASH_3B_ADDR_LEN          3
>>  #define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_3B_ADDR_LEN)
>>  #define SPI_FLASH_16MB_BOUN            0x1000000
>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> index 0483bed..1de2bbb 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -164,15 +164,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>>         flash->name = params->name;
>>         flash->memory_map = spi->memory_map;
>>         flash->dual_flash = flash->spi->option;
>> -#ifdef CONFIG_DM_SPI_FLASH
>> -       flash->flags = params->flags;
>> -#endif
>>
>>         /* Assign spi_flash ops */
>>  #ifndef CONFIG_DM_SPI_FLASH
>>         flash->write = spi_flash_cmd_write_ops;
>>  #if defined(CONFIG_SPI_FLASH_SST)
>>         if (params->flags & SST_WR) {
>> +               flash->flags |= SNOR_F_SST_WR;
>>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>>                         flash->write = sst_write_bp;
>>                 else
>> @@ -467,7 +465,7 @@ int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
>>         struct spi_flash *flash = dev_get_uclass_priv(dev);
>>
>>  #if defined(CONFIG_SPI_FLASH_SST)
>> -       if (flash->flags & SST_WR) {
>> +       if (flash->flags & SNOR_F_SST_WR) {
>>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>>                         return sst_write_bp(flash, offset, len, buf);
>>                 else
>> diff --git a/include/spi_flash.h b/include/spi_flash.h
>> index 3b2d555..8d85468 100644
>> --- a/include/spi_flash.h
>> +++ b/include/spi_flash.h
>> @@ -38,10 +38,10 @@ struct spi_slave;
>>   *
>>   * @spi:               SPI slave
>>   * @dev:               SPI flash device
>> - * @flags:             Indication of spi flash flags
>>   * @name:              Name of SPI flash
>>   * @dual_flash:                Indicates dual flash memories - dual stacked, parallel
>>   * @shift:             Flash shift useful in dual parallel
>> + * @flags:             Indication of spi flash flags
>>   * @size:              Total flash size
>>   * @page_size:         Write (page) size
>>   * @sector_size:       Sector size
>> @@ -67,11 +67,11 @@ struct spi_flash {
>>         struct spi_slave *spi;
>>  #ifdef CONFIG_DM_SPI_FLASH
>>         struct udevice *dev;
>> -       u16 flags;
>>  #endif
>>         const char *name;
>>         u8 dual_flash;
>>         u8 shift;
>> +       u16 flags;
>>
>>         u32 size;
>>         u32 page_size;
>> --
>
> This is the commit that breaks the SF flash on Intel Crown Bay. I've
> tested like this:
>
> Power-on the board, type "setenv testcase whatever" and "saveenv", do
> a "reset", and check if the "testcase" variable is there.
>
> Unfortunately with this commit, sometimes my board does not even boot
> after "reset". I checked that if must be the SF driver writes the
> wrong places into the flash which corrupts the codes.

Sorry, I couldn't find any wrong in this commit.

We assigned directly params (SST_WR, for SST flash's) flags to flash flags
and check the param flags in dm-spi-write calls. So instead of that this commit
assigns local macro to flash flags if SST_WR and this will check in dm-spi-write
calls instead of param flags.

Could you please provide any command error info say sf probe, sf erase/write

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 6/7] sf: Update status reg check in spi_flash_cmd_wait_ready

[Marek Vasut]

On Monday, August 17, 2015 at 12:32:53 PM, Jagan Teki wrote:
> Current flash wait_ready logic is not modular to add new
> register status check, hence few of the logic is used from
> Linux spi-nor framework.
> 
> Below are the sf speed runs with 'sf update' on whole flash, 16MiB.
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 59.564s, speed 289262 B/s
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 62.549s, speed 275036 B/s
> 
> => sf update 0x100 0x0 0x1000000
> device 0 whole chip
> 16777216 bytes written, 0 bytes skipped in 61.276s, speed 284359 B/s
> 
> Signed-off-by: Jagan Teki <jteki@openedev.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Tom Warren <twarren@nvidia.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Tested-by: Jagan Teki <jteki@openedev.com>
> ---
>  drivers/mtd/spi/sf_ops.c | 76
> ++++++++++++------------------------------------ 1 file changed, 18
> insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 0f34450..7d7c264 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -138,72 +138,32 @@ static void spi_flash_dual_flash(struct spi_flash
> *flash, u32 *addr) }
>  #endif
> 
> -static int spi_flash_poll_status(struct spi_slave *spi, unsigned long
> timeout, -				 u8 cmd, u8 poll_bit)
> +/*
> + * Service routine to read status register until ready, or timeout occurs.
> + * Returns non-zero if error.

If you're adding this sort of description, please make it a kerneldoc.

> + */
> +int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
> deadline) {

Please rename $deadline to $timeout and note in the kerneldoc that the
timeout is in mSec.

> -	unsigned long timebase;
> -	unsigned long flags = SPI_XFER_BEGIN;
> -	int ret;
> -	u8 status;
> -	u8 check_status = 0x0;
> +	int timeout, ret, sr;
> 
> -	if (cmd == CMD_FLAG_STATUS)
> -		check_status = poll_bit;
> +	timeout = get_timer(0);

This should be $start or something.

> -#ifdef CONFIG_SF_DUAL_FLASH
> -	if (spi->flags & SPI_XFER_U_PAGE)
> -		flags |= SPI_XFER_U_PAGE;
> -#endif
> -	ret = spi_xfer(spi, 8, &cmd, NULL, flags);
> -	if (ret) {
> -		debug("SF: fail to read %s status register\n",
> -		      cmd == CMD_READ_STATUS ? "read" : "flag");
> -		return ret;
> -	}
> -
> -	timebase = get_timer(0);
> -	do {
> -		WATCHDOG_RESET();
> -
> -		ret = spi_xfer(spi, 8, NULL, &status, 0);
> -		if (ret)
> -			return -1;
> +	while (get_timer(timeout) < deadline) {
> +		ret = spi_flash_cmd_read_status(flash, &sr);
> +		if (ret < 0)
> +			return ret;
> 
> -		if ((status & poll_bit) == check_status)
> +		if (sr < 0)
>  			break;
> +		else if (!(sr & STATUS_WIP))
> +			return 0;
> 
> -	} while (get_timer(timebase) < timeout);
> -
> -	spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
> -
> -	if ((status & poll_bit) == check_status)
> -		return 0;
> -
> -	/* Timed out */
> -	debug("SF: time out!\n");
> -	return -1;
> -}
> -
> -int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
> timeout) -{
> -	struct spi_slave *spi = flash->spi;
> -	int ret;
> -	u8 poll_bit = STATUS_WIP;
> -	u8 cmd = CMD_READ_STATUS;
> -
> -	ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (flash->poll_cmd == CMD_FLAG_STATUS) {
> -		poll_bit = STATUS_PEC;
> -		cmd = CMD_FLAG_STATUS;
> -		ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
> -		if (ret < 0)
> -			return ret;
> +		cond_resched();

Huh ? Why do you need cond_resched() here at all ? Drop it please.

>  	}
> 
> -	return 0;
> +	printf("SF: Timeout!\n");
> +
> +	return -ETIMEDOUT;
>  }
> 
>  int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Marek Vasut]

On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
> This patch adds flag status register reading support to
> spi_flash_cmd_wait_ready.
> 
> Signed-off-by: Jagan Teki <jteki@openedev.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Tom Warren <twarren@nvidia.com>
> Cc: Bin Meng <bmeng.cn@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Hou Zhiqiang <B48286@freescale.com>
> Tested-by: Jagan Teki <jteki@openedev.com>
> ---
>  drivers/mtd/spi/sf_internal.h |  1 +
>  drivers/mtd/spi/sf_ops.c      | 66
> +++++++++++++++++++++++++++++++++++++++---- drivers/mtd/spi/sf_probe.c   
> |  4 +--
>  include/spi_flash.h           |  2 --
>  4 files changed, 62 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index e97c716..4ecfd0c 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -49,6 +49,7 @@ enum {
> 
>  enum spi_nor_option_flags {
>  	SNOR_F_SST_WR		= (1 << 0),
> +	SNOR_F_USE_FSR		= (1 << 1),
>  };
> 
>  #define SPI_FLASH_3B_ADDR_LEN		3
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 7d7c264..a5487ad 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash *flash,
> u8 *rs) return 0;
>  }
> 
> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
> +{
> +	int ret;
> +	u8 cmd;
> +
> +	cmd = CMD_FLAG_STATUS;
> +	ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
> +	if (ret < 0) {
> +		debug("SF: fail to read flag status register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
>  {
>  	u8 cmd;
> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct spi_flash
> *flash, u32 *addr) }
>  #endif
> 
> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
> +{
> +	u8 sr;
> +	int ret;
> +
> +	ret = spi_flash_cmd_read_status(flash, &sr);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (sr < 0)

Have you ever seen u8 value that's < 0 ? :-)

> +		return sr;
> +	else
> +		return !(sr & STATUS_WIP);
> +}

[...]

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Jagan Teki]

On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
> On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
>> This patch adds flag status register reading support to
>> spi_flash_cmd_wait_ready.
>>
>> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Tom Warren <twarren@nvidia.com>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Hou Zhiqiang <B48286@freescale.com>
>> Tested-by: Jagan Teki <jteki@openedev.com>
>> ---
>>  drivers/mtd/spi/sf_internal.h |  1 +
>>  drivers/mtd/spi/sf_ops.c      | 66
>> +++++++++++++++++++++++++++++++++++++++---- drivers/mtd/spi/sf_probe.c
>> |  4 +--
>>  include/spi_flash.h           |  2 --
>>  4 files changed, 62 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index e97c716..4ecfd0c 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -49,6 +49,7 @@ enum {
>>
>>  enum spi_nor_option_flags {
>>       SNOR_F_SST_WR           = (1 << 0),
>> +     SNOR_F_USE_FSR          = (1 << 1),
>>  };
>>
>>  #define SPI_FLASH_3B_ADDR_LEN                3
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 7d7c264..a5487ad 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash *flash,
>> u8 *rs) return 0;
>>  }
>>
>> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
>> +{
>> +     int ret;
>> +     u8 cmd;
>> +
>> +     cmd = CMD_FLAG_STATUS;
>> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
>> +     if (ret < 0) {
>> +             debug("SF: fail to read flag status register\n");
>> +             return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
>>  {
>>       u8 cmd;
>> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct spi_flash
>> *flash, u32 *addr) }
>>  #endif
>>
>> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
>> +{
>> +     u8 sr;
>> +     int ret;
>> +
>> +     ret = spi_flash_cmd_read_status(flash, &sr);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     if (sr < 0)
>
> Have you ever seen u8 value that's < 0 ? :-)

Yes, over looked, It's been fixed on next patches as well.

>
>> +             return sr;
>> +     else
>> +             return !(sr & STATUS_WIP);
>> +}

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 6/7] sf: Update status reg check in spi_flash_cmd_wait_ready

[Jagan Teki]

On 19 August 2015 at 03:32, Marek Vasut <marex@denx.de> wrote:
> On Monday, August 17, 2015 at 12:32:53 PM, Jagan Teki wrote:
>> Current flash wait_ready logic is not modular to add new
>> register status check, hence few of the logic is used from
>> Linux spi-nor framework.
>>
>> Below are the sf speed runs with 'sf update' on whole flash, 16MiB.
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 59.564s, speed 289262 B/s
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 62.549s, speed 275036 B/s
>>
>> => sf update 0x100 0x0 0x1000000
>> device 0 whole chip
>> 16777216 bytes written, 0 bytes skipped in 61.276s, speed 284359 B/s
>>
>> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Michal Simek <michal.simek@xilinx.com>
>> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Tom Warren <twarren@nvidia.com>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Tom Rini <trini@konsulko.com>
>> Tested-by: Jagan Teki <jteki@openedev.com>
>> ---
>>  drivers/mtd/spi/sf_ops.c | 76
>> ++++++++++++------------------------------------ 1 file changed, 18
>> insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 0f34450..7d7c264 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -138,72 +138,32 @@ static void spi_flash_dual_flash(struct spi_flash
>> *flash, u32 *addr) }
>>  #endif
>>
>> -static int spi_flash_poll_status(struct spi_slave *spi, unsigned long
>> timeout, -                             u8 cmd, u8 poll_bit)
>> +/*
>> + * Service routine to read status register until ready, or timeout occurs.
>> + * Returns non-zero if error.
>
> If you're adding this sort of description, please make it a kerneldoc.

Probably I will remove these comments, there are no comments before.

>
>> + */
>> +int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
>> deadline) {
>
> Please rename $deadline to $timeout and note in the kerneldoc that the
> timeout is in mSec.

Yes, will revert back to timeout

>
>> -     unsigned long timebase;
>> -     unsigned long flags = SPI_XFER_BEGIN;
>> -     int ret;
>> -     u8 status;
>> -     u8 check_status = 0x0;
>> +     int timeout, ret, sr;
>>
>> -     if (cmd == CMD_FLAG_STATUS)
>> -             check_status = poll_bit;
>> +     timeout = get_timer(0);
>
> This should be $start or something.
>
>> -#ifdef CONFIG_SF_DUAL_FLASH
>> -     if (spi->flags & SPI_XFER_U_PAGE)
>> -             flags |= SPI_XFER_U_PAGE;
>> -#endif
>> -     ret = spi_xfer(spi, 8, &cmd, NULL, flags);
>> -     if (ret) {
>> -             debug("SF: fail to read %s status register\n",
>> -                   cmd == CMD_READ_STATUS ? "read" : "flag");
>> -             return ret;
>> -     }
>> -
>> -     timebase = get_timer(0);
>> -     do {
>> -             WATCHDOG_RESET();
>> -
>> -             ret = spi_xfer(spi, 8, NULL, &status, 0);
>> -             if (ret)
>> -                     return -1;
>> +     while (get_timer(timeout) < deadline) {
>> +             ret = spi_flash_cmd_read_status(flash, &sr);
>> +             if (ret < 0)
>> +                     return ret;
>>
>> -             if ((status & poll_bit) == check_status)
>> +             if (sr < 0)
>>                       break;
>> +             else if (!(sr & STATUS_WIP))
>> +                     return 0;
>>
>> -     } while (get_timer(timebase) < timeout);
>> -
>> -     spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
>> -
>> -     if ((status & poll_bit) == check_status)
>> -             return 0;
>> -
>> -     /* Timed out */
>> -     debug("SF: time out!\n");
>> -     return -1;
>> -}
>> -
>> -int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
>> timeout) -{
>> -     struct spi_slave *spi = flash->spi;
>> -     int ret;
>> -     u8 poll_bit = STATUS_WIP;
>> -     u8 cmd = CMD_READ_STATUS;
>> -
>> -     ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
>> -     if (ret < 0)
>> -             return ret;
>> -
>> -     if (flash->poll_cmd == CMD_FLAG_STATUS) {
>> -             poll_bit = STATUS_PEC;
>> -             cmd = CMD_FLAG_STATUS;
>> -             ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
>> -             if (ret < 0)
>> -                     return ret;
>> +             cond_resched();
>
> Huh ? Why do you need cond_resched() here at all ? Drop it please.

Yes will do.

>
>>       }
>>
>> -     return 0;
>> +     printf("SF: Timeout!\n");
>> +
>> +     return -ETIMEDOUT;
>>  }

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Marek Vasut]

On Wednesday, August 19, 2015 at 12:36:40 AM, Jagan Teki wrote:
> On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
> > On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
> >> This patch adds flag status register reading support to
> >> spi_flash_cmd_wait_ready.
> >> 
> >> Signed-off-by: Jagan Teki <jteki@openedev.com>
> >> Cc: Simon Glass <sjg@chromium.org>
> >> Cc: Marek Vasut <marex@denx.de>
> >> Cc: Michal Simek <michal.simek@xilinx.com>
> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> Cc: Stefan Roese <sr@denx.de>
> >> Cc: Tom Warren <twarren@nvidia.com>
> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> Cc: Tom Rini <trini@konsulko.com>
> >> Cc: Hou Zhiqiang <B48286@freescale.com>
> >> Tested-by: Jagan Teki <jteki@openedev.com>
> >> ---
> >> 
> >>  drivers/mtd/spi/sf_internal.h |  1 +
> >>  drivers/mtd/spi/sf_ops.c      | 66
> >> 
> >> +++++++++++++++++++++++++++++++++++++++---- drivers/mtd/spi/sf_probe.c
> >> 
> >> |  4 +--
> >>  
> >>  include/spi_flash.h           |  2 --
> >>  4 files changed, 62 insertions(+), 11 deletions(-)
> >> 
> >> diff --git a/drivers/mtd/spi/sf_internal.h
> >> b/drivers/mtd/spi/sf_internal.h index e97c716..4ecfd0c 100644
> >> --- a/drivers/mtd/spi/sf_internal.h
> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> @@ -49,6 +49,7 @@ enum {
> >> 
> >>  enum spi_nor_option_flags {
> >>  
> >>       SNOR_F_SST_WR           = (1 << 0),
> >> 
> >> +     SNOR_F_USE_FSR          = (1 << 1),
> >> 
> >>  };
> >>  
> >>  #define SPI_FLASH_3B_ADDR_LEN                3
> >> 
> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> >> index 7d7c264..a5487ad 100644
> >> --- a/drivers/mtd/spi/sf_ops.c
> >> +++ b/drivers/mtd/spi/sf_ops.c
> >> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash
> >> *flash, u8 *rs) return 0;
> >> 
> >>  }
> >> 
> >> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
> >> +{
> >> +     int ret;
> >> +     u8 cmd;
> >> +
> >> +     cmd = CMD_FLAG_STATUS;
> >> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
> >> +     if (ret < 0) {
> >> +             debug("SF: fail to read flag status register\n");
> >> +             return ret;
> >> +     }
> >> +
> >> +     return 0;
> >> +}
> >> +
> >> 
> >>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
> >>  {
> >>  
> >>       u8 cmd;
> >> 
> >> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct spi_flash
> >> *flash, u32 *addr) }
> >> 
> >>  #endif
> >> 
> >> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
> >> +{
> >> +     u8 sr;
> >> +     int ret;
> >> +
> >> +     ret = spi_flash_cmd_read_status(flash, &sr);
> >> +     if (ret < 0)
> >> +             return ret;
> >> +
> >> +     if (sr < 0)
> > 
> > Have you ever seen u8 value that's < 0 ? :-)
> 
> Yes, over looked, It's been fixed on next patches as well.

Please don't apply broken code, fix this and repost.

Best regards,
Marek Vasut

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Jagan Teki]

On 19 August 2015 at 04:28, Marek Vasut <marex@denx.de> wrote:
> On Wednesday, August 19, 2015 at 12:36:40 AM, Jagan Teki wrote:
>> On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
>> > On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
>> >> This patch adds flag status register reading support to
>> >> spi_flash_cmd_wait_ready.
>> >>
>> >> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> >> Cc: Simon Glass <sjg@chromium.org>
>> >> Cc: Marek Vasut <marex@denx.de>
>> >> Cc: Michal Simek <michal.simek@xilinx.com>
>> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> Cc: Stefan Roese <sr@denx.de>
>> >> Cc: Tom Warren <twarren@nvidia.com>
>> >> Cc: Bin Meng <bmeng.cn@gmail.com>
>> >> Cc: Tom Rini <trini@konsulko.com>
>> >> Cc: Hou Zhiqiang <B48286@freescale.com>
>> >> Tested-by: Jagan Teki <jteki@openedev.com>
>> >> ---
>> >>
>> >>  drivers/mtd/spi/sf_internal.h |  1 +
>> >>  drivers/mtd/spi/sf_ops.c      | 66
>> >>
>> >> +++++++++++++++++++++++++++++++++++++++---- drivers/mtd/spi/sf_probe.c
>> >>
>> >> |  4 +--
>> >>
>> >>  include/spi_flash.h           |  2 --
>> >>  4 files changed, 62 insertions(+), 11 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/spi/sf_internal.h
>> >> b/drivers/mtd/spi/sf_internal.h index e97c716..4ecfd0c 100644
>> >> --- a/drivers/mtd/spi/sf_internal.h
>> >> +++ b/drivers/mtd/spi/sf_internal.h
>> >> @@ -49,6 +49,7 @@ enum {
>> >>
>> >>  enum spi_nor_option_flags {
>> >>
>> >>       SNOR_F_SST_WR           = (1 << 0),
>> >>
>> >> +     SNOR_F_USE_FSR          = (1 << 1),
>> >>
>> >>  };
>> >>
>> >>  #define SPI_FLASH_3B_ADDR_LEN                3
>> >>
>> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> >> index 7d7c264..a5487ad 100644
>> >> --- a/drivers/mtd/spi/sf_ops.c
>> >> +++ b/drivers/mtd/spi/sf_ops.c
>> >> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash
>> >> *flash, u8 *rs) return 0;
>> >>
>> >>  }
>> >>
>> >> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
>> >> +{
>> >> +     int ret;
>> >> +     u8 cmd;
>> >> +
>> >> +     cmd = CMD_FLAG_STATUS;
>> >> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
>> >> +     if (ret < 0) {
>> >> +             debug("SF: fail to read flag status register\n");
>> >> +             return ret;
>> >> +     }
>> >> +
>> >> +     return 0;
>> >> +}
>> >> +
>> >>
>> >>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
>> >>  {
>> >>
>> >>       u8 cmd;
>> >>
>> >> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct spi_flash
>> >> *flash, u32 *addr) }
>> >>
>> >>  #endif
>> >>
>> >> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
>> >> +{
>> >> +     u8 sr;
>> >> +     int ret;
>> >> +
>> >> +     ret = spi_flash_cmd_read_status(flash, &sr);
>> >> +     if (ret < 0)
>> >> +             return ret;
>> >> +
>> >> +     if (sr < 0)
>> >
>> > Have you ever seen u8 value that's < 0 ? :-)
>>
>> Yes, over looked, It's been fixed on next patches as well.
>
> Please don't apply broken code, fix this and repost.

I don't know what your talking about, there is patch already submitted
with this fix and even you're CCed [1]

[1] https://patchwork.ozlabs.org/patch/508166/

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Marek Vasut]

On Wednesday, August 19, 2015 at 09:35:32 AM, Jagan Teki wrote:
> On 19 August 2015 at 04:28, Marek Vasut <marex@denx.de> wrote:
> > On Wednesday, August 19, 2015 at 12:36:40 AM, Jagan Teki wrote:
> >> On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
> >> > On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
> >> >> This patch adds flag status register reading support to
> >> >> spi_flash_cmd_wait_ready.
> >> >> 
> >> >> Signed-off-by: Jagan Teki <jteki@openedev.com>
> >> >> Cc: Simon Glass <sjg@chromium.org>
> >> >> Cc: Marek Vasut <marex@denx.de>
> >> >> Cc: Michal Simek <michal.simek@xilinx.com>
> >> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >> Cc: Stefan Roese <sr@denx.de>
> >> >> Cc: Tom Warren <twarren@nvidia.com>
> >> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> >> Cc: Tom Rini <trini@konsulko.com>
> >> >> Cc: Hou Zhiqiang <B48286@freescale.com>
> >> >> Tested-by: Jagan Teki <jteki@openedev.com>
> >> >> ---
> >> >> 
> >> >>  drivers/mtd/spi/sf_internal.h |  1 +
> >> >>  drivers/mtd/spi/sf_ops.c      | 66
> >> >> 
> >> >> +++++++++++++++++++++++++++++++++++++++----
> >> >> drivers/mtd/spi/sf_probe.c
> >> >> 
> >> >> |  4 +--
> >> >>  
> >> >>  include/spi_flash.h           |  2 --
> >> >>  4 files changed, 62 insertions(+), 11 deletions(-)
> >> >> 
> >> >> diff --git a/drivers/mtd/spi/sf_internal.h
> >> >> b/drivers/mtd/spi/sf_internal.h index e97c716..4ecfd0c 100644
> >> >> --- a/drivers/mtd/spi/sf_internal.h
> >> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> >> @@ -49,6 +49,7 @@ enum {
> >> >> 
> >> >>  enum spi_nor_option_flags {
> >> >>  
> >> >>       SNOR_F_SST_WR           = (1 << 0),
> >> >> 
> >> >> +     SNOR_F_USE_FSR          = (1 << 1),
> >> >> 
> >> >>  };
> >> >>  
> >> >>  #define SPI_FLASH_3B_ADDR_LEN                3
> >> >> 
> >> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> >> >> index 7d7c264..a5487ad 100644
> >> >> --- a/drivers/mtd/spi/sf_ops.c
> >> >> +++ b/drivers/mtd/spi/sf_ops.c
> >> >> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash
> >> >> *flash, u8 *rs) return 0;
> >> >> 
> >> >>  }
> >> >> 
> >> >> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
> >> >> +{
> >> >> +     int ret;
> >> >> +     u8 cmd;
> >> >> +
> >> >> +     cmd = CMD_FLAG_STATUS;
> >> >> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
> >> >> +     if (ret < 0) {
> >> >> +             debug("SF: fail to read flag status register\n");
> >> >> +             return ret;
> >> >> +     }
> >> >> +
> >> >> +     return 0;
> >> >> +}
> >> >> +
> >> >> 
> >> >>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
> >> >>  {
> >> >>  
> >> >>       u8 cmd;
> >> >> 
> >> >> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct
> >> >> spi_flash *flash, u32 *addr) }
> >> >> 
> >> >>  #endif
> >> >> 
> >> >> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
> >> >> +{
> >> >> +     u8 sr;
> >> >> +     int ret;
> >> >> +
> >> >> +     ret = spi_flash_cmd_read_status(flash, &sr);
> >> >> +     if (ret < 0)
> >> >> +             return ret;
> >> >> +
> >> >> +     if (sr < 0)
> >> > 
> >> > Have you ever seen u8 value that's < 0 ? :-)
> >> 
> >> Yes, over looked, It's been fixed on next patches as well.
> > 
> > Please don't apply broken code, fix this and repost.
> 
> I don't know what your talking about, there is patch already submitted
> with this fix and even you're CCed [1]
> 
> [1] https://patchwork.ozlabs.org/patch/508166/

I am talking about not applying patches which are known to be defective.
If you plan to apply this particular patch, make sure to fix it, repost
it and then apply this. Do NOT apply a patch which is broken only to apply
another patch which repairs the breakage, that's just nonsense.

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Jagan Teki]

On 19 August 2015 at 13:09, Marek Vasut <marex@denx.de> wrote:
> On Wednesday, August 19, 2015 at 09:35:32 AM, Jagan Teki wrote:
>> On 19 August 2015 at 04:28, Marek Vasut <marex@denx.de> wrote:
>> > On Wednesday, August 19, 2015 at 12:36:40 AM, Jagan Teki wrote:
>> >> On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
>> >> > On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
>> >> >> This patch adds flag status register reading support to
>> >> >> spi_flash_cmd_wait_ready.
>> >> >>
>> >> >> Signed-off-by: Jagan Teki <jteki@openedev.com>
>> >> >> Cc: Simon Glass <sjg@chromium.org>
>> >> >> Cc: Marek Vasut <marex@denx.de>
>> >> >> Cc: Michal Simek <michal.simek@xilinx.com>
>> >> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
>> >> >> Cc: Stefan Roese <sr@denx.de>
>> >> >> Cc: Tom Warren <twarren@nvidia.com>
>> >> >> Cc: Bin Meng <bmeng.cn@gmail.com>
>> >> >> Cc: Tom Rini <trini@konsulko.com>
>> >> >> Cc: Hou Zhiqiang <B48286@freescale.com>
>> >> >> Tested-by: Jagan Teki <jteki@openedev.com>
>> >> >> ---
>> >> >>
>> >> >>  drivers/mtd/spi/sf_internal.h |  1 +
>> >> >>  drivers/mtd/spi/sf_ops.c      | 66
>> >> >>
>> >> >> +++++++++++++++++++++++++++++++++++++++----
>> >> >> drivers/mtd/spi/sf_probe.c
>> >> >>
>> >> >> |  4 +--
>> >> >>
>> >> >>  include/spi_flash.h           |  2 --
>> >> >>  4 files changed, 62 insertions(+), 11 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/mtd/spi/sf_internal.h
>> >> >> b/drivers/mtd/spi/sf_internal.h index e97c716..4ecfd0c 100644
>> >> >> --- a/drivers/mtd/spi/sf_internal.h
>> >> >> +++ b/drivers/mtd/spi/sf_internal.h
>> >> >> @@ -49,6 +49,7 @@ enum {
>> >> >>
>> >> >>  enum spi_nor_option_flags {
>> >> >>
>> >> >>       SNOR_F_SST_WR           = (1 << 0),
>> >> >>
>> >> >> +     SNOR_F_USE_FSR          = (1 << 1),
>> >> >>
>> >> >>  };
>> >> >>
>> >> >>  #define SPI_FLASH_3B_ADDR_LEN                3
>> >> >>
>> >> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> >> >> index 7d7c264..a5487ad 100644
>> >> >> --- a/drivers/mtd/spi/sf_ops.c
>> >> >> +++ b/drivers/mtd/spi/sf_ops.c
>> >> >> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash
>> >> >> *flash, u8 *rs) return 0;
>> >> >>
>> >> >>  }
>> >> >>
>> >> >> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
>> >> >> +{
>> >> >> +     int ret;
>> >> >> +     u8 cmd;
>> >> >> +
>> >> >> +     cmd = CMD_FLAG_STATUS;
>> >> >> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
>> >> >> +     if (ret < 0) {
>> >> >> +             debug("SF: fail to read flag status register\n");
>> >> >> +             return ret;
>> >> >> +     }
>> >> >> +
>> >> >> +     return 0;
>> >> >> +}
>> >> >> +
>> >> >>
>> >> >>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
>> >> >>  {
>> >> >>
>> >> >>       u8 cmd;
>> >> >>
>> >> >> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct
>> >> >> spi_flash *flash, u32 *addr) }
>> >> >>
>> >> >>  #endif
>> >> >>
>> >> >> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
>> >> >> +{
>> >> >> +     u8 sr;
>> >> >> +     int ret;
>> >> >> +
>> >> >> +     ret = spi_flash_cmd_read_status(flash, &sr);
>> >> >> +     if (ret < 0)
>> >> >> +             return ret;
>> >> >> +
>> >> >> +     if (sr < 0)
>> >> >
>> >> > Have you ever seen u8 value that's < 0 ? :-)
>> >>
>> >> Yes, over looked, It's been fixed on next patches as well.
>> >
>> > Please don't apply broken code, fix this and repost.
>>
>> I don't know what your talking about, there is patch already submitted
>> with this fix and even you're CCed [1]
>>
>> [1] https://patchwork.ozlabs.org/patch/508166/
>
> I am talking about not applying patches which are known to be defective.
> If you plan to apply this particular patch, make sure to fix it, repost
> it and then apply this. Do NOT apply a patch which is broken only to apply
> another patch which repairs the breakage, that's just nonsense.

Sorry, we are not here to apply broken patches, that you must
understand clearly.
And we haven't re-post the series yet, if you have any concerns just wait for
next series and comment. Any applied patches before I have re-posted to ML
then only applied.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v2 7/7] sf: Add FSR support to spi_flash_cmd_wait_ready

[Marek Vasut]

On Wednesday, August 19, 2015 at 10:17:59 AM, Jagan Teki wrote:
> On 19 August 2015 at 13:09, Marek Vasut <marex@denx.de> wrote:
> > On Wednesday, August 19, 2015 at 09:35:32 AM, Jagan Teki wrote:
> >> On 19 August 2015 at 04:28, Marek Vasut <marex@denx.de> wrote:
> >> > On Wednesday, August 19, 2015 at 12:36:40 AM, Jagan Teki wrote:
> >> >> On 19 August 2015 at 03:33, Marek Vasut <marex@denx.de> wrote:
> >> >> > On Monday, August 17, 2015 at 12:32:54 PM, Jagan Teki wrote:
> >> >> >> This patch adds flag status register reading support to
> >> >> >> spi_flash_cmd_wait_ready.
> >> >> >> 
> >> >> >> Signed-off-by: Jagan Teki <jteki@openedev.com>
> >> >> >> Cc: Simon Glass <sjg@chromium.org>
> >> >> >> Cc: Marek Vasut <marex@denx.de>
> >> >> >> Cc: Michal Simek <michal.simek@xilinx.com>
> >> >> >> Cc: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> >> >> >> Cc: Stefan Roese <sr@denx.de>
> >> >> >> Cc: Tom Warren <twarren@nvidia.com>
> >> >> >> Cc: Bin Meng <bmeng.cn@gmail.com>
> >> >> >> Cc: Tom Rini <trini@konsulko.com>
> >> >> >> Cc: Hou Zhiqiang <B48286@freescale.com>
> >> >> >> Tested-by: Jagan Teki <jteki@openedev.com>
> >> >> >> ---
> >> >> >> 
> >> >> >>  drivers/mtd/spi/sf_internal.h |  1 +
> >> >> >>  drivers/mtd/spi/sf_ops.c      | 66
> >> >> >> 
> >> >> >> +++++++++++++++++++++++++++++++++++++++----
> >> >> >> drivers/mtd/spi/sf_probe.c
> >> >> >> 
> >> >> >> |  4 +--
> >> >> >>  
> >> >> >>  include/spi_flash.h           |  2 --
> >> >> >>  4 files changed, 62 insertions(+), 11 deletions(-)
> >> >> >> 
> >> >> >> diff --git a/drivers/mtd/spi/sf_internal.h
> >> >> >> b/drivers/mtd/spi/sf_internal.h index e97c716..4ecfd0c 100644
> >> >> >> --- a/drivers/mtd/spi/sf_internal.h
> >> >> >> +++ b/drivers/mtd/spi/sf_internal.h
> >> >> >> @@ -49,6 +49,7 @@ enum {
> >> >> >> 
> >> >> >>  enum spi_nor_option_flags {
> >> >> >>  
> >> >> >>       SNOR_F_SST_WR           = (1 << 0),
> >> >> >> 
> >> >> >> +     SNOR_F_USE_FSR          = (1 << 1),
> >> >> >> 
> >> >> >>  };
> >> >> >>  
> >> >> >>  #define SPI_FLASH_3B_ADDR_LEN                3
> >> >> >> 
> >> >> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> >> >> >> index 7d7c264..a5487ad 100644
> >> >> >> --- a/drivers/mtd/spi/sf_ops.c
> >> >> >> +++ b/drivers/mtd/spi/sf_ops.c
> >> >> >> @@ -40,6 +40,21 @@ int spi_flash_cmd_read_status(struct spi_flash
> >> >> >> *flash, u8 *rs) return 0;
> >> >> >> 
> >> >> >>  }
> >> >> >> 
> >> >> >> +static int read_fsr(struct spi_flash *flash, u8 *fsr)
> >> >> >> +{
> >> >> >> +     int ret;
> >> >> >> +     u8 cmd;
> >> >> >> +
> >> >> >> +     cmd = CMD_FLAG_STATUS;
> >> >> >> +     ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
> >> >> >> +     if (ret < 0) {
> >> >> >> +             debug("SF: fail to read flag status register\n");
> >> >> >> +             return ret;
> >> >> >> +     }
> >> >> >> +
> >> >> >> +     return 0;
> >> >> >> +}
> >> >> >> +
> >> >> >> 
> >> >> >>  int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
> >> >> >>  {
> >> >> >>  
> >> >> >>       u8 cmd;
> >> >> >> 
> >> >> >> @@ -138,24 +153,63 @@ static void spi_flash_dual_flash(struct
> >> >> >> spi_flash *flash, u32 *addr) }
> >> >> >> 
> >> >> >>  #endif
> >> >> >> 
> >> >> >> +static inline int spi_flash_sr_ready(struct spi_flash *flash)
> >> >> >> +{
> >> >> >> +     u8 sr;
> >> >> >> +     int ret;
> >> >> >> +
> >> >> >> +     ret = spi_flash_cmd_read_status(flash, &sr);
> >> >> >> +     if (ret < 0)
> >> >> >> +             return ret;
> >> >> >> +
> >> >> >> +     if (sr < 0)
> >> >> > 
> >> >> > Have you ever seen u8 value that's < 0 ? :-)
> >> >> 
> >> >> Yes, over looked, It's been fixed on next patches as well.
> >> > 
> >> > Please don't apply broken code, fix this and repost.
> >> 
> >> I don't know what your talking about, there is patch already submitted
> >> with this fix and even you're CCed [1]
> >> 
> >> [1] https://patchwork.ozlabs.org/patch/508166/
> > 
> > I am talking about not applying patches which are known to be defective.
> > If you plan to apply this particular patch, make sure to fix it, repost
> > it and then apply this. Do NOT apply a patch which is broken only to
> > apply another patch which repairs the breakage, that's just nonsense.
> 
> Sorry, we are not here to apply broken patches, that you must
> understand clearly.

I'm stopping this discussion here, sorry.

> And we haven't re-post the series yet, if you have any concerns just wait
> for next series and comment. Any applied patches before I have re-posted
> to ML then only applied.

Sorry, I don't understand this sentence.

> thanks!