Skip to contentSkip to conversation

sf: BAR/wait_ready logic updates

PATCH Superseded Jagan Teki 3 participants 24 messages Sep 2, 2015 – Sep 29, 2015

Subsystems (3)
Memory technology devices (MTD) SPI Arm/Zynq architecture

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Jagan Teki

author
#0 lore raw

v3 cover sf: BAR/wait_ready logic updates

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

Hou Zhiqiang (1):
sf: Add clear flag status register support

Jagan Teki (12):
spi: zynq_spi: Remove unneeded headers
sf: Return 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
sf: Add flash_read_reg support
sf: Use flash_read_reg routine
sf: Add flash_read support
sf: Use flash_read routine in spi_flash_read_ops
sf: Rename read register functions

 drivers/mtd/spi/sf.c          |  46 ++++++++++
 drivers/mtd/spi/sf_internal.h |  30 ++++---
 drivers/mtd/spi/sf_ops.c      | 199 ++++++++++++++++++------------------------
 drivers/mtd/spi/sf_probe.c    |  80 +++++++++--------
 drivers/spi/zynq_spi.c        |   6 +-
 include/spi_flash.h           |   6 +-
 6 files changed, 195 insertions(+), 172 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v3 1/13 spi: zynq_spi: Remove unneeded headers

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

Signed-off-by Jagan Teki <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/spi/zynq_spi.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/spi/zynq_spi.c +1 -5
index 7ae1f0e..b9cf335 100644
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2013 Inc.
+ * (C) Copyright 2013 Xilinx, Inc.
  * (C) Copyright 2015 Jagan Teki <[email protected]>
  *
  * 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;
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v3 2/13 sf: Return bank_sel, if flash->bank_curr == bank_sel

In reply to 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 <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mtd/spi/sf_ops.c +1 -1
index 900ec1f..b33fe5a 100644
@@ -102,7 +102,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;
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v3 3/13 sf: Make BAR discovery, as spi_flash_read_bar

In reply to Jagan Teki

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

Signed-off-by Jagan Teki <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Fixed build warning about 'ret' variable usage

 drivers/mtd/spi/sf_probe.c | 53 +++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 19 deletions(-)
drivers/mtd/spi/sf_probe.c +34 -19
index 954376d..f17ec17 100644
@@ -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)
 {

@@ -235,25 +266,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;
-	}
+	int ret = spi_flash_read_bank(flash, idcode[0]);
+	if (ret < 0)
+		return ret;
 #endif
 
 	/* Flash powers up read-only, so clear BP# bits */
Signature · 1 line
1.9.1
author
#4 lore raw

PATCH v3 4/13 sf: Optimize BAR write code

In reply to Jagan Teki

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

Signed-off-by Jagan Teki <[email protected]>
Cc Simon Glass <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Use bank_sel variable in read ops

 drivers/mtd/spi/sf_ops.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)
drivers/mtd/spi/sf_ops.c +13 -28
index b33fe5a..6e457ec 100644
@@ -95,15 +95,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);

@@ -111,25 +110,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
 

@@ -285,7 +269,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

@@ -327,7 +311,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

@@ -422,9 +406,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;
Signature · 1 line
1.9.1
author
#5 lore raw

PATCH v3 5/13 sf: Make flash->flags use for generic usage

In reply to 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 <[email protected]>
Cc Bin Meng <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 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(-)
drivers/mtd/spi/sf_internal.h +4 -0
index 9c95d56..53998fc 100644
@@ -51,6 +51,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
drivers/mtd/spi/sf_probe.c +2 -4
index f17ec17..85390d4 100644
@@ -163,15 +163,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

@@ -466,7 +464,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
include/spi_flash.h +2 -2
index 3b2d555..8d85468 100644
@@ -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;
Signature · 1 line
1.9.1
#6 lore raw

PATCH v3 5/13 sf: Make flash->flags use for generic usage

Hi Jagan,

Quoted 8 lines · 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 <[email protected]>
Cc Bin Meng <[email protected]>

---
Changes for v3:
- none

I will need test this on Crown Bay. Last time this commit fails. But I see the changelog says no changes? Does this rebase to latest u-boot/master?

Quoted 73 lines

Changes for v2:
- none

 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(-)
drivers/mtd/spi/sf_internal.h +4 -0
index 9c95d56..53998fc 100644
@@ -51,6 +51,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
drivers/mtd/spi/sf_probe.c +2 -4
index f17ec17..85390d4 100644
@@ -163,15 +163,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

@@ -466,7 +464,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
include/spi_flash.h +1 -1
index 3b2d555..8d85468 100644
@@ -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;

include/spi_flash.h struct spi_flash {

Quoted 10 lines
include/spi_flash.h +1 -2
-       u16 flags;
 #endif
        const char *name;
        u8 dual_flash;
        u8 shift;
+       u16 flags;
 
        u32 size;
        u32 page_size;
--

Regards,
Bin

author
#7 lore raw

PATCH v3 5/13 sf: Make flash->flags use for generic usage

Quoted 12 lines · Bin Meng

Hi Jagan,

Quoted 8 lines · 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 <[email protected]>
Cc Bin Meng <[email protected]>

---
Changes for v3:
- none

Quoted 3 lines

I will need test this on Crown Bay. Last time this commit fails. But I see the changelog says no changes? Does this rebase to latest u-boot/master?

No changes it's a rebase. if something fails pleas let me share the error log or something.

Quoted 84 lines
Quoted 84 lines

Changes for v2:
- none

 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(-)
drivers/mtd/spi/sf_internal.h +4 -0
index 9c95d56..53998fc 100644
@@ -51,6 +51,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
drivers/mtd/spi/sf_probe.c +2 -4
index f17ec17..85390d4 100644
@@ -163,15 +163,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

@@ -466,7 +464,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
include/spi_flash.h +2 -3
index 3b2d555..8d85468 100644
@@ -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;
--
Signature · 1 line
Jagan | openedev.
#8 lore raw

PATCH v3 5/13 sf: Make flash->flags use for generic usage

In reply to Jagan Teki

Hi Jagan,

Quoted 6 lines · 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 <[email protected]>
Cc Bin Meng <[email protected]>

---

This v3 commit still breaks Intel Crown Bay. I've tested on latest u-boot/master branch plus this single commit. A simple 'saveenv' just erase all contents without reporting any error.

=> md ffe00000
ffe00000: a38ec887 64756162 65746172 3531313d ....baudrate=115
ffe00010: 00303032 746f6f62 73677261 6f6f723d 200.bootargs=roo
ffe00020: 642f3d74 732f7665 20336264 74696e69 t=/dev/sdb3 init
ffe00030: 62732f3d 692f6e69 2074696e 746f6f72 =/sbin/init root
ffe00040: 74696177 006f7220 746f6f62 3d646d63 wait ro.bootcmd=
ffe00050: 32747865 64616f6c 73637320 3a302069 ext2load scsi 0:
ffe00060: 31302033 30303030 2f203030 746f6f62 3 01000000 /boot
ffe00070: 6c6d762f 7a756e69 627a203b 20746f6f /vmlinuz; zboot
ffe00080: 30303130 30303030 6f6f6200 6c696674 01000000.bootfil
ffe00090: 7a623d65 67616d49 6f630065 6c6f736e e=bzImage.consol
ffe000a0: 76656465 7974743d 65003053 61316874 edev=ttyS0.eth1a
ffe000b0: 3d726464 313a3030 37313a35 3a63653a ddr=00:15:17:ec:
ffe000c0: 653a3165 74650038 64613268 303d7264 e1:e8.eth2addr=0
ffe000d0: 35313a30 3a37313a 653a6365 39653a31 0:15:17:ec:e1:e9
ffe000e0: 68746500 72646461 3a30303d 323a3331 .ethaddr=00:13:2
ffe000f0: 39663a30 3a65303a 67006439 77657461 0:f9:0e:9d.gatew

=> saveenv
Saving Environment to SPI Flash...
Erasing SPI flash...Writing to SPI flash...done

=> md ffe00000
ffe00000: ffffff01 ffffffff ffffffff ffffffff ................

Note there is a suspicious 01 at the first byte location. I think the flash erase succeeded, but flash write does not and failed at the very first byte. But I don't know why it does not report any error to 'saveenv' (the upper layer).

ffe00010: ffffffff ffffffff ffffffff ffffffff ................
ffe00020: ffffffff ffffffff ffffffff ffffffff ................
ffe00030: ffffffff ffffffff ffffffff ffffffff ................
ffe00040: ffffffff ffffffff ffffffff ffffffff ................
ffe00050: ffffffff ffffffff ffffffff ffffffff ................
ffe00060: ffffffff ffffffff ffffffff ffffffff ................
ffe00070: ffffffff ffffffff ffffffff ffffffff ................
ffe00080: ffffffff ffffffff ffffffff ffffffff ................
ffe00090: ffffffff ffffffff ffffffff ffffffff ................
ffe000a0: ffffffff ffffffff ffffffff ffffffff ................
ffe000b0: ffffffff ffffffff ffffffff ffffffff ................
ffe000c0: ffffffff ffffffff ffffffff ffffffff ................
ffe000d0: ffffffff ffffffff ffffffff ffffffff ................
ffe000e0: ffffffff ffffffff ffffffff ffffffff ..............

Quoted 75 lines

Changes for v3:
- none
Changes for v2:
- none

 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(-)
drivers/mtd/spi/sf_internal.h +4 -0
index 9c95d56..53998fc 100644
@@ -51,6 +51,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
drivers/mtd/spi/sf_probe.c +2 -4
index f17ec17..85390d4 100644
@@ -163,15 +163,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

@@ -466,7 +464,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
include/spi_flash.h +1 -1
index 3b2d555..8d85468 100644
@@ -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;

include/spi_flash.h struct spi_flash {

Quoted 10 lines
include/spi_flash.h +1 -2
-       u16 flags;
 #endif
        const char *name;
        u8 dual_flash;
        u8 shift;
+       u16 flags;
 
        u32 size;
        u32 page_size;
--

Regards,
Bin

#9 lore raw

PATCH v3 5/13 sf: Make flash->flags use for generic usage

Quoted 44 lines · Bin Meng

Hi Jagan,

Quoted 6 lines · 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 <[email protected]>
Cc Bin Meng <[email protected]>

---

This v3 commit still breaks Intel Crown Bay. I've tested on latest u-boot/master branch plus this single commit. A simple 'saveenv' just erase all contents without reporting any error.

=> md ffe00000
ffe00000: a38ec887 64756162 65746172 3531313d ....baudrate=115
ffe00010: 00303032 746f6f62 73677261 6f6f723d 200.bootargs=roo
ffe00020: 642f3d74 732f7665 20336264 74696e69 t=/dev/sdb3 init
ffe00030: 62732f3d 692f6e69 2074696e 746f6f72 =/sbin/init root
ffe00040: 74696177 006f7220 746f6f62 3d646d63 wait ro.bootcmd=
ffe00050: 32747865 64616f6c 73637320 3a302069 ext2load scsi 0:
ffe00060: 31302033 30303030 2f203030 746f6f62 3 01000000 /boot
ffe00070: 6c6d762f 7a756e69 627a203b 20746f6f /vmlinuz; zboot
ffe00080: 30303130 30303030 6f6f6200 6c696674 01000000.bootfil
ffe00090: 7a623d65 67616d49 6f630065 6c6f736e e=bzImage.consol
ffe000a0: 76656465 7974743d 65003053 61316874 edev=ttyS0.eth1a
ffe000b0: 3d726464 313a3030 37313a35 3a63653a ddr=00:15:17:ec:
ffe000c0: 653a3165 74650038 64613268 303d7264 e1:e8.eth2addr=0
ffe000d0: 35313a30 3a37313a 653a6365 39653a31 0:15:17:ec:e1:e9
ffe000e0: 68746500 72646461 3a30303d 323a3331 .ethaddr=00:13:2
ffe000f0: 39663a30 3a65303a 67006439 77657461 0:f9:0e:9d.gatew

=> saveenv
Saving Environment to SPI Flash...
Erasing SPI flash...Writing to SPI flash...done

=> md ffe00000
ffe00000: ffffff01 ffffffff ffffffff ffffffff ................

Note there is a suspicious 01 at the first byte location. I think the flash erase succeeded, but flash write does not and failed at the very first byte. But I don't know why it does not report any error to 'saveenv' (the upper layer).

Quoted 15 lines

ffe00010: ffffffff ffffffff ffffffff ffffffff ................
ffe00020: ffffffff ffffffff ffffffff ffffffff ................
ffe00030: ffffffff ffffffff ffffffff ffffffff ................
ffe00040: ffffffff ffffffff ffffffff ffffffff ................
ffe00050: ffffffff ffffffff ffffffff ffffffff ................
ffe00060: ffffffff ffffffff ffffffff ffffffff ................
ffe00070: ffffffff ffffffff ffffffff ffffffff ................
ffe00080: ffffffff ffffffff ffffffff ffffffff ................
ffe00090: ffffffff ffffffff ffffffff ffffffff ................
ffe000a0: ffffffff ffffffff ffffffff ffffffff ................
ffe000b0: ffffffff ffffffff ffffffff ffffffff ................
ffe000c0: ffffffff ffffffff ffffffff ffffffff ................
ffe000d0: ffffffff ffffffff ffffffff ffffffff ................
ffe000e0: ffffffff ffffffff ffffffff ffffffff ..............

I've figured it out. Please check the patch @
http://patchwork.ozlabs.org/patch/517704/. Thanks!

[snip]

Regards,
Bin

author
#10 lore raw

PATCH v3 6/13 sf: Update status reg check in spi_flash_cmd_wait_ready

In reply to Jagan Teki

Current flash wait_ready logic is not modular to add new register status check, hence updated the status check for adding few more register checks in future.

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 <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Tested-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Remove unneeded comments.

 drivers/mtd/spi/sf_ops.c | 70 +++++++++---------------------------------------
 1 file changed, 12 insertions(+), 58 deletions(-)
drivers/mtd/spi/sf_ops.c +12 -58
index 6e457ec..c1cdde7 100644
@@ -139,72 +139,26 @@ 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)
-{
-	unsigned long timebase;
-	unsigned long flags = SPI_XFER_BEGIN;
-	int ret;
-	u8 status;
-	u8 check_status = 0x0;
-
-	if (cmd == CMD_FLAG_STATUS)
-		check_status = poll_bit;
-
-#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;
-
-		if ((status & poll_bit) == check_status)
-			break;
-
-	} 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;
+	int timebase, ret, sr;
 
-	ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
-	if (ret < 0)
-		return ret;
+	timebase = get_timer(0);
 
-	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);
+	while (get_timer(timebase) < timeout) {
+		ret = spi_flash_cmd_read_status(flash, &sr);
 		if (ret < 0)
 			return ret;
+
+		if (sr < 0)
+			break;
+		else if (!(sr & STATUS_WIP))
+			return 0;
 	}
 
-	return 0;
+	printf("SF: Timeout!\n");
+
+	return -ETIMEDOUT;
 }
 
 int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
Signature · 1 line
1.9.1
#11 lore raw

PATCH v3 6/13 sf: Update status reg check in spi_flash_cmd_wait_ready

Quoted 28 lines · Jagan Teki

Current flash wait_ready logic is not modular to add new register status check, hence updated the status check for adding few more register checks in future.

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 <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Tested-by Jagan Teki <[email protected]>
Quoted 1 line

---

Please fix the compiler warnings below:

drivers/mtd/spi/sf_ops.c: In function ?spi_flash_cmd_wait_ready?:
drivers/mtd/spi/sf_ops.c:165:3: warning: passing argument 2 of ?spi_flash_cmd_read_status? from incompatible pointer type [enabled by default]
drivers/mtd/spi/sf_ops.c:29:5: note: expected ?u8 *? but argument is of type ?int *?

Other than that,

Tested on Intel Crown Bay

Tested-by Bin Meng <[email protected]>
Quoted 88 lines

Changes for v3:
- none
Changes for v2:
- Remove unneeded comments.

 drivers/mtd/spi/sf_ops.c | 70 +++++++++---------------------------------------
 1 file changed, 12 insertions(+), 58 deletions(-)
drivers/mtd/spi/sf_ops.c +9 -57
index 6e457ec..c1cdde7 100644
@@ -139,72 +139,26 @@ 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)
-{
-       unsigned long timebase;
-       unsigned long flags = SPI_XFER_BEGIN;
-       int ret;
-       u8 status;
-       u8 check_status = 0x0;
-
-       if (cmd == CMD_FLAG_STATUS)
-               check_status = poll_bit;
-
-#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;
-
-               if ((status & poll_bit) == check_status)
-                       break;
-
-       } 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;
+       int timebase, ret, sr;
 
-       ret = spi_flash_poll_status(spi, timeout, cmd, poll_bit);
-       if (ret < 0)
-               return ret;
+       timebase = get_timer(0);
 
-       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);
+       while (get_timer(timebase) < timeout) {
+               ret = spi_flash_cmd_read_status(flash, &sr);
                if (ret < 0)
                        return ret;
+
+               if (sr < 0)
+                       break;
+               else if (!(sr & STATUS_WIP))
+                       return 0;

drivers/mtd/spi/sf_ops.c static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)

Quoted 10 lines
drivers/mtd/spi/sf_ops.c +3 -2
        }
 
-       return 0;
+       printf("SF: Timeout!\n");
+
+       return -ETIMEDOUT;
 }
 
 int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
--

Regards,
Bin

author
#12 lore raw

PATCH v3 7/13 sf: Add FSR support to spi_flash_cmd_wait_ready

In reply to Jagan Teki

This patch adds flag status register reading support to spi_flash_cmd_wait_ready.

Signed-off-by Jagan Teki <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Cc Hou Zhiqiang <[email protected]>
Tested-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Optime code for fsr

 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 60 ++++++++++++++++++++++++++++++++++++++-----
 drivers/mtd/spi/sf_probe.c    |  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 56 insertions(+), 11 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -0
index 53998fc..8a3e5ec 100644
@@ -53,6 +53,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
drivers/mtd/spi/sf_ops.c +54 -6

80 lines not on this page · raw message

drivers/mtd/spi/sf_probe.c +1 -3
index 85390d4..758347b 100644
@@ -255,11 +255,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 */
include/spi_flash.h +0 -2
index 8d85468..4312d3d 100644
@@ -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;
Signature · 1 line
1.9.1
#13 lore raw

PATCH v3 7/13 sf: Add FSR support to spi_flash_cmd_wait_ready

Quoted 44 lines · Jagan Teki

This patch adds flag status register reading support to spi_flash_cmd_wait_ready.

Signed-off-by Jagan Teki <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Cc Hou Zhiqiang <[email protected]>
Tested-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Optime code for fsr

 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 60
++++++++++++++++++++++++++++++++++++++----- drivers/mtd/spi/sf_probe.c   
|  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 56 insertions(+), 11 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -0
index 53998fc..8a3e5ec 100644
@@ -53,6 +53,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
drivers/mtd/spi/sf_ops.c +0 -0
index c1cdde7..85e15b9 100644

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_read_status(struct spi_flash *flash, · line 49

Quoted 9 lines
drivers/mtd/spi/sf_ops.c +6 -0
@@ -41,6 +41,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;

You can use const u8 cmd = CMD_...

Quoted 13 lines
drivers/mtd/spi/sf_ops.c +9 -0
+	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;

drivers/mtd/spi/sf_ops.c static void spi_flash_dual_flash(struct spi_flash · line 157

Quoted 4 lines
drivers/mtd/spi/sf_ops.c +1 -0
@@ -139,20 +154,53 @@ static void spi_flash_dual_flash(struct spi_flash
 *flash, u32 *addr) }
 #endif
 
+static inline int spi_flash_sr_ready(struct spi_flash *flash)

Drop the bogus inline keyword, compiler will handle this better.

Quoted 20 lines
drivers/mtd/spi/sf_ops.c +20 -0
+{
+	u8 sr;
+	int ret;
+
+	ret = spi_flash_cmd_read_status(flash, &sr);
+	if (ret < 0)
+		return ret;
+
+	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;
+

drivers/mtd/spi/sf_ops.c static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr) · line 187

Quoted 10 lines
drivers/mtd/spi/sf_ops.c +10 -0
+	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;

The ternary operator is just making the code more confusing, don't use it.

Quoted 55 lines
drivers/mtd/spi/sf_ops.c +8 -6
+	if (fsr < 0)
+		return fsr;
+	return sr && fsr;
+}
+
 int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
 timeout) {
-	int timebase, ret, sr;
+	int timebase, ret;
 
 	timebase = get_timer(0);
 
 	while (get_timer(timebase) < timeout) {
-		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;
 	}
drivers/mtd/spi/sf_probe.c +1 -3
index 85390d4..758347b 100644
@@ -255,11 +255,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 */
include/spi_flash.h +0 -1
index 8d85468..4312d3d 100644
@@ -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.

include/spi_flash.h struct spi_flash { · line 86

Quoted 7 lines
include/spi_flash.h +0 -1
@@ -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;

Best regards,
Marek Vasut

author
#14 lore raw

PATCH v3 7/13 sf: Add FSR support to spi_flash_cmd_wait_ready

Quoted 73 lines · Marek Vasut
Quoted 44 lines · Jagan Teki

This patch adds flag status register reading support to spi_flash_cmd_wait_ready.

Signed-off-by Jagan Teki <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Cc Hou Zhiqiang <[email protected]>
Tested-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- Optime code for fsr

 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 60
++++++++++++++++++++++++++++++++++++++----- drivers/mtd/spi/sf_probe.c
|  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 56 insertions(+), 11 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -0
index 53998fc..8a3e5ec 100644
@@ -53,6 +53,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
drivers/mtd/spi/sf_ops.c +0 -0
index c1cdde7..85e15b9 100644

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_read_status(struct spi_flash *flash, · line 49

Quoted 9 lines
drivers/mtd/spi/sf_ops.c +6 -0
@@ -41,6 +41,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;

You can use const u8 cmd = CMD_...

Quoted 16 lines
drivers/mtd/spi/sf_ops.c +9 -0
+     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;

@@ -139,20 +154,53 @@ static void spi_flash_dual_flash(struct spi_flash
 *flash, u32 *addr) }
 #endif
Quoted 3 lines

drivers/mtd/spi/sf_ops.c static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr) · line 157

Quoted 1 line
drivers/mtd/spi/sf_ops.c +1 -0
+static inline int spi_flash_sr_ready(struct spi_flash *flash)

Drop the bogus inline keyword, compiler will handle this better.

Does this means it requires -finline-small-functions optimization flag?

Based on the function-definition-size it will optimize using this flag w/o need of 'inline' keyword, is this the case here?

Quoted 88 lines
Quoted 21 lines
drivers/mtd/spi/sf_ops.c +20 -0
+{
+     u8 sr;
+     int ret;
+
+     ret = spi_flash_cmd_read_status(flash, &sr);
+     if (ret < 0)
+             return ret;
+
+     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;
+

drivers/mtd/spi/sf_ops.c static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr) · line 187

Quoted 10 lines
drivers/mtd/spi/sf_ops.c +10 -0
+     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;

The ternary operator is just making the code more confusing, don't use it.

Quoted 54 lines
drivers/mtd/spi/sf_ops.c +8 -6
+     if (fsr < 0)
+             return fsr;
+     return sr && fsr;
+}
+
 int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long
 timeout) {
-     int timebase, ret, sr;
+     int timebase, ret;
 
      timebase = get_timer(0);
 
      while (get_timer(timebase) < timeout) {
-             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;
      }
drivers/mtd/spi/sf_probe.c +1 -3
index 85390d4..758347b 100644
@@ -255,11 +255,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 */
include/spi_flash.h +0 -1
index 8d85468..4312d3d 100644
@@ -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.
Quoted 8 lines

include/spi_flash.h struct spi_flash { · line 86

Quoted 8 lines
include/spi_flash.h +0 -1
@@ -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;

-- Jagan.

#15 lore raw

PATCH v3 7/13 sf: Add FSR support to spi_flash_cmd_wait_ready

In reply to Jagan Teki
Quoted 14 lines · Jagan Teki

This patch adds flag status register reading support to spi_flash_cmd_wait_ready.

Signed-off-by Jagan Teki <[email protected]>
Cc Simon Glass <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Michal Simek <[email protected]>
Cc Siva Durga Prasad Paladugu <[email protected]>
Cc Stefan Roese <[email protected]>
Cc Tom Warren <[email protected]>
Cc Bin Meng <[email protected]>
Cc Tom Rini <[email protected]>
Cc Hou Zhiqiang <[email protected]>
Tested-by Jagan Teki <[email protected]>
Quoted 1 line

---

Tested on Intel Crown Bay

Tested-by Bin Meng <[email protected]>
Quoted 139 lines

Changes for v3:
- none
Changes for v2:
- Optime code for fsr

 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 60 ++++++++++++++++++++++++++++++++++++++-----
 drivers/mtd/spi/sf_probe.c    |  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 56 insertions(+), 11 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -0
index 53998fc..8a3e5ec 100644
@@ -53,6 +53,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
drivers/mtd/spi/sf_ops.c +54 -6
index c1cdde7..85e15b9 100644
@@ -41,6 +41,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;

@@ -139,20 +154,53 @@ 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;
+
+       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;
+
+       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;
+}
+
 int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
 {
-       int timebase, ret, sr;
+       int timebase, ret;
 
        timebase = get_timer(0);
 
        while (get_timer(timebase) < timeout) {
-               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;
        }
drivers/mtd/spi/sf_probe.c +1 -3
index 85390d4..758347b 100644
@@ -255,11 +255,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 */
include/spi_flash.h +0 -1
index 8d85468..4312d3d 100644
@@ -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.

include/spi_flash.h struct spi_flash {

Quoted 8 lines
include/spi_flash.h +0 -2
@@ -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;
--

Regards,
Bin

author
#16 lore raw

PATCH v3 8/13 sf: Add flash_read_reg support

In reply to Jagan Teki

Generic function for all spi-flash register reads.

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf.c          | 23 +++++++++++++++++++++++
 drivers/mtd/spi/sf_internal.h |  3 +++
 2 files changed, 26 insertions(+)
drivers/mtd/spi/sf.c +23 -0
index 664e860..cb36946 100644
@@ -9,6 +9,7 @@
 
 #include <common.h>
 #include <spi.h>
+#include <spi_flash.h>
 
 static int spi_flash_read_write(struct spi_slave *spi,
 				const u8 *cmd, size_t cmd_len,

@@ -56,3 +57,25 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 {
 	return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
 }
+
+int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len)
+{
+	struct spi_slave *spi = flash->spi;
+	int ret;
+
+	ret = spi_claim_bus(spi);
+	if (ret < 0) {
+		debug("SF: unable to claim SPI bus\n");
+		return ret;
+	}
+
+	ret = spi_flash_read_write(spi, &cmd, 1, NULL, val, len);
+	if (ret < 0) {
+		debug("SF: error %d reading register %x\n", ret, cmd);
+		return ret;
+	}
+
+	spi_release_bus(spi);
+
+	return ret;
+}
drivers/mtd/spi/sf_internal.h +3 -0
index 8a3e5ec..14b8b55 100644
@@ -156,6 +156,9 @@ int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
 int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
 		size_t cmd_len, void *data, size_t data_len);
 
+/* Read flash register - common call for all flash register reads */
+int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len);
+
 /*
  * Send a multi-byte command to the device followed by (optional)
  * data. Used for programming the flash array, etc.
Signature · 1 line
1.9.1
#17 lore raw

PATCH v3 8/13 sf: Add flash_read_reg support

Hi Jagan,

Quoted 26 lines · Jagan Teki

Generic function for all spi-flash register reads.

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf.c          | 23 +++++++++++++++++++++++
 drivers/mtd/spi/sf_internal.h |  3 +++
 2 files changed, 26 insertions(+)
drivers/mtd/spi/sf.c +1 -0
index 664e860..cb36946 100644
@@ -9,6 +9,7 @@
 
 #include <common.h>
 #include <spi.h>
+#include <spi_flash.h>
 
 static int spi_flash_read_write(struct spi_slave *spi,
                                const u8 *cmd, size_t cmd_len,

drivers/mtd/spi/sf.c int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, · line 61

Quoted 5 lines
drivers/mtd/spi/sf.c +2 -0
@@ -56,3 +57,25 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 {
        return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
 }
+
+int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len)

Should it be named as spi_flash_read_reg() to indicate this is for SPI flash only?

Quoted 25 lines
drivers/mtd/spi/sf.c +20 -0
+{
+       struct spi_slave *spi = flash->spi;
+       int ret;
+
+       ret = spi_claim_bus(spi);
+       if (ret < 0) {
+               debug("SF: unable to claim SPI bus\n");
+               return ret;
+       }
+
+       ret = spi_flash_read_write(spi, &cmd, 1, NULL, val, len);
+       if (ret < 0) {
+               debug("SF: error %d reading register %x\n", ret, cmd);
+               return ret;
+       }
+
+       spi_release_bus(spi);
+
+       return ret;
+}
drivers/mtd/spi/sf_internal.h +0 -0
index 8a3e5ec..14b8b55 100644

drivers/mtd/spi/sf_internal.h int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);

Quoted 10 lines
drivers/mtd/spi/sf_internal.h +3 -1
@@ -156,6 +156,9 @@ int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
 int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
                size_t cmd_len, void *data, size_t data_len);
 
+/* Read flash register - common call for all flash register reads */
+int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len);
+
 /*
  * Send a multi-byte command to the device followed by (optional)
  * data. Used for programming the flash array, etc.
--

Regards,
Bin

author
#18 lore raw

PATCH v3 9/13 sf: Use flash_read_reg routine

In reply to Jagan Teki

This patch uses flash_read_reg routine for all respective read register calls.

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_internal.h |  4 +--
 drivers/mtd/spi/sf_ops.c      | 63 +++++++++++++++++++------------------------
 drivers/mtd/spi/sf_probe.c    | 21 +++++++--------
 3 files changed, 39 insertions(+), 49 deletions(-)
drivers/mtd/spi/sf_internal.h +2 -2
index 14b8b55..9341fb3 100644
@@ -171,13 +171,13 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len);
 
 /* Read the status register */
-int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
+int spi_flash_cmd_read_status(struct spi_flash *flash);
 
 /* Program the status register */
 int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
 
 /* Read the config register */
-int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc);
+int spi_flash_cmd_read_config(struct spi_flash *flash);
 
 /* Program the config register */
 int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc);
drivers/mtd/spi/sf_ops.c +28 -35

118 lines not on this page · raw message

drivers/mtd/spi/sf_probe.c +9 -12
index 758347b..262849e 100644
@@ -35,12 +35,11 @@ static u8 spi_read_cmds_array[] = {
 #ifdef CONFIG_SPI_FLASH_MACRONIX
 static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 {
-	u8 qeb_status;
-	int ret;
+	int qeb_status, ret;
 
-	ret = spi_flash_cmd_read_status(flash, &qeb_status);
-	if (ret < 0)
-		return ret;
+	qeb_status = spi_flash_cmd_read_config(flash);
+	if (qeb_status < 0)
+		return qeb_status;
 
 	if (qeb_status & STATUS_QEB_MXIC) {
 		debug("SF: mxic: QEB is already set\n");

@@ -57,12 +56,11 @@ static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
 static int spi_flash_set_qeb_winspan(struct spi_flash *flash)
 {
-	u8 qeb_status;
-	int ret;
+	int qeb_status, ret;
 
-	ret = spi_flash_cmd_read_config(flash, &qeb_status);
-	if (ret < 0)
-		return ret;
+	qeb_status = spi_flash_cmd_read_config(flash);
+	if (qeb_status < 0)
+		return qeb_status;
 
 	if (qeb_status & STATUS_QEB_WINSPAN) {
 		debug("SF: winspan: QEB is already set\n");

@@ -117,8 +115,7 @@ static int spi_flash_read_bank(struct spi_flash *flash, u8 idcode0)
 		flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
 	}
 
-	ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
-				    &curr_bank, 1);
+	ret = flash_read_reg(flash, flash->bank_read_cmd, &curr_bank, 1);
 	if (ret) {
 		debug("SF: fail to read bank addr register\n");
 		return ret;
Signature · 1 line
1.9.1
author
#19 lore raw

PATCH v3 10/13 sf: Add flash_read support

In reply to Jagan Teki

Generic function for spi-flash memory reading.

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf.c          | 23 +++++++++++++++++++++++
 drivers/mtd/spi/sf_internal.h |  3 +++
 2 files changed, 26 insertions(+)
drivers/mtd/spi/sf.c +23 -0
index cb36946..2670207 100644
@@ -79,3 +79,26 @@ int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len)
 
 	return ret;
 }
+
+int flash_read(struct spi_flash *flash, const u8 *cmd, size_t cmd_len,
+				void *data, size_t data_len)
+{
+	struct spi_slave *spi = flash->spi;
+	int ret;
+
+	ret = spi_claim_bus(spi);
+	if (ret < 0) {
+		debug("SF: unable to claim SPI bus\n");
+		return ret;
+	}
+
+	ret = spi_flash_read_write(spi, cmd, cmd_len, NULL, data, data_len);
+	if (ret < 0) {
+		debug("SF: error %d reading %x\n", ret, *cmd);
+		return ret;
+	}
+
+	spi_release_bus(spi);
+
+	return ret;
+}
drivers/mtd/spi/sf_internal.h +3 -0
index 9341fb3..4dfc67d 100644
@@ -159,6 +159,9 @@ int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
 /* Read flash register - common call for all flash register reads */
 int flash_read_reg(struct spi_flash *flash, u8 cmd, u8 *val, int len);
 
+/* Read flash memory - Read data from spi flash */
+int flash_read(struct spi_flash *flash, const u8 *cmd, size_t cmd_len,
+				void *data, size_t data_len);
 /*
  * Send a multi-byte command to the device followed by (optional)
  * data. Used for programming the flash array, etc.
Signature · 1 line
1.9.1
author
#20 lore raw

PATCH v3 11/13 sf: Use flash_read routine in spi_flash_read_ops

In reply to Jagan Teki

Use flash_read directly instead of spi_flash_read_common.

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_internal.h |  7 -------
 drivers/mtd/spi/sf_ops.c      | 25 +------------------------
 2 files changed, 1 insertion(+), 31 deletions(-)
drivers/mtd/spi/sf_internal.h +0 -7
index 4dfc67d..3304217 100644
@@ -222,13 +222,6 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
 int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		size_t len, const void *buf);
 
-/*
- * Same as spi_flash_cmd_read() except it also claims/releases the SPI
- * bus. Used as common part of the ->read() operation.
- */
-int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, void *data, size_t data_len);
-
 /* Flash read operation, support all possible read commands */
 int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		size_t len, void *data);
drivers/mtd/spi/sf_ops.c +1 -24
index 4c807ed..ae4a937 100644
@@ -335,29 +335,6 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 	return ret;
 }
 
-int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, void *data, size_t data_len)
-{
-	struct spi_slave *spi = flash->spi;
-	int ret;
-
-	ret = spi_claim_bus(flash->spi);
-	if (ret) {
-		debug("SF: unable to claim SPI bus\n");
-		return ret;
-	}
-
-	ret = spi_flash_cmd_read(spi, cmd, cmd_len, data, data_len);
-	if (ret < 0) {
-		debug("SF: read cmd failed\n");
-		return ret;
-	}
-
-	spi_release_bus(spi);
-
-	return ret;
-}
-
 void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
 {
 	memcpy(data, offset, len);

@@ -415,7 +392,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 
 		spi_flash_addr(read_addr, cmd);
 
-		ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
+		ret = flash_read(flash, cmd, cmdsz, data, read_len);
 		if (ret < 0) {
 			debug("SF: read failed\n");
 			break;
Signature · 1 line
1.9.1
author
#21 lore raw

PATCH v3 12/13 sf: Add clear flag status register support

In reply to Jagan Teki

From: Hou Zhiqiang <[email protected]>

The clear flag status register operation is required by Micron SPI-NOR chips, which support FSR. And if an error bit of FSR have been set like protection, voltage, erase, and program, it must be cleared by the clear FSR operation.

Signed-off-by Hou Zhiqiang <[email protected]>
Signed-off-by Mingkai.Hu <[email protected]>
Signed-off-by Jagan Teki <[email protected]>
Reviewed-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_internal.h |  7 +++++++
 drivers/mtd/spi/sf_ops.c      | 18 ++++++++++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/mtd/spi/sf_internal.h +7 -0
index 3304217..e4cb198 100644
@@ -81,6 +81,7 @@ enum spi_nor_option_flags {
 #define CMD_READ_STATUS1		0x35
 #define CMD_WRITE_ENABLE		0x06
 #define CMD_READ_CONFIG			0x35
+#define CMD_CLEAR_FLAG_STATUS		0x50
 #define CMD_FLAG_STATUS			0x70
 
 /* Read commands */

@@ -104,7 +105,13 @@ enum spi_nor_option_flags {
 #define STATUS_WIP			(1 << 0)
 #define STATUS_QEB_WINSPAN		(1 << 1)
 #define STATUS_QEB_MXIC		(1 << 6)
+#define STATUS_ERR_PROT			(1 << 1)
+#define STATUS_ERR_VOLT			(1 << 3)
+#define STATUS_ERR_PROG			(1 << 4)
+#define STATUS_ERR_ERASE		(1 << 5)
 #define STATUS_PEC			(1 << 7)
+#define FSR_ERR_MSK			(STATUS_ERR_PROT | STATUS_ERR_VOLT | \
+					STATUS_ERR_PROG | STATUS_ERR_ERASE)
 
 /* Flash timeout values */
 #define SPI_FLASH_PROG_TIMEOUT		(2 * CONFIG_SYS_HZ)
drivers/mtd/spi/sf_ops.c +14 -4
index ae4a937..77b053a 100644
@@ -153,6 +153,11 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
 }
 #endif
 
+static inline int write_cfsr(struct spi_flash *flash)
+{
+	return spi_flash_cmd(flash->spi, CMD_CLEAR_FLAG_STATUS, NULL, 0);
+}
+
 static inline int spi_flash_sr_ready(struct spi_flash *flash)
 {
 	int sr = spi_flash_cmd_read_status(flash);

@@ -165,10 +170,15 @@ static inline int spi_flash_sr_ready(struct spi_flash *flash)
 static inline int spi_flash_fsr_ready(struct spi_flash *flash)
 {
 	int fsr = read_fsr(flash);
-	if (fsr < 0)
-		return fsr;
-	else
-		return fsr & STATUS_PEC;
+	if (fsr & FSR_ERR_MSK) {
+		printf("SF: flag status(0x%x) error occured\n", fsr);
+		int cfsr = write_cfsr(flash);
+		if (cfsr < 0)
+			printf("SF: clear flag status failed\n");
+		return -EINVAL;
+	}
+
+	return fsr & STATUS_PEC;
 }
 
 static int spi_flash_ready(struct spi_flash *flash)
Signature · 1 line
1.9.1
author
#22 lore raw

PATCH v3 13/13 sf: Rename read register functions

In reply to Jagan Teki

spi_flash_cmd_read_status -> read_sr
spi_flash_cmd_read_config -> read_cr

Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_internal.h | 5 +----
 drivers/mtd/spi/sf_ops.c      | 8 ++++----
 drivers/mtd/spi/sf_probe.c    | 4 ++--
 3 files changed, 7 insertions(+), 10 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -4
index e4cb198..e618306 100644
@@ -180,14 +180,11 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 /* Flash erase(sectors) operation, support all possible erase commands */
 int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len);
 
-/* Read the status register */
-int spi_flash_cmd_read_status(struct spi_flash *flash);
-
 /* Program the status register */
 int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
 
 /* Read the config register */
-int spi_flash_cmd_read_config(struct spi_flash *flash);
+int read_cr(struct spi_flash *flash);
 
 /* Program the config register */
 int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc);
drivers/mtd/spi/sf_ops.c +4 -4
index 77b053a..a27beb5 100644
@@ -26,7 +26,7 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
 	cmd[3] = addr >> 0;
 }
 
-int spi_flash_cmd_read_status(struct spi_flash *flash)
+int read_sr(struct spi_flash *flash)
 {
 	int ret;
 	u8 rs;

@@ -70,7 +70,7 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
 }
 
 #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
-int spi_flash_cmd_read_config(struct spi_flash *flash)
+int read_cr(struct spi_flash *flash)
 {
 	int ret;
 	u8 rc;

@@ -90,7 +90,7 @@ int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
 	u8 cmd;
 	int ret, val;
 
-	val = spi_flash_cmd_read_status(flash);
+	val = read_sr(flash);
 	if (val < 0)
 		return val;
 

@@ -160,7 +160,7 @@ static inline int write_cfsr(struct spi_flash *flash)
 
 static inline int spi_flash_sr_ready(struct spi_flash *flash)
 {
-	int sr = spi_flash_cmd_read_status(flash);
+	int sr = read_sr(flash);
 	if (sr < 0)
 		return sr;
 	else
drivers/mtd/spi/sf_probe.c +2 -2
index 262849e..bf84f06 100644
@@ -37,7 +37,7 @@ static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 {
 	int qeb_status, ret;
 
-	qeb_status = spi_flash_cmd_read_config(flash);
+	qeb_status = read_cr(flash);
 	if (qeb_status < 0)
 		return qeb_status;
 

@@ -58,7 +58,7 @@ static int spi_flash_set_qeb_winspan(struct spi_flash *flash)
 {
 	int qeb_status, ret;
 
-	qeb_status = spi_flash_cmd_read_config(flash);
+	qeb_status = read_cr(flash);
 	if (qeb_status < 0)
 		return qeb_status;
Signature · 1 line
1.9.1
#23 lore raw

PATCH v3 13/13 sf: Rename read register functions

Hi Jagan,

Quoted 2 lines · Jagan Teki

spi_flash_cmd_read_status -> read_sr
spi_flash_cmd_read_config -> read_cr

Can you elaborate why this rename is needed? To me, read_sr() and read_cr() are too generic names, and they are not static neither.

Quoted 87 lines
Signed-off-by Jagan Teki <[email protected]>

---
Changes for v3:
- none
Changes for v2:
- none

 drivers/mtd/spi/sf_internal.h | 5 +----
 drivers/mtd/spi/sf_ops.c      | 8 ++++----
 drivers/mtd/spi/sf_probe.c    | 4 ++--
 3 files changed, 7 insertions(+), 10 deletions(-)
drivers/mtd/spi/sf_internal.h +1 -4
index e4cb198..e618306 100644
@@ -180,14 +180,11 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 /* Flash erase(sectors) operation, support all possible erase commands */
 int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len);
 
-/* Read the status register */
-int spi_flash_cmd_read_status(struct spi_flash *flash);
-
 /* Program the status register */
 int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
 
 /* Read the config register */
-int spi_flash_cmd_read_config(struct spi_flash *flash);
+int read_cr(struct spi_flash *flash);
 
 /* Program the config register */
 int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc);
drivers/mtd/spi/sf_ops.c +4 -4
index 77b053a..a27beb5 100644
@@ -26,7 +26,7 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
        cmd[3] = addr >> 0;
 }
 
-int spi_flash_cmd_read_status(struct spi_flash *flash)
+int read_sr(struct spi_flash *flash)
 {
        int ret;
        u8 rs;

@@ -70,7 +70,7 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
 }
 
 #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
-int spi_flash_cmd_read_config(struct spi_flash *flash)
+int read_cr(struct spi_flash *flash)
 {
        int ret;
        u8 rc;

@@ -90,7 +90,7 @@ int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
        u8 cmd;
        int ret, val;
 
-       val = spi_flash_cmd_read_status(flash);
+       val = read_sr(flash);
        if (val < 0)
                return val;
 

@@ -160,7 +160,7 @@ static inline int write_cfsr(struct spi_flash *flash)
 
 static inline int spi_flash_sr_ready(struct spi_flash *flash)
 {
-       int sr = spi_flash_cmd_read_status(flash);
+       int sr = read_sr(flash);
        if (sr < 0)
                return sr;
        else
drivers/mtd/spi/sf_probe.c +1 -1
index 262849e..bf84f06 100644
@@ -37,7 +37,7 @@ static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 {
        int qeb_status, ret;
 
-       qeb_status = spi_flash_cmd_read_config(flash);
+       qeb_status = read_cr(flash);
        if (qeb_status < 0)
                return qeb_status;

drivers/mtd/spi/sf_probe.c static int spi_flash_set_qeb_winspan(struct spi_flash *flash)

Quoted 9 lines
drivers/mtd/spi/sf_probe.c +1 -2
@@ -58,7 +58,7 @@ static int spi_flash_set_qeb_winspan(struct spi_flash *flash)
 {
        int qeb_status, ret;
 
-       qeb_status = spi_flash_cmd_read_config(flash);
+       qeb_status = read_cr(flash);
        if (qeb_status < 0)
                return qeb_status;
 
--

Regards,
Bin