[U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer

public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
@ 2015-10-12 15:00 Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers Jagan Teki
                   ` (21 more replies)
  0 siblings, 22 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

Previous version link:
http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262

spi-flash layer need to tune a lot for better code handling and
to sync with Linux spi-nor. So below areas got updated in this series.
- BAR handling
- spi_flash_cmd_wait_ready updates.
- Separate core spi-flash handling and spi-flash interface
  (interface between spi drivers vs spi-flash layer)

Currently I'm working on spi-nor framework for u-boot which
is slighly same as Linux spi-nor core with addition of
u-boot driver model to it.

This series will be starting point to add spi-nor functionalities.

TODO:
- MTD core addition to spi-flash layer.
- spi-nor core addition.

Code sizes:
After:
dm:
   text	   data	    bss	    dec	    hex	filename
 354820	  12016	 221112	 587948	  8f8ac	u-boot
non-dm
   text	   data	    bss	    dec	    hex	filename
 354317	  11876	 221124	 587317	  8f635	u-boot

Before:
dm
   text	   data	    bss	    dec	    hex	filename
 354878	  12016	 221096	 587990	  8f8d6	u-boot
non-dm
   text	   data	    bss	    dec	    hex	filename
 354447	  11876	 221124	 587447	  8f6b7	u-boot

Testing:
$ git clone git://git.denx.de/u-boot-spi.git
$ cd u-boot-spi
$ git checkout -b spi-nor-tune origin/next-spi-nor-tune

thanks!
Jagan.

Jagan Teki (21):
  spi: zynq_spi: Remove unneeded headers
  sf: Return bank_sel, if flash->bank_curr == bank_sel
  sf: Add 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: spi_flash_validate_params => spi_flash_scan
  sf: Move spi_flash_scan code to sf_ops
  sf: Move the read_id code to sf_ops
  sf: Move BAR defined code at once place
  sf: Use static for file-scope functions
  sf: Fix Makefile
  sf: Use simple name for register access functions
  sf: Use flash function pointers in dm_spi_flash_ops
  sf: Flash power up read-only based on idcode0
  sf: Use static for file-scope functions
  sf: Remove unneeded header includes
  sf: probe: Use spi_flash_scan in dm-spi-flash
  sf: Re-factorize spi_flash_probe_tail code
  dm-sf: Re-factorize spi_flash_std_probe code

 drivers/mtd/spi/Makefile      |   6 +-
 drivers/mtd/spi/sf_internal.h |  57 ++---
 drivers/mtd/spi/sf_ops.c      | 488 +++++++++++++++++++++++++++++++++++-------
 drivers/mtd/spi/sf_probe.c    | 446 ++++++--------------------------------
 drivers/spi/zynq_spi.c        |   6 +-
 include/spi_flash.h           |  19 +-
 6 files changed, 494 insertions(+), 528 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:32   ` Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel Jagan Teki
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

- 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>
---
Changes for v4, v3, v2:
	- none

 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 65a9633..33a13bf 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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers Jagan Teki
@ 2015-10-25  7:32   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:32 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, Jagan Teki <jteki@openedev.com> wrote:
> - 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>

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:33   ` Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar Jagan Teki
                   ` (19 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

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>
---
Changes for v4, v3, v2:
	- none

 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 900ec1f..b33fe5a 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -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;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel Jagan Teki
@ 2015-10-25  7:33   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:33 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, Jagan Teki <jteki@openedev.com> wrote:
> 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>

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:35   ` Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code Jagan Teki
                   ` (18 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

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>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_probe.c | 53 +++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 954376d..f17ec17 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)
 {
@@ -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 */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar Jagan Teki
@ 2015-10-25  7:35   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:35 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, 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>
> ---

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (2 preceding siblings ...)
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:36   ` Jagan Teki
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage Jagan Teki
                   ` (17 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

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>
---
Changes for v4, v3, v2:
	- none

 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 b33fe5a..6e457ec 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -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;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code Jagan Teki
@ 2015-10-25  7:36   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:36 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, Jagan Teki <jteki@openedev.com> wrote:
> 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>
> ---

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (3 preceding siblings ...)
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:38   ` Jagan Teki
  2015-11-16  2:59   ` Bin Meng
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready Jagan Teki
                   ` (16 subsequent siblings)
  21 siblings, 2 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

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>
[Correct the spi flash flags detect logic]
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes for v4:
	- Fixed SNOR_F_SST_WR
Changes for v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h |  4 ++++
 drivers/mtd/spi/sf_probe.c    | 10 +++++-----
 include/spi_flash.h           |  4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 9c95d56..53998fc 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -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
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index f17ec17..2634e90 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -163,15 +163,15 @@ 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) {
+	if (params->flags & SST_WR)
+		flash->flags |= SNOR_F_SST_WR;
+
+	if (params->flags & SNOR_F_SST_WR) {
 		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
 			flash->write = sst_write_bp;
 		else
@@ -466,7 +466,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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage Jagan Teki
@ 2015-10-25  7:38   ` Jagan Teki
  2015-11-16  2:59   ` Bin Meng
  1 sibling, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:38 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, 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>
> [Correct the spi flash flags detect logic]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> ---

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage Jagan Teki
  2015-10-25  7:38   ` Jagan Teki
@ 2015-11-16  2:59   ` Bin Meng
  2015-11-18  1:14     ` Bin Meng
  2015-11-18  6:58     ` Jagan Teki
  1 sibling, 2 replies; 41+ messages in thread
From: Bin Meng @ 2015-11-16  2:59 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On Mon, Oct 12, 2015 at 11:00 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>
> [Correct the spi flash flags detect logic]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> Changes for v4:
>         - Fixed SNOR_F_SST_WR
> Changes for v3, v2:
>         - none
>

It turns out this patch breaks the Intel Crown Bay SPI flash. I
compared my original submitted patch with this patch you applied, and
surprisingly found there are differences ...

My version is http://patchwork.ozlabs.org/patch/517704/
You version below seems to modify some places which you thought would
be correct, but that's unfortunately wrong. If you reworked my patch,
I think you should remove at least my "Tested-by:" tag and ask me to
retest.

Can you please help me understand what happened? I expected that you
should just grab my version and include it in your patch series.

>  drivers/mtd/spi/sf_internal.h |  4 ++++
>  drivers/mtd/spi/sf_probe.c    | 10 +++++-----
>  include/spi_flash.h           |  4 ++--
>  3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 9c95d56..53998fc 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -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
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index f17ec17..2634e90 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -163,15 +163,15 @@ 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) {
> +       if (params->flags & SST_WR)
> +               flash->flags |= SNOR_F_SST_WR;
> +
> +       if (params->flags & SNOR_F_SST_WR) {
>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>                         flash->write = sst_write_bp;
>                 else
> @@ -466,7 +466,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;
> --

Regards,
Bin

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-11-16  2:59   ` Bin Meng
@ 2015-11-18  1:14     ` Bin Meng
  2015-11-18  1:27       ` Tom Rini
  2015-11-18  6:58     ` Jagan Teki
  1 sibling, 1 reply; 41+ messages in thread
From: Bin Meng @ 2015-11-18  1:14 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On Mon, Nov 16, 2015 at 10:59 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Oct 12, 2015 at 11:00 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>
>> [Correct the spi flash flags detect logic]
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>> Changes for v4:
>>         - Fixed SNOR_F_SST_WR
>> Changes for v3, v2:
>>         - none
>>
>
> It turns out this patch breaks the Intel Crown Bay SPI flash. I
> compared my original submitted patch with this patch you applied, and
> surprisingly found there are differences ...
>
> My version is http://patchwork.ozlabs.org/patch/517704/
> You version below seems to modify some places which you thought would
> be correct, but that's unfortunately wrong. If you reworked my patch,
> I think you should remove at least my "Tested-by:" tag and ask me to
> retest.
>
> Can you please help me understand what happened? I expected that you
> should just grab my version and include it in your patch series.
>

I see you were replying yesterday, but did not see any response on this one.

[snip]

Regards,
Bin

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-11-18  1:14     ` Bin Meng
@ 2015-11-18  1:27       ` Tom Rini
  0 siblings, 0 replies; 41+ messages in thread
From: Tom Rini @ 2015-11-18  1:27 UTC (permalink / raw)
  To: u-boot

On Wed, Nov 18, 2015 at 09:14:01AM +0800, Bin Meng wrote:
> Hi Jagan,
> 
> On Mon, Nov 16, 2015 at 10:59 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> > Hi Jagan,
> >
> > On Mon, Oct 12, 2015 at 11:00 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>
> >> [Correct the spi flash flags detect logic]
> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> >> ---
> >> Changes for v4:
> >>         - Fixed SNOR_F_SST_WR
> >> Changes for v3, v2:
> >>         - none
> >>
> >
> > It turns out this patch breaks the Intel Crown Bay SPI flash. I
> > compared my original submitted patch with this patch you applied, and
> > surprisingly found there are differences ...
> >
> > My version is http://patchwork.ozlabs.org/patch/517704/
> > You version below seems to modify some places which you thought would
> > be correct, but that's unfortunately wrong. If you reworked my patch,
> > I think you should remove at least my "Tested-by:" tag and ask me to
> > retest.
> >
> > Can you please help me understand what happened? I expected that you
> > should just grab my version and include it in your patch series.
> >
> 
> I see you were replying yesterday, but did not see any response on this one.

Modifying patches between the mailing list and applying, outside of
making things apply again or extremely obvious fixes is really really
frowned upon.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151117/84611084/attachment.sig>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage
  2015-11-16  2:59   ` Bin Meng
  2015-11-18  1:14     ` Bin Meng
@ 2015-11-18  6:58     ` Jagan Teki
  1 sibling, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-11-18  6:58 UTC (permalink / raw)
  To: u-boot

On 16 November 2015 at 08:29, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Mon, Oct 12, 2015 at 11:00 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>
>> [Correct the spi flash flags detect logic]
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>> Changes for v4:
>>         - Fixed SNOR_F_SST_WR
>> Changes for v3, v2:
>>         - none
>>
>
> It turns out this patch breaks the Intel Crown Bay SPI flash. I
> compared my original submitted patch with this patch you applied, and
> surprisingly found there are differences ...
>
> My version is http://patchwork.ozlabs.org/patch/517704/
> You version below seems to modify some places which you thought would
> be correct, but that's unfortunately wrong. If you reworked my patch,
> I think you should remove at least my "Tested-by:" tag and ask me to
> retest.
>
> Can you please help me understand what happened? I expected that you
> should just grab my version and include it in your patch series.

I have moved flash->flags |= SNOR_F_SST_WR; assignment to #ifdef *_SST
since it's been part of SST flash, but seems like dm will require that
flags to call the respective sst write ops - it's a mistake. I
thought you may comment the same since I CCed you. I will grab the fix
patch you sent.

>
>>  drivers/mtd/spi/sf_internal.h |  4 ++++
>>  drivers/mtd/spi/sf_probe.c    | 10 +++++-----
>>  include/spi_flash.h           |  4 ++--
>>  3 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 9c95d56..53998fc 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -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
>> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
>> index f17ec17..2634e90 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -163,15 +163,15 @@ 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) {
>> +       if (params->flags & SST_WR)
>> +               flash->flags |= SNOR_F_SST_WR;
>> +
>> +       if (params->flags & SNOR_F_SST_WR) {
>>                 if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
>>                         flash->write = sst_write_bp;
>>                 else
>> @@ -466,7 +466,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;
>> --

thanks!
-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (4 preceding siblings ...)
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage Jagan Teki
@ 2015-10-12 15:00 ` Jagan Teki
  2015-10-25  7:39   ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready Jagan Teki
                   ` (15 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:00 UTC (permalink / raw)
  To: u-boot

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 <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: Tom Rini <trini@konsulko.com>
Tested-by: Jagan Teki <jteki@openedev.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes for v4:
	- none
Changes for v3:
	- Fixed comments from Marek
Changes for v2:
	- none

 drivers/mtd/spi/sf_ops.c | 71 +++++++++---------------------------------------
 1 file changed, 13 insertions(+), 58 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 6e457ec..c820d48 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -139,72 +139,27 @@ 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;
+	u8 sr;
+	int timebase, ret;
 
-	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 & STATUS_WIP))
+			return 0;
+		else
+			break;
 	}
 
-	return 0;
+	printf("SF: Timeout!\n");
+
+	return -ETIMEDOUT;
 }
 
 int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready Jagan Teki
@ 2015-10-25  7:39   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:39 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:30, Jagan Teki <jteki@openedev.com> wrote:
> 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 <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: Tom Rini <trini@konsulko.com>
> Tested-by: Jagan Teki <jteki@openedev.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> ---

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (5 preceding siblings ...)
  2015-10-12 15:00 ` [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-25  7:40   ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 08/21] sf: spi_flash_validate_params => spi_flash_scan Jagan Teki
                   ` (14 subsequent siblings)
  21 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

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: Tom Rini <trini@konsulko.com>
Cc: Hou Zhiqiang <B48286@freescale.com>
Tested-by: Jagan Teki <jteki@openedev.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h |  1 +
 drivers/mtd/spi/sf_ops.c      | 64 +++++++++++++++++++++++++++++++++++++++----
 drivers/mtd/spi/sf_probe.c    |  4 +--
 include/spi_flash.h           |  2 --
 4 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 53998fc..8a3e5ec 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -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
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index c820d48..f2a9244 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -41,6 +41,20 @@ 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;
+	const u8 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,22 +153,60 @@ static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
 }
 #endif
 
-int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
+static 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 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 = 1;
+	if (flash->flags & SNOR_F_USE_FSR) {
+		fsr = spi_flash_fsr_ready(flash);
+		if (fsr < 0)
+			return fsr;
+	}
+
+	return sr && fsr;
+}
+
+int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
+{
 	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 & STATUS_WIP))
+		if (ret)
 			return 0;
-		else
-			break;
 	}
 
 	printf("SF: Timeout!\n");
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 2634e90..f591ab1 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -257,11 +257,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

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready Jagan Teki
@ 2015-10-25  7:40   ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-25  7:40 UTC (permalink / raw)
  To: u-boot

On 12 October 2015 at 20:31, Jagan Teki <jteki@openedev.com> 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: Tom Rini <trini@konsulko.com>
> Cc: Hou Zhiqiang <B48286@freescale.com>
> Tested-by: Jagan Teki <jteki@openedev.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> ---

Applied to u-boot-spi/master

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 08/21] sf: spi_flash_validate_params => spi_flash_scan
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (6 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 09/21] sf: Move spi_flash_scan code to sf_ops Jagan Teki
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Rename spi_flash_validate_params to spi_flash_scan as this
code not only deals with params setup but also configure
all spi_flash attributes. And also moved all flash related
code into spi_flash_scan for future functionality addition.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_probe.c | 145 +++++++++++++++++++++++----------------------
 1 file changed, 75 insertions(+), 70 deletions(-)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index f591ab1..a150263 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -130,13 +130,42 @@ bank_end:
 }
 #endif
 
-static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+	fdt_addr_t addr;
+	fdt_size_t size;
+	int node;
+
+	/* If there is no node, do nothing */
+	node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+	if (node < 0)
+		return 0;
+
+	addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+	if (addr == FDT_ADDR_T_NONE) {
+		debug("%s: Cannot decode address\n", __func__);
+		return 0;
+	}
+
+	if (flash->size != size) {
+		debug("%s: Memory map must cover entire device\n", __func__);
+		return -1;
+	}
+	flash->memory_map = map_sysmem(addr, size);
+
+	return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
 				     struct spi_flash *flash)
 {
 	const struct spi_flash_params *params;
 	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;
@@ -158,6 +187,13 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 		return -EPROTONOSUPPORT;
 	}
 
+	/* Flash powers up read-only, so clear BP# bits */
+#if defined(CONFIG_SPI_FLASH_ATMEL) || \
+	defined(CONFIG_SPI_FLASH_MACRONIX) || \
+	defined(CONFIG_SPI_FLASH_SST)
+		spi_flash_cmd_write_status(flash, 0);
+#endif
+
 	/* Assign spi data */
 	flash->spi = spi;
 	flash->name = params->name;
@@ -238,6 +274,17 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 		/* Go for default supported write cmd */
 		flash->write_cmd = CMD_PAGE_PROGRAM;
 
+	/* Set the quad enable bit - only for quad commands */
+	if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
+	    (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
+	    (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
+		ret = spi_flash_set_qeb(flash, idcode[0]);
+		if (ret) {
+			debug("SF: Fail to set QEB for %02x\n", idcode[0]);
+			return -EINVAL;
+		}
+	}
+
 	/* Read dummy_byte: dummy byte is determined based on the
 	 * dummy cycles of a particular command.
 	 * Fast commands - dummy_byte = dummy_cycles/8
@@ -264,48 +311,41 @@ 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
-	int ret = spi_flash_read_bank(flash, idcode[0]);
+	ret = spi_flash_read_bank(flash, idcode[0]);
 	if (ret < 0)
 		return ret;
 #endif
 
-	/* Flash powers up read-only, so clear BP# bits */
-#if defined(CONFIG_SPI_FLASH_ATMEL) || \
-	defined(CONFIG_SPI_FLASH_MACRONIX) || \
-	defined(CONFIG_SPI_FLASH_SST)
-		spi_flash_cmd_write_status(flash, 0);
-#endif
-
-	return 0;
-}
-
 #if CONFIG_IS_ENABLED(OF_CONTROL)
-int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
-{
-	fdt_addr_t addr;
-	fdt_size_t size;
-	int node;
-
-	/* If there is no node, do nothing */
-	node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
-	if (node < 0)
-		return 0;
-
-	addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
-	if (addr == FDT_ADDR_T_NONE) {
-		debug("%s: Cannot decode address\n", __func__);
-		return 0;
+	ret = spi_flash_decode_fdt(gd->fdt_blob, flash);
+	if (ret) {
+		debug("SF: FDT decode error\n");
+		return -EINVAL;
 	}
+#endif
 
-	if (flash->size != size) {
-		debug("%s: Memory map must cover entire device\n", __func__);
-		return -1;
+#ifndef CONFIG_SPL_BUILD
+	printf("SF: Detected %s with page size ", flash->name);
+	print_size(flash->page_size, ", erase size ");
+	print_size(flash->erase_size, ", total ");
+	print_size(flash->size, "");
+	if (flash->memory_map)
+		printf(", mapped at %p", flash->memory_map);
+	puts("\n");
+#endif
+
+#ifndef CONFIG_SPI_FLASH_BAR
+	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
+	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
+	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+		puts("SF: Warning - Only lower 16MiB accessible,");
+		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
 	}
-	flash->memory_map = map_sysmem(addr, size);
+#endif
 
-	return 0;
+	return ret;
 }
-#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
 
 /**
  * spi_flash_probe_slave() - Probe for a SPI flash device on a bus
@@ -344,47 +384,12 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 	print_buffer(0, idcode, 1, sizeof(idcode), 0);
 #endif
 
-	if (spi_flash_validate_params(spi, idcode, flash)) {
+	ret = spi_flash_scan(spi, idcode, flash);
+	if (ret) {
 		ret = -EINVAL;
 		goto err_read_id;
 	}
 
-	/* Set the quad enable bit - only for quad commands */
-	if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
-	    (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
-	    (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
-		if (spi_flash_set_qeb(flash, idcode[0])) {
-			debug("SF: Fail to set QEB for %02x\n", idcode[0]);
-			ret = -EINVAL;
-			goto err_read_id;
-		}
-	}
-
-#if CONFIG_IS_ENABLED(OF_CONTROL)
-	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
-		debug("SF: FDT decode error\n");
-		ret = -EINVAL;
-		goto err_read_id;
-	}
-#endif
-#ifndef CONFIG_SPL_BUILD
-	printf("SF: Detected %s with page size ", flash->name);
-	print_size(flash->page_size, ", erase size ");
-	print_size(flash->erase_size, ", total ");
-	print_size(flash->size, "");
-	if (flash->memory_map)
-		printf(", mapped at %p", flash->memory_map);
-	puts("\n");
-#endif
-#ifndef CONFIG_SPI_FLASH_BAR
-	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
-	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
-		puts("SF: Warning - Only lower 16MiB accessible,");
-		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
-	}
-#endif
 #ifdef CONFIG_SPI_FLASH_MTD
 	ret = spi_flash_mtd_register(flash);
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 09/21] sf: Move spi_flash_scan code to sf_ops
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (7 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 08/21] sf: spi_flash_validate_params => spi_flash_scan Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 10/21] sf: Move the read_id " Jagan Teki
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Intension is that sf_ops should deals all spi_flash related
stuff and sf_probe(which should renamed future) should be an
interface layer for spi_flash versus spi drivers.

sf_ops => spi_flash interface
sf_probe => interface layer vs spi_flash(sf_probe) to spi drivers

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h |  14 ++
 drivers/mtd/spi/sf_ops.c      | 328 ++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi/sf_probe.c    | 327 -----------------------------------------
 3 files changed, 342 insertions(+), 327 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 8a3e5ec..3f07560 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -232,4 +232,18 @@ int spi_flash_mtd_register(struct spi_flash *flash);
 void spi_flash_mtd_unregister(void);
 #endif
 
+/**
+ * spi_flash_scan - scan the SPI FLASH
+ * @spi:	the spi slave structure
+ * @idcode:	idcode of spi flash
+ * @flash:	the spi flash structure
+ *
+ * The drivers can use this fuction to scan the SPI FLASH.
+ * In the scanning, it will try to get all the necessary information to
+ * fill the spi_flash{}.
+ *
+ * Return: 0 for success, others for failure.
+ */
+int spi_flash_scan(struct spi_slave *spi, u8 *idcode, struct spi_flash *flash);
+
 #endif /* _SF_INTERNAL_H_ */
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index f2a9244..d37b0a9 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -11,6 +11,7 @@
 #include <common.h>
 #include <errno.h>
 #include <malloc.h>
+#include <mapmem.h>
 #include <spi.h>
 #include <spi_flash.h>
 #include <watchdog.h>
@@ -18,6 +19,8 @@
 
 #include "sf_internal.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static void spi_flash_addr(u32 addr, u8 *cmd)
 {
 	/* cmd[0] is actual command */
@@ -565,3 +568,328 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
 	return ret;
 }
 #endif
+
+
+/* Read commands array */
+static u8 spi_read_cmds_array[] = {
+	CMD_READ_ARRAY_SLOW,
+	CMD_READ_ARRAY_FAST,
+	CMD_READ_DUAL_OUTPUT_FAST,
+	CMD_READ_DUAL_IO_FAST,
+	CMD_READ_QUAD_OUTPUT_FAST,
+	CMD_READ_QUAD_IO_FAST,
+};
+
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
+{
+	u8 qeb_status;
+	int ret;
+
+	ret = spi_flash_cmd_read_status(flash, &qeb_status);
+	if (ret < 0)
+		return ret;
+
+	if (qeb_status & STATUS_QEB_MXIC) {
+		debug("SF: mxic: QEB is already set\n");
+	} else {
+		ret = spi_flash_cmd_write_status(flash, STATUS_QEB_MXIC);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+#endif
+
+#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;
+
+	ret = spi_flash_cmd_read_config(flash, &qeb_status);
+	if (ret < 0)
+		return ret;
+
+	if (qeb_status & STATUS_QEB_WINSPAN) {
+		debug("SF: winspan: QEB is already set\n");
+	} else {
+		ret = spi_flash_cmd_write_config(flash, STATUS_QEB_WINSPAN);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+#endif
+
+static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
+{
+	switch (idcode0) {
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+	case SPI_FLASH_CFI_MFR_MACRONIX:
+		return spi_flash_set_qeb_mxic(flash);
+#endif
+#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
+	case SPI_FLASH_CFI_MFR_SPANSION:
+	case SPI_FLASH_CFI_MFR_WINBOND:
+		return spi_flash_set_qeb_winspan(flash);
+#endif
+#ifdef CONFIG_SPI_FLASH_STMICRO
+	case SPI_FLASH_CFI_MFR_STMICRO:
+		debug("SF: QEB is volatile for %02x flash\n", idcode0);
+		return 0;
+#endif
+	default:
+		printf("SF: Need set QEB func for %02x flash\n", idcode0);
+		return -1;
+	}
+}
+
+#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
+
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
+{
+	fdt_addr_t addr;
+	fdt_size_t size;
+	int node;
+
+	/* If there is no node, do nothing */
+	node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
+	if (node < 0)
+		return 0;
+
+	addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
+	if (addr == FDT_ADDR_T_NONE) {
+		debug("%s: Cannot decode address\n", __func__);
+		return 0;
+	}
+
+	if (flash->size != size) {
+		debug("%s: Memory map must cover entire device\n", __func__);
+		return -1;
+	}
+	flash->memory_map = map_sysmem(addr, size);
+
+	return 0;
+}
+#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
+
+int spi_flash_scan(struct spi_slave *spi, u8 *idcode, struct spi_flash *flash)
+{
+	const struct spi_flash_params *params;
+	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;
+	for (; params->name != NULL; params++) {
+		if ((params->jedec >> 16) == idcode[0]) {
+			if ((params->jedec & 0xFFFF) == jedec) {
+				if (params->ext_jedec == 0)
+					break;
+				else if (params->ext_jedec == ext_jedec)
+					break;
+			}
+		}
+	}
+
+	if (!params->name) {
+		printf("SF: Unsupported flash IDs: ");
+		printf("manuf %02x, jedec %04x, ext_jedec %04x\n",
+		       idcode[0], jedec, ext_jedec);
+		return -EPROTONOSUPPORT;
+	}
+
+	/* Flash powers up read-only, so clear BP# bits */
+#if defined(CONFIG_SPI_FLASH_ATMEL) || \
+	defined(CONFIG_SPI_FLASH_MACRONIX) || \
+	defined(CONFIG_SPI_FLASH_SST)
+		spi_flash_cmd_write_status(flash, 0);
+#endif
+
+	/* Assign spi data */
+	flash->spi = spi;
+	flash->name = params->name;
+	flash->memory_map = spi->memory_map;
+	flash->dual_flash = flash->spi->option;
+
+	/* 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 (params->flags & SNOR_F_SST_WR) {
+		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
+			flash->write = sst_write_bp;
+		else
+			flash->write = sst_write_wp;
+	}
+#endif
+	flash->erase = spi_flash_cmd_erase_ops;
+	flash->read = spi_flash_cmd_read_ops;
+#endif
+
+	/* Compute the flash size */
+	flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
+	/*
+	 * The Spansion S25FL032P and S25FL064P have 256b pages, yet use the
+	 * 0x4d00 Extended JEDEC code. The rest of the Spansion flashes with
+	 * the 0x4d00 Extended JEDEC code have 512b pages. All of the others
+	 * have 256b pages.
+	 */
+	if (ext_jedec == 0x4d00) {
+		if ((jedec == 0x0215) || (jedec == 0x216))
+			flash->page_size = 256;
+		else
+			flash->page_size = 512;
+	} else {
+		flash->page_size = 256;
+	}
+	flash->page_size <<= flash->shift;
+	flash->sector_size = params->sector_size << flash->shift;
+	flash->size = flash->sector_size * params->nr_sectors << flash->shift;
+#ifdef CONFIG_SF_DUAL_FLASH
+	if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
+		flash->size <<= 1;
+#endif
+
+	/* Compute erase sector and command */
+	if (params->flags & SECT_4K) {
+		flash->erase_cmd = CMD_ERASE_4K;
+		flash->erase_size = 4096 << flash->shift;
+	} else if (params->flags & SECT_32K) {
+		flash->erase_cmd = CMD_ERASE_32K;
+		flash->erase_size = 32768 << flash->shift;
+	} else {
+		flash->erase_cmd = CMD_ERASE_64K;
+		flash->erase_size = flash->sector_size;
+	}
+
+	/* Now erase size becomes valid sector size */
+	flash->sector_size = flash->erase_size;
+
+	/* Look for the fastest read cmd */
+	cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
+	if (cmd) {
+		cmd = spi_read_cmds_array[cmd - 1];
+		flash->read_cmd = cmd;
+	} else {
+		/* Go for default supported read cmd */
+		flash->read_cmd = CMD_READ_ARRAY_FAST;
+	}
+
+	/* Not require to look for fastest only two write cmds yet */
+	if (params->flags & WR_QPP && flash->spi->op_mode_tx & SPI_OPM_TX_QPP)
+		flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
+	else
+		/* Go for default supported write cmd */
+		flash->write_cmd = CMD_PAGE_PROGRAM;
+
+	/* Set the quad enable bit - only for quad commands */
+	if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
+	    (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
+	    (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
+		ret = spi_flash_set_qeb(flash, idcode[0]);
+		if (ret) {
+			debug("SF: Fail to set QEB for %02x\n", idcode[0]);
+			return -EINVAL;
+		}
+	}
+
+	/* Read dummy_byte: dummy byte is determined based on the
+	 * dummy cycles of a particular command.
+	 * Fast commands - dummy_byte = dummy_cycles/8
+	 * I/O commands- dummy_byte = (dummy_cycles * no.of lines)/8
+	 * For I/O commands except cmd[0] everything goes on no.of lines
+	 * based on particular command but incase of fast commands except
+	 * data all go on single line irrespective of command.
+	 */
+	switch (flash->read_cmd) {
+	case CMD_READ_QUAD_IO_FAST:
+		flash->dummy_byte = 2;
+		break;
+	case CMD_READ_ARRAY_SLOW:
+		flash->dummy_byte = 0;
+		break;
+	default:
+		flash->dummy_byte = 1;
+	}
+
+#ifdef CONFIG_SPI_FLASH_STMICRO
+	if (params->flags & E_FSR)
+		flash->flags |= SNOR_F_USE_FSR;
+#endif
+
+	/* Configure the BAR - discover bank cmds and read current bank */
+#ifdef CONFIG_SPI_FLASH_BAR
+	ret = spi_flash_read_bank(flash, idcode[0]);
+	if (ret < 0)
+		return ret;
+#endif
+
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+	ret = spi_flash_decode_fdt(gd->fdt_blob, flash);
+	if (ret) {
+		debug("SF: FDT decode error\n");
+		return -EINVAL;
+	}
+#endif
+
+#ifndef CONFIG_SPL_BUILD
+	printf("SF: Detected %s with page size ", flash->name);
+	print_size(flash->page_size, ", erase size ");
+	print_size(flash->erase_size, ", total ");
+	print_size(flash->size, "");
+	if (flash->memory_map)
+		printf(", mapped at %p", flash->memory_map);
+	puts("\n");
+#endif
+
+#ifndef CONFIG_SPI_FLASH_BAR
+	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
+	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
+	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
+	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+		puts("SF: Warning - Only lower 16MiB accessible,");
+		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
+	}
+#endif
+
+	return ret;
+}
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index a150263..d2991de 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -20,333 +20,6 @@
 
 #include "sf_internal.h"
 
-DECLARE_GLOBAL_DATA_PTR;
-
-/* Read commands array */
-static u8 spi_read_cmds_array[] = {
-	CMD_READ_ARRAY_SLOW,
-	CMD_READ_ARRAY_FAST,
-	CMD_READ_DUAL_OUTPUT_FAST,
-	CMD_READ_DUAL_IO_FAST,
-	CMD_READ_QUAD_OUTPUT_FAST,
-	CMD_READ_QUAD_IO_FAST,
-};
-
-#ifdef CONFIG_SPI_FLASH_MACRONIX
-static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
-{
-	u8 qeb_status;
-	int ret;
-
-	ret = spi_flash_cmd_read_status(flash, &qeb_status);
-	if (ret < 0)
-		return ret;
-
-	if (qeb_status & STATUS_QEB_MXIC) {
-		debug("SF: mxic: QEB is already set\n");
-	} else {
-		ret = spi_flash_cmd_write_status(flash, STATUS_QEB_MXIC);
-		if (ret < 0)
-			return ret;
-	}
-
-	return ret;
-}
-#endif
-
-#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;
-
-	ret = spi_flash_cmd_read_config(flash, &qeb_status);
-	if (ret < 0)
-		return ret;
-
-	if (qeb_status & STATUS_QEB_WINSPAN) {
-		debug("SF: winspan: QEB is already set\n");
-	} else {
-		ret = spi_flash_cmd_write_config(flash, STATUS_QEB_WINSPAN);
-		if (ret < 0)
-			return ret;
-	}
-
-	return ret;
-}
-#endif
-
-static int spi_flash_set_qeb(struct spi_flash *flash, u8 idcode0)
-{
-	switch (idcode0) {
-#ifdef CONFIG_SPI_FLASH_MACRONIX
-	case SPI_FLASH_CFI_MFR_MACRONIX:
-		return spi_flash_set_qeb_mxic(flash);
-#endif
-#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
-	case SPI_FLASH_CFI_MFR_SPANSION:
-	case SPI_FLASH_CFI_MFR_WINBOND:
-		return spi_flash_set_qeb_winspan(flash);
-#endif
-#ifdef CONFIG_SPI_FLASH_STMICRO
-	case SPI_FLASH_CFI_MFR_STMICRO:
-		debug("SF: QEB is volatile for %02x flash\n", idcode0);
-		return 0;
-#endif
-	default:
-		printf("SF: Need set QEB func for %02x flash\n", idcode0);
-		return -1;
-	}
-}
-
-#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
-
-#if CONFIG_IS_ENABLED(OF_CONTROL)
-int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
-{
-	fdt_addr_t addr;
-	fdt_size_t size;
-	int node;
-
-	/* If there is no node, do nothing */
-	node = fdtdec_next_compatible(blob, 0, COMPAT_GENERIC_SPI_FLASH);
-	if (node < 0)
-		return 0;
-
-	addr = fdtdec_get_addr_size(blob, node, "memory-map", &size);
-	if (addr == FDT_ADDR_T_NONE) {
-		debug("%s: Cannot decode address\n", __func__);
-		return 0;
-	}
-
-	if (flash->size != size) {
-		debug("%s: Memory map must cover entire device\n", __func__);
-		return -1;
-	}
-	flash->memory_map = map_sysmem(addr, size);
-
-	return 0;
-}
-#endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
-
-static int spi_flash_scan(struct spi_slave *spi, u8 *idcode,
-				     struct spi_flash *flash)
-{
-	const struct spi_flash_params *params;
-	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;
-	for (; params->name != NULL; params++) {
-		if ((params->jedec >> 16) == idcode[0]) {
-			if ((params->jedec & 0xFFFF) == jedec) {
-				if (params->ext_jedec == 0)
-					break;
-				else if (params->ext_jedec == ext_jedec)
-					break;
-			}
-		}
-	}
-
-	if (!params->name) {
-		printf("SF: Unsupported flash IDs: ");
-		printf("manuf %02x, jedec %04x, ext_jedec %04x\n",
-		       idcode[0], jedec, ext_jedec);
-		return -EPROTONOSUPPORT;
-	}
-
-	/* Flash powers up read-only, so clear BP# bits */
-#if defined(CONFIG_SPI_FLASH_ATMEL) || \
-	defined(CONFIG_SPI_FLASH_MACRONIX) || \
-	defined(CONFIG_SPI_FLASH_SST)
-		spi_flash_cmd_write_status(flash, 0);
-#endif
-
-	/* Assign spi data */
-	flash->spi = spi;
-	flash->name = params->name;
-	flash->memory_map = spi->memory_map;
-	flash->dual_flash = flash->spi->option;
-
-	/* 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 (params->flags & SNOR_F_SST_WR) {
-		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
-			flash->write = sst_write_bp;
-		else
-			flash->write = sst_write_wp;
-	}
-#endif
-	flash->erase = spi_flash_cmd_erase_ops;
-	flash->read = spi_flash_cmd_read_ops;
-#endif
-
-	/* Compute the flash size */
-	flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
-	/*
-	 * The Spansion S25FL032P and S25FL064P have 256b pages, yet use the
-	 * 0x4d00 Extended JEDEC code. The rest of the Spansion flashes with
-	 * the 0x4d00 Extended JEDEC code have 512b pages. All of the others
-	 * have 256b pages.
-	 */
-	if (ext_jedec == 0x4d00) {
-		if ((jedec == 0x0215) || (jedec == 0x216))
-			flash->page_size = 256;
-		else
-			flash->page_size = 512;
-	} else {
-		flash->page_size = 256;
-	}
-	flash->page_size <<= flash->shift;
-	flash->sector_size = params->sector_size << flash->shift;
-	flash->size = flash->sector_size * params->nr_sectors << flash->shift;
-#ifdef CONFIG_SF_DUAL_FLASH
-	if (flash->dual_flash & SF_DUAL_STACKED_FLASH)
-		flash->size <<= 1;
-#endif
-
-	/* Compute erase sector and command */
-	if (params->flags & SECT_4K) {
-		flash->erase_cmd = CMD_ERASE_4K;
-		flash->erase_size = 4096 << flash->shift;
-	} else if (params->flags & SECT_32K) {
-		flash->erase_cmd = CMD_ERASE_32K;
-		flash->erase_size = 32768 << flash->shift;
-	} else {
-		flash->erase_cmd = CMD_ERASE_64K;
-		flash->erase_size = flash->sector_size;
-	}
-
-	/* Now erase size becomes valid sector size */
-	flash->sector_size = flash->erase_size;
-
-	/* Look for the fastest read cmd */
-	cmd = fls(params->e_rd_cmd & flash->spi->op_mode_rx);
-	if (cmd) {
-		cmd = spi_read_cmds_array[cmd - 1];
-		flash->read_cmd = cmd;
-	} else {
-		/* Go for default supported read cmd */
-		flash->read_cmd = CMD_READ_ARRAY_FAST;
-	}
-
-	/* Not require to look for fastest only two write cmds yet */
-	if (params->flags & WR_QPP && flash->spi->op_mode_tx & SPI_OPM_TX_QPP)
-		flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
-	else
-		/* Go for default supported write cmd */
-		flash->write_cmd = CMD_PAGE_PROGRAM;
-
-	/* Set the quad enable bit - only for quad commands */
-	if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
-	    (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
-	    (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
-		ret = spi_flash_set_qeb(flash, idcode[0]);
-		if (ret) {
-			debug("SF: Fail to set QEB for %02x\n", idcode[0]);
-			return -EINVAL;
-		}
-	}
-
-	/* Read dummy_byte: dummy byte is determined based on the
-	 * dummy cycles of a particular command.
-	 * Fast commands - dummy_byte = dummy_cycles/8
-	 * I/O commands- dummy_byte = (dummy_cycles * no.of lines)/8
-	 * For I/O commands except cmd[0] everything goes on no.of lines
-	 * based on particular command but incase of fast commands except
-	 * data all go on single line irrespective of command.
-	 */
-	switch (flash->read_cmd) {
-	case CMD_READ_QUAD_IO_FAST:
-		flash->dummy_byte = 2;
-		break;
-	case CMD_READ_ARRAY_SLOW:
-		flash->dummy_byte = 0;
-		break;
-	default:
-		flash->dummy_byte = 1;
-	}
-
-#ifdef CONFIG_SPI_FLASH_STMICRO
-	if (params->flags & E_FSR)
-		flash->flags |= SNOR_F_USE_FSR;
-#endif
-
-	/* Configure the BAR - discover bank cmds and read current bank */
-#ifdef CONFIG_SPI_FLASH_BAR
-	ret = spi_flash_read_bank(flash, idcode[0]);
-	if (ret < 0)
-		return ret;
-#endif
-
-#if CONFIG_IS_ENABLED(OF_CONTROL)
-	ret = spi_flash_decode_fdt(gd->fdt_blob, flash);
-	if (ret) {
-		debug("SF: FDT decode error\n");
-		return -EINVAL;
-	}
-#endif
-
-#ifndef CONFIG_SPL_BUILD
-	printf("SF: Detected %s with page size ", flash->name);
-	print_size(flash->page_size, ", erase size ");
-	print_size(flash->erase_size, ", total ");
-	print_size(flash->size, "");
-	if (flash->memory_map)
-		printf(", mapped at %p", flash->memory_map);
-	puts("\n");
-#endif
-
-#ifndef CONFIG_SPI_FLASH_BAR
-	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
-	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
-		puts("SF: Warning - Only lower 16MiB accessible,");
-		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
-	}
-#endif
-
-	return ret;
-}
-
 /**
  * spi_flash_probe_slave() - Probe for a SPI flash device on a bus
  *
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 10/21] sf: Move the read_id code to sf_ops
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (8 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 09/21] sf: Move spi_flash_scan code to sf_ops Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 11/21] sf: Move BAR defined code at once place Jagan Teki
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

read_id code is related to spi_flash stuff hence moved to sf_ops.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h |  3 +--
 drivers/mtd/spi/sf_ops.c      | 21 ++++++++++++++++++---
 drivers/mtd/spi/sf_probe.c    | 15 +--------------
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 3f07560..8216fdc 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -235,7 +235,6 @@ void spi_flash_mtd_unregister(void);
 /**
  * spi_flash_scan - scan the SPI FLASH
  * @spi:	the spi slave structure
- * @idcode:	idcode of spi flash
  * @flash:	the spi flash structure
  *
  * The drivers can use this fuction to scan the SPI FLASH.
@@ -244,6 +243,6 @@ void spi_flash_mtd_unregister(void);
  *
  * Return: 0 for success, others for failure.
  */
-int spi_flash_scan(struct spi_slave *spi, u8 *idcode, struct spi_flash *flash);
+int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash);
 
 #endif /* _SF_INTERNAL_H_ */
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index d37b0a9..016a5df 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -706,14 +706,29 @@ int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
 }
 #endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
 
-int spi_flash_scan(struct spi_slave *spi, u8 *idcode, struct spi_flash *flash)
+int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 {
 	const struct spi_flash_params *params;
+	u16 jedec, ext_jedec;
+	u8 idcode[5];
 	u8 cmd;
-	u16 jedec = idcode[1] << 8 | idcode[2];
-	u16 ext_jedec = idcode[3] << 8 | idcode[4];
 	int ret;
 
+	/* Read the ID codes */
+	ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
+	if (ret) {
+		printf("SF: Failed to get idcodes\n");
+		return -EINVAL;
+	}
+
+#ifdef DEBUG
+	printf("SF: Got idcodes\n");
+	print_buffer(0, idcode, 1, sizeof(idcode), 0);
+#endif
+
+	jedec = idcode[1] << 8 | idcode[2];
+	ext_jedec = idcode[3] << 8 | idcode[4];
+
 	/* Validate params from spi_flash_params table */
 	params = spi_flash_params_table;
 	for (; params->name != NULL; params++) {
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index d2991de..a712792 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -29,7 +29,6 @@
  */
 int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 {
-	u8 idcode[5];
 	int ret;
 
 	/* Setup spi_slave */
@@ -45,19 +44,7 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 		return ret;
 	}
 
-	/* Read the ID codes */
-	ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
-	if (ret) {
-		printf("SF: Failed to get idcodes\n");
-		goto err_read_id;
-	}
-
-#ifdef DEBUG
-	printf("SF: Got idcodes\n");
-	print_buffer(0, idcode, 1, sizeof(idcode), 0);
-#endif
-
-	ret = spi_flash_scan(spi, idcode, flash);
+	ret = spi_flash_scan(spi, flash);
 	if (ret) {
 		ret = -EINVAL;
 		goto err_read_id;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 11/21] sf: Move BAR defined code at once place
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (9 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 10/21] sf: Move the read_id " Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 12/21] sf: Use static for file-scope functions Jagan Teki
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Move bar read code below the bar write hance both at once place,
hence it easy for #ifdef macro only once and readable.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_ops.c | 81 +++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 016a5df..e20a3d6 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -29,6 +29,16 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
 	cmd[3] = addr >> 0;
 }
 
+/* Read commands array */
+static u8 spi_read_cmds_array[] = {
+	CMD_READ_ARRAY_SLOW,
+	CMD_READ_ARRAY_FAST,
+	CMD_READ_DUAL_OUTPUT_FAST,
+	CMD_READ_DUAL_IO_FAST,
+	CMD_READ_QUAD_OUTPUT_FAST,
+	CMD_READ_QUAD_IO_FAST,
+};
+
 int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs)
 {
 	int ret;
@@ -132,6 +142,35 @@ bar_end:
 	flash->bank_curr = bank_sel;
 	return flash->bank_curr;
 }
+
+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
 
 #ifdef CONFIG_SF_DUAL_FLASH
@@ -569,17 +608,6 @@ int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
 }
 #endif
 
-
-/* Read commands array */
-static u8 spi_read_cmds_array[] = {
-	CMD_READ_ARRAY_SLOW,
-	CMD_READ_ARRAY_FAST,
-	CMD_READ_DUAL_OUTPUT_FAST,
-	CMD_READ_DUAL_IO_FAST,
-	CMD_READ_QUAD_OUTPUT_FAST,
-	CMD_READ_QUAD_IO_FAST,
-};
-
 #ifdef CONFIG_SPI_FLASH_MACRONIX
 static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 {
@@ -647,37 +675,6 @@ 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
-
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 12/21] sf: Use static for file-scope functions
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (10 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 11/21] sf: Move BAR defined code at once place Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 13/21] sf: Fix Makefile Jagan Teki
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Use static for file-scope functions and removed them from
header files.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h | 18 ------------------
 drivers/mtd/spi/sf_ops.c      | 11 ++++++-----
 2 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 8216fdc..8ef88f4 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -167,18 +167,6 @@ 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, u8 *rs);
-
-/* 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);
-
-/* Program the config register */
-int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc);
-
 /* Enable writing on the SPI flash */
 static inline int spi_flash_cmd_write_enable(struct spi_flash *flash)
 {
@@ -192,12 +180,6 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash)
 }
 
 /*
- * Send the read status command to the device and wait for the wip
- * (write-in-progress) bit to clear itself.
- */
-int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout);
-
-/*
  * Used for spi_flash write operation
  * - SPI claim
  * - spi_flash_cmd_write_enable
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index e20a3d6..af94c0b 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -39,7 +39,7 @@ static u8 spi_read_cmds_array[] = {
 	CMD_READ_QUAD_IO_FAST,
 };
 
-int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs)
+static int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs)
 {
 	int ret;
 	u8 cmd;
@@ -68,7 +68,7 @@ static int read_fsr(struct spi_flash *flash, u8 *fsr)
 	return 0;
 }
 
-int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
+static int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
 {
 	u8 cmd;
 	int ret;
@@ -84,7 +84,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, u8 *rc)
+static int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc)
 {
 	int ret;
 	u8 cmd;
@@ -99,7 +99,7 @@ int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc)
 	return 0;
 }
 
-int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
+static int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
 {
 	u8 data[2];
 	u8 cmd;
@@ -237,7 +237,8 @@ static int spi_flash_ready(struct spi_flash *flash)
 	return sr && fsr;
 }
 
-int spi_flash_cmd_wait_ready(struct spi_flash *flash, unsigned long timeout)
+static int spi_flash_cmd_wait_ready(struct spi_flash *flash,
+					unsigned long timeout)
 {
 	int timebase, ret;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 13/21] sf: Fix Makefile
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (11 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 12/21] sf: Use static for file-scope functions Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 14/21] sf: Use simple name for register access functions Jagan Teki
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

This patch removes unneeded ifdef and fixed accordingly.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/Makefile | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index ff48b25..66c4424 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -12,11 +12,7 @@ obj-$(CONFIG_SPL_SPI_LOAD)	+= spi_spl_load.o
 obj-$(CONFIG_SPL_SPI_BOOT)	+= fsl_espi_spl.o
 endif
 
-#ifndef CONFIG_DM_SPI
-obj-$(CONFIG_SPI_FLASH) += sf_probe.o
-#endif
-obj-$(CONFIG_CMD_SF) += sf.o
-obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
+obj-$(CONFIG_SPI_FLASH) += sf_probe.o sf_ops.o sf_params.o sf.o
 obj-$(CONFIG_SPI_FLASH_DATAFLASH) += sf_dataflash.o
 obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
 obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 14/21] sf: Use simple name for register access functions
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (12 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 13/21] sf: Fix Makefile Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 15/21] sf: Use flash function pointers in dm_spi_flash_ops Jagan Teki
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Most of the register access function are static, so used
simple name to represent each.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_ops.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index af94c0b..97e644d 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -39,7 +39,7 @@ static u8 spi_read_cmds_array[] = {
 	CMD_READ_QUAD_IO_FAST,
 };
 
-static int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs)
+static int read_sr(struct spi_flash *flash, u8 *rs)
 {
 	int ret;
 	u8 cmd;
@@ -68,7 +68,7 @@ static int read_fsr(struct spi_flash *flash, u8 *fsr)
 	return 0;
 }
 
-static int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
+static int write_sr(struct spi_flash *flash, u8 ws)
 {
 	u8 cmd;
 	int ret;
@@ -84,7 +84,7 @@ static int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws)
 }
 
 #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
-static int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc)
+static int read_cr(struct spi_flash *flash, u8 *rc)
 {
 	int ret;
 	u8 cmd;
@@ -99,13 +99,13 @@ static int spi_flash_cmd_read_config(struct spi_flash *flash, u8 *rc)
 	return 0;
 }
 
-static int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
+static int write_cr(struct spi_flash *flash, u8 wc)
 {
 	u8 data[2];
 	u8 cmd;
 	int ret;
 
-	ret = spi_flash_cmd_read_status(flash, &data[0]);
+	ret = read_sr(flash, &data[0]);
 	if (ret < 0)
 		return ret;
 
@@ -122,7 +122,7 @@ static int spi_flash_cmd_write_config(struct spi_flash *flash, u8 wc)
 #endif
 
 #ifdef CONFIG_SPI_FLASH_BAR
-static int spi_flash_write_bank(struct spi_flash *flash, u32 offset)
+static int spi_flash_write_bar(struct spi_flash *flash, u32 offset)
 {
 	u8 cmd, bank_sel;
 	int ret;
@@ -143,7 +143,7 @@ bar_end:
 	return flash->bank_curr;
 }
 
-static int spi_flash_read_bank(struct spi_flash *flash, u8 idcode0)
+static int spi_flash_read_bar(struct spi_flash *flash, u8 idcode0)
 {
 	u8 curr_bank = 0;
 	int ret;
@@ -174,7 +174,7 @@ bank_end:
 #endif
 
 #ifdef CONFIG_SF_DUAL_FLASH
-static void spi_flash_dual_flash(struct spi_flash *flash, u32 *addr)
+static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
 {
 	switch (flash->dual_flash) {
 	case SF_DUAL_STACKED_FLASH:
@@ -200,7 +200,7 @@ static int spi_flash_sr_ready(struct spi_flash *flash)
 	u8 sr;
 	int ret;
 
-	ret = spi_flash_cmd_read_status(flash, &sr);
+	ret = read_sr(flash, &sr);
 	if (ret < 0)
 		return ret;
 
@@ -316,10 +316,10 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 
 #ifdef CONFIG_SF_DUAL_FLASH
 		if (flash->dual_flash > SF_SINGLE_FLASH)
-			spi_flash_dual_flash(flash, &erase_addr);
+			spi_flash_dual(flash, &erase_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		ret = spi_flash_write_bank(flash, erase_addr);
+		ret = spi_flash_write_bar(flash, erase_addr);
 		if (ret < 0)
 			return ret;
 #endif
@@ -358,10 +358,10 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 
 #ifdef CONFIG_SF_DUAL_FLASH
 		if (flash->dual_flash > SF_SINGLE_FLASH)
-			spi_flash_dual_flash(flash, &write_addr);
+			spi_flash_dual(flash, &write_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		ret = spi_flash_write_bank(flash, write_addr);
+		ret = spi_flash_write_bar(flash, write_addr);
 		if (ret < 0)
 			return ret;
 #endif
@@ -453,10 +453,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 
 #ifdef CONFIG_SF_DUAL_FLASH
 		if (flash->dual_flash > SF_SINGLE_FLASH)
-			spi_flash_dual_flash(flash, &read_addr);
+			spi_flash_dual(flash, &read_addr);
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
-		ret = spi_flash_write_bank(flash, read_addr);
+		ret = spi_flash_write_bar(flash, read_addr);
 		if (ret < 0)
 			return ret;
 		bank_sel = flash->bank_curr;
@@ -615,14 +615,14 @@ static int spi_flash_set_qeb_mxic(struct spi_flash *flash)
 	u8 qeb_status;
 	int ret;
 
-	ret = spi_flash_cmd_read_status(flash, &qeb_status);
+	ret = read_sr(flash, &qeb_status);
 	if (ret < 0)
 		return ret;
 
 	if (qeb_status & STATUS_QEB_MXIC) {
 		debug("SF: mxic: QEB is already set\n");
 	} else {
-		ret = spi_flash_cmd_write_status(flash, STATUS_QEB_MXIC);
+		ret = write_sr(flash, STATUS_QEB_MXIC);
 		if (ret < 0)
 			return ret;
 	}
@@ -637,14 +637,14 @@ static int spi_flash_set_qeb_winspan(struct spi_flash *flash)
 	u8 qeb_status;
 	int ret;
 
-	ret = spi_flash_cmd_read_config(flash, &qeb_status);
+	ret = read_cr(flash, &qeb_status);
 	if (ret < 0)
 		return ret;
 
 	if (qeb_status & STATUS_QEB_WINSPAN) {
 		debug("SF: winspan: QEB is already set\n");
 	} else {
-		ret = spi_flash_cmd_write_config(flash, STATUS_QEB_WINSPAN);
+		ret = write_cr(flash, STATUS_QEB_WINSPAN);
 		if (ret < 0)
 			return ret;
 	}
@@ -751,7 +751,7 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 #if defined(CONFIG_SPI_FLASH_ATMEL) || \
 	defined(CONFIG_SPI_FLASH_MACRONIX) || \
 	defined(CONFIG_SPI_FLASH_SST)
-		spi_flash_cmd_write_status(flash, 0);
+		write_sr(flash, 0);
 #endif
 
 	/* Assign spi data */
@@ -871,7 +871,7 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
-	ret = spi_flash_read_bank(flash, idcode[0]);
+	ret = spi_flash_read_bar(flash, idcode[0]);
 	if (ret < 0)
 		return ret;
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 15/21] sf: Use flash function pointers in dm_spi_flash_ops
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (13 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 14/21] sf: Use simple name for register access functions Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 16/21] sf: Flash power up read-only based on idcode0 Jagan Teki
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

flash operations are defined as static and reuse them with
function-pointers so call them with generic function pounters
instead of calling like normal functions.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_ops.c   |  2 --
 drivers/mtd/spi/sf_probe.c | 15 +++------------
 include/spi_flash.h        | 13 -------------
 3 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 97e644d..f63ce7a 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -761,7 +761,6 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 	flash->dual_flash = flash->spi->option;
 
 	/* 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)
@@ -776,7 +775,6 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 #endif
 	flash->erase = spi_flash_cmd_erase_ops;
 	flash->read = spi_flash_cmd_read_ops;
-#endif
 
 	/* Compute the flash size */
 	flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : 0;
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index a712792..e2dac7e 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -120,7 +120,7 @@ static int spi_flash_std_read(struct udevice *dev, u32 offset, size_t len,
 {
 	struct spi_flash *flash = dev_get_uclass_priv(dev);
 
-	return spi_flash_cmd_read_ops(flash, offset, len, buf);
+	return flash->read(flash, offset, len, buf);
 }
 
 int spi_flash_std_write(struct udevice *dev, u32 offset, size_t len,
@@ -128,23 +128,14 @@ 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 & SNOR_F_SST_WR) {
-		if (flash->spi->op_mode_tx & SPI_OPM_TX_BP)
-			return sst_write_bp(flash, offset, len, buf);
-		else
-			return sst_write_wp(flash, offset, len, buf);
-	}
-#endif
-
-	return spi_flash_cmd_write_ops(flash, offset, len, buf);
+	return flash->write(flash, offset, len, buf);
 }
 
 int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
 {
 	struct spi_flash *flash = dev_get_uclass_priv(dev);
 
-	return spi_flash_cmd_erase_ops(flash, offset, len);
+	return flash->erase(flash, offset, len);
 }
 
 int spi_flash_std_probe(struct udevice *dev)
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 4312d3d..0732172 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -87,23 +87,10 @@ struct spi_flash {
 	u8 dummy_byte;
 
 	void *memory_map;
-#ifndef CONFIG_DM_SPI_FLASH
-	/*
-	 * These are not strictly needed for driver model, but keep them here
-	 * while the transition is in progress.
-	 *
-	 * Normally each driver would provide its own operations, but for
-	 * SPI flash most chips use the same algorithms. One approach is
-	 * to create a 'common' SPI flash device which knows how to talk
-	 * to most devices, and then allow other drivers to be used instead
-	 * if required, perhaps with a way of scanning through the list to
-	 * find the driver that matches the device.
-	 */
 	int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf);
 	int (*write)(struct spi_flash *flash, u32 offset, size_t len,
 			const void *buf);
 	int (*erase)(struct spi_flash *flash, u32 offset, size_t len);
-#endif
 };
 
 struct dm_spi_flash_ops {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 16/21] sf: Flash power up read-only based on idcode0
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (14 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 15/21] sf: Use flash function pointers in dm_spi_flash_ops Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 17/21] sf: Use static for file-scope functions Jagan Teki
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Using macro's for flash power up read-only access code leads
wrong behaviour hence use idcode0 for runtime detection, hence
the flash which require this functionality gets detected at runtime.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h | 2 ++
 drivers/mtd/spi/sf_ops.c      | 7 +++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 8ef88f4..39749c2 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -64,7 +64,9 @@ enum spi_nor_option_flags {
 #define SPI_FLASH_CFI_MFR_SPANSION	0x01
 #define SPI_FLASH_CFI_MFR_STMICRO	0x20
 #define SPI_FLASH_CFI_MFR_MACRONIX	0xc2
+#define SPI_FLASH_CFI_MFR_SST		0xbf
 #define SPI_FLASH_CFI_MFR_WINBOND	0xef
+#define SPI_FLASH_CFI_MFR_ATMEL		0x1f
 
 /* Erase commands */
 #define CMD_ERASE_4K			0x20
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index f63ce7a..f90f041 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -748,11 +748,10 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 	}
 
 	/* Flash powers up read-only, so clear BP# bits */
-#if defined(CONFIG_SPI_FLASH_ATMEL) || \
-	defined(CONFIG_SPI_FLASH_MACRONIX) || \
-	defined(CONFIG_SPI_FLASH_SST)
+	if (idcode[0] == SPI_FLASH_CFI_MFR_ATMEL ||
+	    idcode[0] == SPI_FLASH_CFI_MFR_MACRONIX ||
+	    idcode[0] == SPI_FLASH_CFI_MFR_SST)
 		write_sr(flash, 0);
-#endif
 
 	/* Assign spi data */
 	flash->spi = spi;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 17/21] sf: Use static for file-scope functions
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (15 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 16/21] sf: Flash power up read-only based on idcode0 Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 18/21] sf: Remove unneeded header includes Jagan Teki
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Use static for file-scope functions and removed them from
header files.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h | 20 --------------------
 drivers/mtd/spi/sf_ops.c      | 11 ++++++-----
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 39749c2..2873402 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -117,11 +117,6 @@ enum spi_nor_option_flags {
 #ifdef CONFIG_SPI_FLASH_SST
 # define CMD_SST_BP		0x02    /* Byte Program */
 # define CMD_SST_AAI_WP		0xAD	/* Auto Address Incr Word Program */
-
-int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
-		const void *buf);
-int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
-		const void *buf);
 #endif
 
 /**
@@ -166,9 +161,6 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 		const void *data, size_t data_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);
-
 /* Enable writing on the SPI flash */
 static inline int spi_flash_cmd_write_enable(struct spi_flash *flash)
 {
@@ -193,24 +185,12 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
 		size_t cmd_len, const void *buf, size_t buf_len);
 
 /*
- * Flash write operation, support all possible write commands.
- * Write the requested data out breaking it up into multiple write
- * commands as needed per the write size.
- */
-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);
-
 #ifdef CONFIG_SPI_FLASH_MTD
 int spi_flash_mtd_register(struct spi_flash *flash);
 void spi_flash_mtd_unregister(void);
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index f90f041..e04f8aa 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -298,7 +298,8 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
 	return ret;
 }
 
-int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
+static int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset,
+					size_t len)
 {
 	u32 erase_size, erase_addr;
 	u8 cmd[SPI_FLASH_CMD_LEN];
@@ -341,7 +342,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 	return ret;
 }
 
-int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
+static int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		size_t len, const void *buf)
 {
 	unsigned long byte_addr, page_size;
@@ -418,7 +419,7 @@ void __weak spi_flash_copy_mmap(void *data, void *offset, size_t len)
 	memcpy(data, offset, len);
 }
 
-int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
+static int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		size_t len, void *data)
 {
 	u8 *cmd, cmdsz;
@@ -510,7 +511,7 @@ static int sst_byte_write(struct spi_flash *flash, u32 offset, const void *buf)
 	return spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
 }
 
-int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
+static int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 		const void *buf)
 {
 	size_t actual, cmd_len;
@@ -577,7 +578,7 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 	return ret;
 }
 
-int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
+static int sst_write_bp(struct spi_flash *flash, u32 offset, size_t len,
 		const void *buf)
 {
 	size_t actual;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 18/21] sf: Remove unneeded header includes
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (16 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 17/21] sf: Use static for file-scope functions Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 19/21] sf: probe: Use spi_flash_scan in dm-spi-flash Jagan Teki
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

Removed unneeded header includes in sf_ops and sf_probe.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_ops.c   | 2 --
 drivers/mtd/spi/sf_probe.c | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index e04f8aa..d3e8a83 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -14,8 +14,6 @@
 #include <mapmem.h>
 #include <spi.h>
 #include <spi_flash.h>
-#include <watchdog.h>
-#include <linux/compiler.h>
 
 #include "sf_internal.h"
 
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index e2dac7e..cfc696f 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -11,12 +11,9 @@
 #include <common.h>
 #include <dm.h>
 #include <errno.h>
-#include <fdtdec.h>
 #include <malloc.h>
-#include <mapmem.h>
 #include <spi.h>
 #include <spi_flash.h>
-#include <asm/io.h>
 
 #include "sf_internal.h"
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 19/21] sf: probe: Use spi_flash_scan in dm-spi-flash
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (17 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 18/21] sf: Remove unneeded header includes Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 20/21] sf: Re-factorize spi_flash_probe_tail code Jagan Teki
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

This patch add support to use spi_flash_scan in
dm-spi-flash probe, so-that it can access the spi_flash
functionalities same as non-dm sf probe.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_probe.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index cfc696f..6ce82a7 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -137,14 +137,36 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
 
 int spi_flash_std_probe(struct udevice *dev)
 {
-	struct spi_slave *slave = dev_get_parentdata(dev);
 	struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
+	struct spi_slave *slave = dev_get_parentdata(dev);
 	struct spi_flash *flash;
+	int ret;
+
+	debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs);
 
 	flash = dev_get_uclass_priv(dev);
 	flash->dev = dev;
-	debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs);
-	return spi_flash_probe_slave(slave, flash);
+
+	/* Claim spi bus */
+	ret = spi_claim_bus(slave);
+	if (ret) {
+		debug("SF: Failed to claim SPI bus: %d\n", ret);
+		return ret;
+	}
+
+	ret = spi_flash_scan(slave, flash);
+	if (ret) {
+		ret = -EINVAL;
+		goto err_read_id;
+	}
+
+#ifdef CONFIG_SPI_FLASH_MTD
+	ret = spi_flash_mtd_register(flash);
+#endif
+
+err_read_id:
+	spi_release_bus(slave);
+	return ret;
 }
 
 static const struct dm_spi_flash_ops spi_flash_std_ops = {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 20/21] sf: Re-factorize spi_flash_probe_tail code
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (18 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 19/21] sf: probe: Use spi_flash_scan in dm-spi-flash Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 21/21] dm-sf: Re-factorize spi_flash_std_probe code Jagan Teki
  2015-10-18 20:27 ` [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Simon Glass
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

spi_flash_probe_tail code looks not in proper shape to
add more functionalities. hence refactorized so-that it's
more readable and hence we may extend more functionalies to it.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_internal.h |  3 +-
 drivers/mtd/spi/sf_ops.c      |  7 ++---
 drivers/mtd/spi/sf_probe.c    | 72 +++++++++++++++++--------------------------
 3 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 2873402..d74bc18 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -198,7 +198,6 @@ void spi_flash_mtd_unregister(void);
 
 /**
  * spi_flash_scan - scan the SPI FLASH
- * @spi:	the spi slave structure
  * @flash:	the spi flash structure
  *
  * The drivers can use this fuction to scan the SPI FLASH.
@@ -207,6 +206,6 @@ void spi_flash_mtd_unregister(void);
  *
  * Return: 0 for success, others for failure.
  */
-int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash);
+int spi_flash_scan(struct spi_flash *flash);
 
 #endif /* _SF_INTERNAL_H_ */
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index d3e8a83..703099f 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -703,7 +703,7 @@ int spi_flash_decode_fdt(const void *blob, struct spi_flash *flash)
 }
 #endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
 
-int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
+int spi_flash_scan(struct spi_flash *flash)
 {
 	const struct spi_flash_params *params;
 	u16 jedec, ext_jedec;
@@ -712,7 +712,7 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 	int ret;
 
 	/* Read the ID codes */
-	ret = spi_flash_cmd(spi, CMD_READ_ID, idcode, sizeof(idcode));
+	ret = spi_flash_cmd(flash->spi, CMD_READ_ID, idcode, sizeof(idcode));
 	if (ret) {
 		printf("SF: Failed to get idcodes\n");
 		return -EINVAL;
@@ -753,9 +753,8 @@ int spi_flash_scan(struct spi_slave *spi, struct spi_flash *flash)
 		write_sr(flash, 0);
 
 	/* Assign spi data */
-	flash->spi = spi;
 	flash->name = params->name;
-	flash->memory_map = spi->memory_map;
+	flash->memory_map = flash->spi->memory_map;
 	flash->dual_flash = flash->spi->option;
 
 	/* Assign spi_flash ops */
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 6ce82a7..319b7e6 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -17,49 +17,11 @@
 
 #include "sf_internal.h"
 
-/**
- * spi_flash_probe_slave() - Probe for a SPI flash device on a bus
- *
- * @spi: Bus to probe
- * @flashp: Pointer to place to put flash info, which may be NULL if the
- * space should be allocated
- */
-int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
-{
-	int ret;
-
-	/* Setup spi_slave */
-	if (!spi) {
-		printf("SF: Failed to set up slave\n");
-		return -ENODEV;
-	}
-
-	/* Claim spi bus */
-	ret = spi_claim_bus(spi);
-	if (ret) {
-		debug("SF: Failed to claim SPI bus: %d\n", ret);
-		return ret;
-	}
-
-	ret = spi_flash_scan(spi, flash);
-	if (ret) {
-		ret = -EINVAL;
-		goto err_read_id;
-	}
-
-#ifdef CONFIG_SPI_FLASH_MTD
-	ret = spi_flash_mtd_register(flash);
-#endif
-
-err_read_id:
-	spi_release_bus(spi);
-	return ret;
-}
-
 #ifndef CONFIG_DM_SPI_FLASH
 struct spi_flash *spi_flash_probe_tail(struct spi_slave *bus)
 {
 	struct spi_flash *flash;
+	int ret;
 
 	/* Allocate space if needed (not used by sf-uclass */
 	flash = calloc(1, sizeof(*flash));
@@ -68,13 +30,37 @@ struct spi_flash *spi_flash_probe_tail(struct spi_slave *bus)
 		return NULL;
 	}
 
-	if (spi_flash_probe_slave(bus, flash)) {
-		spi_free_slave(bus);
-		free(flash);
-		return NULL;
+	flash->spi = bus;
+
+	/* Claim spi bus */
+	ret = spi_claim_bus(bus);
+	if (ret) {
+		debug("SF: Failed to claim SPI bus: %d\n", ret);
+		goto err_claim;
 	}
 
+	ret = spi_flash_scan(flash);
+	if (ret)
+		goto err_scan;
+
+#ifdef CONFIG_SPI_FLASH_MTD
+	ret = spi_flash_mtd_register(flash);
+	if (ret) {
+		printf("SF: failed to register mtd device: %d\n", ret);
+		goto err_mtd;
+	}
+#endif
 	return flash;
+
+#ifdef CONFIG_SPI_FLASH_MTD
+err_mtd:
+	spi_free_slave(bus);
+#endif
+err_scan:
+	spi_release_bus(bus);
+err_claim:
+	free(flash);
+	return NULL;
 }
 
 struct spi_flash *spi_flash_probe(unsigned int busnum, unsigned int cs,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 21/21] dm-sf: Re-factorize spi_flash_std_probe code
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (19 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 20/21] sf: Re-factorize spi_flash_probe_tail code Jagan Teki
@ 2015-10-12 15:01 ` Jagan Teki
  2015-10-18 20:27 ` [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Simon Glass
  21 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-12 15:01 UTC (permalink / raw)
  To: u-boot

spi_flash_probe_tail code looks not in proper shape to
add more functionalities. hence refactorized so-that it's
more readable and hence we may extend more functionalies to it.

Signed-off-by: Jagan Teki <jteki@openedev.com>
---
Changes for v4, v3, v2:
	- none

 drivers/mtd/spi/sf_probe.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 319b7e6..87ac33e 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -123,15 +123,12 @@ int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
 
 int spi_flash_std_probe(struct udevice *dev)
 {
-	struct dm_spi_slave_platdata *plat = dev_get_parent_platdata(dev);
+	struct spi_flash *flash = dev_get_uclass_priv(dev);
 	struct spi_slave *slave = dev_get_parentdata(dev);
-	struct spi_flash *flash;
 	int ret;
 
-	debug("%s: slave=%p, cs=%d\n", __func__, slave, plat->cs);
-
-	flash = dev_get_uclass_priv(dev);
 	flash->dev = dev;
+	flash->spi = slave;
 
 	/* Claim spi bus */
 	ret = spi_claim_bus(slave);
@@ -140,17 +137,26 @@ int spi_flash_std_probe(struct udevice *dev)
 		return ret;
 	}
 
-	ret = spi_flash_scan(slave, flash);
+	ret = spi_flash_scan(flash);
 	if (ret) {
 		ret = -EINVAL;
-		goto err_read_id;
+		goto err_scan;
 	}
 
 #ifdef CONFIG_SPI_FLASH_MTD
 	ret = spi_flash_mtd_register(flash);
+	if (ret) {
+		printf("SF: failed to register mtd device: %d\n", ret);
+		goto err_mtd;
+	}
 #endif
+	return ret;
 
-err_read_id:
+#ifdef CONFIG_SPI_FLASH_MTD
+err_mtd:
+	spi_free_slave(slave);
+#endif
+err_scan:
 	spi_release_bus(slave);
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
                   ` (20 preceding siblings ...)
  2015-10-12 15:01 ` [U-Boot] [PATCH v4 21/21] dm-sf: Re-factorize spi_flash_std_probe code Jagan Teki
@ 2015-10-18 20:27 ` Simon Glass
  2015-10-19  9:28   ` Jagan Teki
  21 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-10-18 20:27 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
> Previous version link:
> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>
> spi-flash layer need to tune a lot for better code handling and
> to sync with Linux spi-nor. So below areas got updated in this series.
> - BAR handling
> - spi_flash_cmd_wait_ready updates.
> - Separate core spi-flash handling and spi-flash interface
>   (interface between spi drivers vs spi-flash layer)
>
> Currently I'm working on spi-nor framework for u-boot which
> is slighly same as Linux spi-nor core with addition of
> u-boot driver model to it.
>
> This series will be starting point to add spi-nor functionalities.
>
> TODO:
> - MTD core addition to spi-flash layer.
> - spi-nor core addition.
>
> Code sizes:
> After:
> dm:
>    text    data     bss     dec     hex filename
>  354820   12016  221112  587948   8f8ac u-boot
> non-dm
>    text    data     bss     dec     hex filename
>  354317   11876  221124  587317   8f635 u-boot
>
> Before:
> dm
>    text    data     bss     dec     hex filename
>  354878   12016  221096  587990   8f8d6 u-boot
> non-dm
>    text    data     bss     dec     hex filename
>  354447   11876  221124  587447   8f6b7 u-boot

I don't think you should be adding new features to the
non-driver-model SPI flash code. We are supposed to be migrating
everything to driver model, so it would be better to move your boards
over, and then work to deprecate and remove the old code. Adding new
features to it sends the wrong message.

Sorry if I am misunderstanding your intent here.

>
> Testing:
> $ git clone git://git.denx.de/u-boot-spi.git
> $ cd u-boot-spi
> $ git checkout -b spi-nor-tune origin/next-spi-nor-tune
>
> thanks!
> Jagan.
>
> Jagan Teki (21):
>   spi: zynq_spi: Remove unneeded headers
>   sf: Return bank_sel, if flash->bank_curr == bank_sel
>   sf: Add 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: spi_flash_validate_params => spi_flash_scan
>   sf: Move spi_flash_scan code to sf_ops
>   sf: Move the read_id code to sf_ops
>   sf: Move BAR defined code at once place
>   sf: Use static for file-scope functions
>   sf: Fix Makefile
>   sf: Use simple name for register access functions
>   sf: Use flash function pointers in dm_spi_flash_ops
>   sf: Flash power up read-only based on idcode0
>   sf: Use static for file-scope functions
>   sf: Remove unneeded header includes
>   sf: probe: Use spi_flash_scan in dm-spi-flash
>   sf: Re-factorize spi_flash_probe_tail code
>   dm-sf: Re-factorize spi_flash_std_probe code
>
>  drivers/mtd/spi/Makefile      |   6 +-
>  drivers/mtd/spi/sf_internal.h |  57 ++---
>  drivers/mtd/spi/sf_ops.c      | 488 +++++++++++++++++++++++++++++++++++-------
>  drivers/mtd/spi/sf_probe.c    | 446 ++++++--------------------------------
>  drivers/spi/zynq_spi.c        |   6 +-
>  include/spi_flash.h           |  19 +-
>  6 files changed, 494 insertions(+), 528 deletions(-)
>
> --
> 1.9.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-18 20:27 ` [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Simon Glass
@ 2015-10-19  9:28   ` Jagan Teki
  2015-10-28 18:47     ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-19  9:28 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
>> Previous version link:
>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>>
>> spi-flash layer need to tune a lot for better code handling and
>> to sync with Linux spi-nor. So below areas got updated in this series.
>> - BAR handling
>> - spi_flash_cmd_wait_ready updates.
>> - Separate core spi-flash handling and spi-flash interface
>>   (interface between spi drivers vs spi-flash layer)
>>
>> Currently I'm working on spi-nor framework for u-boot which
>> is slighly same as Linux spi-nor core with addition of
>> u-boot driver model to it.
>>
>> This series will be starting point to add spi-nor functionalities.
>>
>> TODO:
>> - MTD core addition to spi-flash layer.
>> - spi-nor core addition.
>>
>> Code sizes:
>> After:
>> dm:
>>    text    data     bss     dec     hex filename
>>  354820   12016  221112  587948   8f8ac u-boot
>> non-dm
>>    text    data     bss     dec     hex filename
>>  354317   11876  221124  587317   8f635 u-boot
>>
>> Before:
>> dm
>>    text    data     bss     dec     hex filename
>>  354878   12016  221096  587990   8f8d6 u-boot
>> non-dm
>>    text    data     bss     dec     hex filename
>>  354447   11876  221124  587447   8f6b7 u-boot
>
> I don't think you should be adding new features to the
> non-driver-model SPI flash code. We are supposed to be migrating
> everything to driver model, so it would be better to move your boards
> over, and then work to deprecate and remove the old code. Adding new
> features to it sends the wrong message.

spi-flash core code doesn't require to add driver model, and cmd_sf to
spi-flash code is already supporting driver model.

OK, let me explain in-detail.

Code in sf_probe.c supports both dm and non dm-spi-flash and flash
initialization code using
spi_flash_validate_params. sf.c acts as interface between spi drivers
vs spi-flash code.
So the spi-flash initialization code(part of sf_probe) and code in
sf_ops are commonly categorized as spi-flash core code and this will
not require driver model, so-that the dm drivers will simply use this
common code for spi-flash core functionality.

This patch series will separate all the necessary existing code into
core and spi-flash vs spi drivers interface code. So at ends
- sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
so this will acts a spi-flash vs spi drivers interface. (which has dm
and non-dm as same as before)
- sf_ops is core spi-flash functionality.

On top of this I'm adding actual spi-nor core code, where sf_ops.c
will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
- spi-nor.c: Core SPI NOR
- spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
non-dm as same as before)

The reason for adding this spi-nor is to move flash code from
spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
from spi drivers to spi-nor that will be in driver model.

I'm simply adding new core functionality with adding new drivers as
dm-driven, I don't think this will not effect/change the driver model
growth.

 View of spi-nor framework:

    -----------------------------------------------------
                        cmd_sf
    -----------------------------------------------------
                        spi_flash
    -----------------------------------------------------
                        MTD Core
    -----------------------------------------------------
                        sf-uclass
    -----------------------------------------------------
                        SPI-NOR
    -----------------------------------------------------
        spi-nor-flash           drivers/mtd/spi/*
    -----------------------------------------------------
        spi-uclass
    -----------------------------------------------------
        drivers/spi/*
    -----------------------------------------------------

drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)

Please let me know for any more comments.

>
> Sorry if I am misunderstanding your intent here.
>
>>
>> Testing:
>> $ git clone git://git.denx.de/u-boot-spi.git
>> $ cd u-boot-spi
>> $ git checkout -b spi-nor-tune origin/next-spi-nor-tune
>>
>> thanks!
>> Jagan.
>>
>> Jagan Teki (21):
>>   spi: zynq_spi: Remove unneeded headers
>>   sf: Return bank_sel, if flash->bank_curr == bank_sel
>>   sf: Add 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: spi_flash_validate_params => spi_flash_scan
>>   sf: Move spi_flash_scan code to sf_ops
>>   sf: Move the read_id code to sf_ops
>>   sf: Move BAR defined code at once place
>>   sf: Use static for file-scope functions
>>   sf: Fix Makefile
>>   sf: Use simple name for register access functions
>>   sf: Use flash function pointers in dm_spi_flash_ops
>>   sf: Flash power up read-only based on idcode0
>>   sf: Use static for file-scope functions
>>   sf: Remove unneeded header includes
>>   sf: probe: Use spi_flash_scan in dm-spi-flash
>>   sf: Re-factorize spi_flash_probe_tail code
>>   dm-sf: Re-factorize spi_flash_std_probe code
>>
>>  drivers/mtd/spi/Makefile      |   6 +-
>>  drivers/mtd/spi/sf_internal.h |  57 ++---
>>  drivers/mtd/spi/sf_ops.c      | 488 +++++++++++++++++++++++++++++++++++-------
>>  drivers/mtd/spi/sf_probe.c    | 446 ++++++--------------------------------
>>  drivers/spi/zynq_spi.c        |   6 +-
>>  include/spi_flash.h           |  19 +-
>>  6 files changed, 494 insertions(+), 528 deletions(-)
>>
>> --
>> 1.9.1

--  Jagan.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-19  9:28   ` Jagan Teki
@ 2015-10-28 18:47     ` Simon Glass
  2015-10-28 19:08       ` Jagan Teki
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-10-28 18:47 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
> Hi Simon,
>
> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
>> Hi Jagan,
>>
>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
>>> Previous version link:
>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>>>
>>> spi-flash layer need to tune a lot for better code handling and
>>> to sync with Linux spi-nor. So below areas got updated in this series.
>>> - BAR handling
>>> - spi_flash_cmd_wait_ready updates.
>>> - Separate core spi-flash handling and spi-flash interface
>>>   (interface between spi drivers vs spi-flash layer)
>>>
>>> Currently I'm working on spi-nor framework for u-boot which
>>> is slighly same as Linux spi-nor core with addition of
>>> u-boot driver model to it.
>>>
>>> This series will be starting point to add spi-nor functionalities.
>>>
>>> TODO:
>>> - MTD core addition to spi-flash layer.
>>> - spi-nor core addition.
>>>
>>> Code sizes:
>>> After:
>>> dm:
>>>    text    data     bss     dec     hex filename
>>>  354820   12016  221112  587948   8f8ac u-boot
>>> non-dm
>>>    text    data     bss     dec     hex filename
>>>  354317   11876  221124  587317   8f635 u-boot
>>>
>>> Before:
>>> dm
>>>    text    data     bss     dec     hex filename
>>>  354878   12016  221096  587990   8f8d6 u-boot
>>> non-dm
>>>    text    data     bss     dec     hex filename
>>>  354447   11876  221124  587447   8f6b7 u-boot
>>
>> I don't think you should be adding new features to the
>> non-driver-model SPI flash code. We are supposed to be migrating
>> everything to driver model, so it would be better to move your boards
>> over, and then work to deprecate and remove the old code. Adding new
>> features to it sends the wrong message.
>
> spi-flash core code doesn't require to add driver model, and cmd_sf to
> spi-flash code is already supporting driver model.
>
> OK, let me explain in-detail.
>
> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
> initialization code using
> spi_flash_validate_params. sf.c acts as interface between spi drivers
> vs spi-flash code.
> So the spi-flash initialization code(part of sf_probe) and code in
> sf_ops are commonly categorized as spi-flash core code and this will
> not require driver model, so-that the dm drivers will simply use this
> common code for spi-flash core functionality.
>
> This patch series will separate all the necessary existing code into
> core and spi-flash vs spi drivers interface code. So at ends
> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
> so this will acts a spi-flash vs spi drivers interface. (which has dm
> and non-dm as same as before)
> - sf_ops is core spi-flash functionality.
>
> On top of this I'm adding actual spi-nor core code, where sf_ops.c
> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
> - spi-nor.c: Core SPI NOR
> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
> non-dm as same as before)
>
> The reason for adding this spi-nor is to move flash code from
> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
> from spi drivers to spi-nor that will be in driver model.
>
> I'm simply adding new core functionality with adding new drivers as
> dm-driven, I don't think this will not effect/change the driver model
> growth.
>
>  View of spi-nor framework:
>
>     -----------------------------------------------------
>                         cmd_sf
>     -----------------------------------------------------
>                         spi_flash
>     -----------------------------------------------------
>                         MTD Core
>     -----------------------------------------------------
>                         sf-uclass
>     -----------------------------------------------------
>                         SPI-NOR
>     -----------------------------------------------------
>         spi-nor-flash           drivers/mtd/spi/*
>     -----------------------------------------------------
>         spi-uclass
>     -----------------------------------------------------
>         drivers/spi/*
>     -----------------------------------------------------
>
> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
> drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)
>
> Please let me know for any more comments.

Perhaps another way of asking this is, what is the plan to remove the
non-DM code from SF or at least stop adding new features to it.

>
>>
>> Sorry if I am misunderstanding your intent here.
>>
>>>
>>> Testing:
>>> $ git clone git://git.denx.de/u-boot-spi.git
>>> $ cd u-boot-spi
>>> $ git checkout -b spi-nor-tune origin/next-spi-nor-tune
>>>
>>> thanks!
>>> Jagan.
>>>
>>> Jagan Teki (21):
>>>   spi: zynq_spi: Remove unneeded headers
>>>   sf: Return bank_sel, if flash->bank_curr == bank_sel
>>>   sf: Add 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: spi_flash_validate_params => spi_flash_scan
>>>   sf: Move spi_flash_scan code to sf_ops
>>>   sf: Move the read_id code to sf_ops
>>>   sf: Move BAR defined code at once place
>>>   sf: Use static for file-scope functions
>>>   sf: Fix Makefile
>>>   sf: Use simple name for register access functions
>>>   sf: Use flash function pointers in dm_spi_flash_ops
>>>   sf: Flash power up read-only based on idcode0
>>>   sf: Use static for file-scope functions
>>>   sf: Remove unneeded header includes
>>>   sf: probe: Use spi_flash_scan in dm-spi-flash
>>>   sf: Re-factorize spi_flash_probe_tail code
>>>   dm-sf: Re-factorize spi_flash_std_probe code
>>>
>>>  drivers/mtd/spi/Makefile      |   6 +-
>>>  drivers/mtd/spi/sf_internal.h |  57 ++---
>>>  drivers/mtd/spi/sf_ops.c      | 488 +++++++++++++++++++++++++++++++++++-------
>>>  drivers/mtd/spi/sf_probe.c    | 446 ++++++--------------------------------
>>>  drivers/spi/zynq_spi.c        |   6 +-
>>>  include/spi_flash.h           |  19 +-
>>>  6 files changed, 494 insertions(+), 528 deletions(-)
>>>
>>> --
>>> 1.9.1
>
> --  Jagan.


Regards,
Simon

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-28 18:47     ` Simon Glass
@ 2015-10-28 19:08       ` Jagan Teki
  2015-10-29 23:25         ` Simon Glass
  0 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-28 19:08 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 29 October 2015 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
>> Hi Simon,
>>
>> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Jagan,
>>>
>>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
>>>> Previous version link:
>>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>>>>
>>>> spi-flash layer need to tune a lot for better code handling and
>>>> to sync with Linux spi-nor. So below areas got updated in this series.
>>>> - BAR handling
>>>> - spi_flash_cmd_wait_ready updates.
>>>> - Separate core spi-flash handling and spi-flash interface
>>>>   (interface between spi drivers vs spi-flash layer)
>>>>
>>>> Currently I'm working on spi-nor framework for u-boot which
>>>> is slighly same as Linux spi-nor core with addition of
>>>> u-boot driver model to it.
>>>>
>>>> This series will be starting point to add spi-nor functionalities.
>>>>
>>>> TODO:
>>>> - MTD core addition to spi-flash layer.
>>>> - spi-nor core addition.
>>>>
>>>> Code sizes:
>>>> After:
>>>> dm:
>>>>    text    data     bss     dec     hex filename
>>>>  354820   12016  221112  587948   8f8ac u-boot
>>>> non-dm
>>>>    text    data     bss     dec     hex filename
>>>>  354317   11876  221124  587317   8f635 u-boot
>>>>
>>>> Before:
>>>> dm
>>>>    text    data     bss     dec     hex filename
>>>>  354878   12016  221096  587990   8f8d6 u-boot
>>>> non-dm
>>>>    text    data     bss     dec     hex filename
>>>>  354447   11876  221124  587447   8f6b7 u-boot
>>>
>>> I don't think you should be adding new features to the
>>> non-driver-model SPI flash code. We are supposed to be migrating
>>> everything to driver model, so it would be better to move your boards
>>> over, and then work to deprecate and remove the old code. Adding new
>>> features to it sends the wrong message.
>>
>> spi-flash core code doesn't require to add driver model, and cmd_sf to
>> spi-flash code is already supporting driver model.
>>
>> OK, let me explain in-detail.
>>
>> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
>> initialization code using
>> spi_flash_validate_params. sf.c acts as interface between spi drivers
>> vs spi-flash code.
>> So the spi-flash initialization code(part of sf_probe) and code in
>> sf_ops are commonly categorized as spi-flash core code and this will
>> not require driver model, so-that the dm drivers will simply use this
>> common code for spi-flash core functionality.
>>
>> This patch series will separate all the necessary existing code into
>> core and spi-flash vs spi drivers interface code. So at ends
>> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
>> so this will acts a spi-flash vs spi drivers interface. (which has dm
>> and non-dm as same as before)
>> - sf_ops is core spi-flash functionality.
>>
>> On top of this I'm adding actual spi-nor core code, where sf_ops.c
>> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
>> - spi-nor.c: Core SPI NOR
>> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
>> non-dm as same as before)
>>
>> The reason for adding this spi-nor is to move flash code from
>> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
>> from spi drivers to spi-nor that will be in driver model.
>>
>> I'm simply adding new core functionality with adding new drivers as
>> dm-driven, I don't think this will not effect/change the driver model
>> growth.
>>
>>  View of spi-nor framework:
>>
>>     -----------------------------------------------------
>>                         cmd_sf
>>     -----------------------------------------------------
>>                         spi_flash
>>     -----------------------------------------------------
>>                         MTD Core
>>     -----------------------------------------------------
>>                         sf-uclass
>>     -----------------------------------------------------
>>                         SPI-NOR
>>     -----------------------------------------------------
>>         spi-nor-flash           drivers/mtd/spi/*
>>     -----------------------------------------------------
>>         spi-uclass
>>     -----------------------------------------------------
>>         drivers/spi/*
>>     -----------------------------------------------------
>>
>> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
>> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
>> drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)
>>
>> Please let me know for any more comments.
>
> Perhaps another way of asking this is, what is the plan to remove the
> non-DM code from SF or at least stop adding new features to it.

Sorry I didn't understand "remove non-dm code" or either I missed
something here.

The plan is not to remove any code intentionally it's about following
feature additions
1) Tuning up spi-flash framework: Separating Core spi-flash code and
interface code between spi-flash vs spi drivers
2) Adding MTD core support to spi-flash core (no spi_flash ops -
mtd_ops will use)
3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
spi-nor controller drivers are part of this like fsl_qspi or etc.
spi-nor controllers and interface code between spi-flash vs
spi-drivers become UCLASS_SPI_NOR

Agenda is to add SPI-NOR framework(almost similar to Linux) with
driver model(as UCLASS_SPI_NOR)

-----------------------------------------------------------------------------------------------
      cmd_spi                                      cmd_sf
-------|----------------------------------------------------------------------------------------
        |                                              spi_flash
-------|----------------------------------------------------------------------------------------
        |                                             MTD Core
-------|----------------------------------------------------------------------------------------
        |                                           spi-nor-uclass
-------|----------------------------------------------------------------------------------------
        |                                            SPI-NOR Core (spi-nor.c)
-------|-----------------------------------------------------------------------------------------
        |          |=========spi-nor-flash
drivers/mtd/spi/fsl_qspi
         |          |                   (m25p80.c)
              (fsl-quadspi.c)
--------|-------V-----------------------------------------------------------------------------------
         spi-uclass
--------------------------------------------------------------------------------------------------
         drivers/spi/*
 -----------------------------------------------------

Let me know for any more comments?

>
>>
>>>
>>> Sorry if I am misunderstanding your intent here.
>>>
>>>>
>>>> Testing:
>>>> $ git clone git://git.denx.de/u-boot-spi.git
>>>> $ cd u-boot-spi
>>>> $ git checkout -b spi-nor-tune origin/next-spi-nor-tune

-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-28 19:08       ` Jagan Teki
@ 2015-10-29 23:25         ` Simon Glass
  2015-10-30 20:02           ` Jagan Teki
  0 siblings, 1 reply; 41+ messages in thread
From: Simon Glass @ 2015-10-29 23:25 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On 28 October 2015 at 13:08, Jagan Teki <jteki@openedev.com> wrote:
>
> Hi Simon,
>
> On 29 October 2015 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> > Hi Jagan,
> >
> > On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
> >> Hi Simon,
> >>
> >> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
> >>> Hi Jagan,
> >>>
> >>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
> >>>> Previous version link:
> >>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
> >>>>
> >>>> spi-flash layer need to tune a lot for better code handling and
> >>>> to sync with Linux spi-nor. So below areas got updated in this series.
> >>>> - BAR handling
> >>>> - spi_flash_cmd_wait_ready updates.
> >>>> - Separate core spi-flash handling and spi-flash interface
> >>>>   (interface between spi drivers vs spi-flash layer)
> >>>>
> >>>> Currently I'm working on spi-nor framework for u-boot which
> >>>> is slighly same as Linux spi-nor core with addition of
> >>>> u-boot driver model to it.
> >>>>
> >>>> This series will be starting point to add spi-nor functionalities.
> >>>>
> >>>> TODO:
> >>>> - MTD core addition to spi-flash layer.
> >>>> - spi-nor core addition.
> >>>>
> >>>> Code sizes:
> >>>> After:
> >>>> dm:
> >>>>    text    data     bss     dec     hex filename
> >>>>  354820   12016  221112  587948   8f8ac u-boot
> >>>> non-dm
> >>>>    text    data     bss     dec     hex filename
> >>>>  354317   11876  221124  587317   8f635 u-boot
> >>>>
> >>>> Before:
> >>>> dm
> >>>>    text    data     bss     dec     hex filename
> >>>>  354878   12016  221096  587990   8f8d6 u-boot
> >>>> non-dm
> >>>>    text    data     bss     dec     hex filename
> >>>>  354447   11876  221124  587447   8f6b7 u-boot
> >>>
> >>> I don't think you should be adding new features to the
> >>> non-driver-model SPI flash code. We are supposed to be migrating
> >>> everything to driver model, so it would be better to move your boards
> >>> over, and then work to deprecate and remove the old code. Adding new
> >>> features to it sends the wrong message.
> >>
> >> spi-flash core code doesn't require to add driver model, and cmd_sf to
> >> spi-flash code is already supporting driver model.
> >>
> >> OK, let me explain in-detail.
> >>
> >> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
> >> initialization code using
> >> spi_flash_validate_params. sf.c acts as interface between spi drivers
> >> vs spi-flash code.
> >> So the spi-flash initialization code(part of sf_probe) and code in
> >> sf_ops are commonly categorized as spi-flash core code and this will
> >> not require driver model, so-that the dm drivers will simply use this
> >> common code for spi-flash core functionality.
> >>
> >> This patch series will separate all the necessary existing code into
> >> core and spi-flash vs spi drivers interface code. So at ends
> >> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
> >> so this will acts a spi-flash vs spi drivers interface. (which has dm
> >> and non-dm as same as before)
> >> - sf_ops is core spi-flash functionality.
> >>
> >> On top of this I'm adding actual spi-nor core code, where sf_ops.c
> >> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
> >> - spi-nor.c: Core SPI NOR
> >> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
> >> non-dm as same as before)
> >>
> >> The reason for adding this spi-nor is to move flash code from
> >> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
> >> from spi drivers to spi-nor that will be in driver model.
> >>
> >> I'm simply adding new core functionality with adding new drivers as
> >> dm-driven, I don't think this will not effect/change the driver model
> >> growth.
> >>
> >>  View of spi-nor framework:
> >>
> >>     -----------------------------------------------------
> >>                         cmd_sf
> >>     -----------------------------------------------------
> >>                         spi_flash
> >>     -----------------------------------------------------
> >>                         MTD Core
> >>     -----------------------------------------------------
> >>                         sf-uclass
> >>     -----------------------------------------------------
> >>                         SPI-NOR
> >>     -----------------------------------------------------
> >>         spi-nor-flash           drivers/mtd/spi/*
> >>     -----------------------------------------------------
> >>         spi-uclass
> >>     -----------------------------------------------------
> >>         drivers/spi/*
> >>     -----------------------------------------------------
> >>
> >> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
> >> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
> >> drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)
> >>
> >> Please let me know for any more comments.
> >
> > Perhaps another way of asking this is, what is the plan to remove the
> > non-DM code from SF or at least stop adding new features to it.
>
> Sorry I didn't understand "remove non-dm code" or either I missed
> something here.
>
> The plan is not to remove any code intentionally it's about following
> feature additions
> 1) Tuning up spi-flash framework: Separating Core spi-flash code and
> interface code between spi-flash vs spi drivers
> 2) Adding MTD core support to spi-flash core (no spi_flash ops -
> mtd_ops will use)
> 3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
> spi-nor controller drivers are part of this like fsl_qspi or etc.
> spi-nor controllers and interface code between spi-flash vs
> spi-drivers become UCLASS_SPI_NOR
>
> Agenda is to add SPI-NOR framework(almost similar to Linux) with
> driver model(as UCLASS_SPI_NOR)
>
> -----------------------------------------------------------------------------------------------
>       cmd_spi                                      cmd_sf
> -------|----------------------------------------------------------------------------------------
>         |                                              spi_flash
> -------|----------------------------------------------------------------------------------------
>         |                                             MTD Core
> -------|----------------------------------------------------------------------------------------
>         |                                           spi-nor-uclass
> -------|----------------------------------------------------------------------------------------
>         |                                            SPI-NOR Core (spi-nor.c)
> -------|-----------------------------------------------------------------------------------------
>         |          |=========spi-nor-flash
> drivers/mtd/spi/fsl_qspi
>          |          |                   (m25p80.c)
>               (fsl-quadspi.c)
> --------|-------V-----------------------------------------------------------------------------------
>          spi-uclass
> --------------------------------------------------------------------------------------------------
>          drivers/spi/*
>  -----------------------------------------------------
>
> Let me know for any more comments?

This is quite a big topic. Also you are the maintainer, not me. So I
don't want to interfere. It's just that I am conscious that my SPI
flash conversion was incomplete, in that:

- it still provides the old API (and we need to keep this until every
board migrates)
- it still uses struct spi_flash as a parameter, when it should really
use struct udevice
- the concept of chip select number and bus number are still present,
but in the end these should just be devices

So I'm keen to keep moving things in the direction of 'fully migrated'.

Adding a new feature like SPI nor seems like a good oppty to encourage
people to move over to driver model - .e.g if the new feature is only
available with driver model.

Regards,
Simon

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-29 23:25         ` Simon Glass
@ 2015-10-30 20:02           ` Jagan Teki
  2015-10-30 20:30             ` Marek Vasut
  0 siblings, 1 reply; 41+ messages in thread
From: Jagan Teki @ 2015-10-30 20:02 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 30 October 2015 at 04:55, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagan,
>
> On 28 October 2015 at 13:08, Jagan Teki <jteki@openedev.com> wrote:
>>
>> Hi Simon,
>>
>> On 29 October 2015 at 00:17, Simon Glass <sjg@chromium.org> wrote:
>> > Hi Jagan,
>> >
>> > On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
>> >> Hi Simon,
>> >>
>> >> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
>> >>> Hi Jagan,
>> >>>
>> >>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
>> >>>> Previous version link:
>> >>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>> >>>>
>> >>>> spi-flash layer need to tune a lot for better code handling and
>> >>>> to sync with Linux spi-nor. So below areas got updated in this series.
>> >>>> - BAR handling
>> >>>> - spi_flash_cmd_wait_ready updates.
>> >>>> - Separate core spi-flash handling and spi-flash interface
>> >>>>   (interface between spi drivers vs spi-flash layer)
>> >>>>
>> >>>> Currently I'm working on spi-nor framework for u-boot which
>> >>>> is slighly same as Linux spi-nor core with addition of
>> >>>> u-boot driver model to it.
>> >>>>
>> >>>> This series will be starting point to add spi-nor functionalities.
>> >>>>
>> >>>> TODO:
>> >>>> - MTD core addition to spi-flash layer.
>> >>>> - spi-nor core addition.
>> >>>>
>> >>>> Code sizes:
>> >>>> After:
>> >>>> dm:
>> >>>>    text    data     bss     dec     hex filename
>> >>>>  354820   12016  221112  587948   8f8ac u-boot
>> >>>> non-dm
>> >>>>    text    data     bss     dec     hex filename
>> >>>>  354317   11876  221124  587317   8f635 u-boot
>> >>>>
>> >>>> Before:
>> >>>> dm
>> >>>>    text    data     bss     dec     hex filename
>> >>>>  354878   12016  221096  587990   8f8d6 u-boot
>> >>>> non-dm
>> >>>>    text    data     bss     dec     hex filename
>> >>>>  354447   11876  221124  587447   8f6b7 u-boot
>> >>>
>> >>> I don't think you should be adding new features to the
>> >>> non-driver-model SPI flash code. We are supposed to be migrating
>> >>> everything to driver model, so it would be better to move your boards
>> >>> over, and then work to deprecate and remove the old code. Adding new
>> >>> features to it sends the wrong message.
>> >>
>> >> spi-flash core code doesn't require to add driver model, and cmd_sf to
>> >> spi-flash code is already supporting driver model.
>> >>
>> >> OK, let me explain in-detail.
>> >>
>> >> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
>> >> initialization code using
>> >> spi_flash_validate_params. sf.c acts as interface between spi drivers
>> >> vs spi-flash code.
>> >> So the spi-flash initialization code(part of sf_probe) and code in
>> >> sf_ops are commonly categorized as spi-flash core code and this will
>> >> not require driver model, so-that the dm drivers will simply use this
>> >> common code for spi-flash core functionality.
>> >>
>> >> This patch series will separate all the necessary existing code into
>> >> core and spi-flash vs spi drivers interface code. So at ends
>> >> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash code
>> >> so this will acts a spi-flash vs spi drivers interface. (which has dm
>> >> and non-dm as same as before)
>> >> - sf_ops is core spi-flash functionality.
>> >>
>> >> On top of this I'm adding actual spi-nor core code, where sf_ops.c
>> >> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
>> >> - spi-nor.c: Core SPI NOR
>> >> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
>> >> non-dm as same as before)
>> >>
>> >> The reason for adding this spi-nor is to move flash code from
>> >> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
>> >> from spi drivers to spi-nor that will be in driver model.
>> >>
>> >> I'm simply adding new core functionality with adding new drivers as
>> >> dm-driven, I don't think this will not effect/change the driver model
>> >> growth.
>> >>
>> >>  View of spi-nor framework:
>> >>
>> >>     -----------------------------------------------------
>> >>                         cmd_sf
>> >>     -----------------------------------------------------
>> >>                         spi_flash
>> >>     -----------------------------------------------------
>> >>                         MTD Core
>> >>     -----------------------------------------------------
>> >>                         sf-uclass
>> >>     -----------------------------------------------------
>> >>                         SPI-NOR
>> >>     -----------------------------------------------------
>> >>         spi-nor-flash           drivers/mtd/spi/*
>> >>     -----------------------------------------------------
>> >>         spi-uclass
>> >>     -----------------------------------------------------
>> >>         drivers/spi/*
>> >>     -----------------------------------------------------
>> >>
>> >> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
>> >> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface (dm-driven)
>> >> drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver (dm-driven)
>> >>
>> >> Please let me know for any more comments.
>> >
>> > Perhaps another way of asking this is, what is the plan to remove the
>> > non-DM code from SF or at least stop adding new features to it.
>>
>> Sorry I didn't understand "remove non-dm code" or either I missed
>> something here.
>>
>> The plan is not to remove any code intentionally it's about following
>> feature additions
>> 1) Tuning up spi-flash framework: Separating Core spi-flash code and
>> interface code between spi-flash vs spi drivers
>> 2) Adding MTD core support to spi-flash core (no spi_flash ops -
>> mtd_ops will use)
>> 3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
>> spi-nor controller drivers are part of this like fsl_qspi or etc.
>> spi-nor controllers and interface code between spi-flash vs
>> spi-drivers become UCLASS_SPI_NOR
>>
>> Agenda is to add SPI-NOR framework(almost similar to Linux) with
>> driver model(as UCLASS_SPI_NOR)
>>
>> -----------------------------------------------------------------------------------------------
>>       cmd_spi                                      cmd_sf
>> -------|----------------------------------------------------------------------------------------
>>         |                                              spi_flash
>> -------|----------------------------------------------------------------------------------------
>>         |                                             MTD Core
>> -------|----------------------------------------------------------------------------------------
>>         |                                           spi-nor-uclass
>> -------|----------------------------------------------------------------------------------------
>>         |                                            SPI-NOR Core (spi-nor.c)
>> -------|-----------------------------------------------------------------------------------------
>>         |          |=========spi-nor-flash
>> drivers/mtd/spi/fsl_qspi
>>          |          |                   (m25p80.c)
>>               (fsl-quadspi.c)
>> --------|-------V-----------------------------------------------------------------------------------
>>          spi-uclass
>> --------------------------------------------------------------------------------------------------
>>          drivers/spi/*
>>  -----------------------------------------------------
>>
>> Let me know for any more comments?
>
> This is quite a big topic. Also you are the maintainer, not me. So I
> don't want to interfere. It's just that I am conscious that my SPI
> flash conversion was incomplete, in that:
>
> - it still provides the old API (and we need to keep this until every
> board migrates)
> - it still uses struct spi_flash as a parameter, when it should really
> use struct udevice
> - the concept of chip select number and bus number are still present,
> but in the end these should just be devices
>
> So I'm keen to keep moving things in the direction of 'fully migrated'.
>
> Adding a new feature like SPI nor seems like a good oppty to encourage
> people to move over to driver model - .e.g if the new feature is only
> available with driver model.

Just park this spi-nor stuff, once I send all patches will discuss more.

This series is for code re-factorization like separating spi-flash
core functionality vs spi-driver interface from spi-flash core. Did
you find any wrong on this?

thanks!
-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-30 20:02           ` Jagan Teki
@ 2015-10-30 20:30             ` Marek Vasut
  2015-10-30 20:33               ` Jagan Teki
  0 siblings, 1 reply; 41+ messages in thread
From: Marek Vasut @ 2015-10-30 20:30 UTC (permalink / raw)
  To: u-boot

On Friday, October 30, 2015 at 09:02:15 PM, Jagan Teki wrote:
> Hi Simon,
> 
> On 30 October 2015 at 04:55, Simon Glass <sjg@chromium.org> wrote:
> > Hi Jagan,
> > 
> > On 28 October 2015 at 13:08, Jagan Teki <jteki@openedev.com> wrote:
> >> Hi Simon,
> >> 
> >> On 29 October 2015 at 00:17, Simon Glass <sjg@chromium.org> wrote:
> >> > Hi Jagan,
> >> > 
> >> > On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
> >> >> Hi Simon,
> >> >> 
> >> >> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
> >> >>> Hi Jagan,
> >> >>> 
> >> >>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
> >> >>>> Previous version link:
> >> >>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
> >> >>>> 
> >> >>>> spi-flash layer need to tune a lot for better code handling and
> >> >>>> to sync with Linux spi-nor. So below areas got updated in this
> >> >>>> series. - BAR handling
> >> >>>> - spi_flash_cmd_wait_ready updates.
> >> >>>> - Separate core spi-flash handling and spi-flash interface
> >> >>>> 
> >> >>>>   (interface between spi drivers vs spi-flash layer)
> >> >>>> 
> >> >>>> Currently I'm working on spi-nor framework for u-boot which
> >> >>>> is slighly same as Linux spi-nor core with addition of
> >> >>>> u-boot driver model to it.
> >> >>>> 
> >> >>>> This series will be starting point to add spi-nor functionalities.
> >> >>>> 
> >> >>>> TODO:
> >> >>>> - MTD core addition to spi-flash layer.
> >> >>>> - spi-nor core addition.
> >> >>>> 
> >> >>>> Code sizes:
> >> >>>> After:
> >> >>>> 
> >> >>>> dm:
> >> >>>>    text    data     bss     dec     hex filename
> >> >>>>  
> >> >>>>  354820   12016  221112  587948   8f8ac u-boot
> >> >>>> 
> >> >>>> non-dm
> >> >>>> 
> >> >>>>    text    data     bss     dec     hex filename
> >> >>>>  
> >> >>>>  354317   11876  221124  587317   8f635 u-boot
> >> >>>> 
> >> >>>> Before:
> >> >>>> dm
> >> >>>> 
> >> >>>>    text    data     bss     dec     hex filename
> >> >>>>  
> >> >>>>  354878   12016  221096  587990   8f8d6 u-boot
> >> >>>> 
> >> >>>> non-dm
> >> >>>> 
> >> >>>>    text    data     bss     dec     hex filename
> >> >>>>  
> >> >>>>  354447   11876  221124  587447   8f6b7 u-boot
> >> >>> 
> >> >>> I don't think you should be adding new features to the
> >> >>> non-driver-model SPI flash code. We are supposed to be migrating
> >> >>> everything to driver model, so it would be better to move your
> >> >>> boards over, and then work to deprecate and remove the old code.
> >> >>> Adding new features to it sends the wrong message.
> >> >> 
> >> >> spi-flash core code doesn't require to add driver model, and cmd_sf
> >> >> to spi-flash code is already supporting driver model.
> >> >> 
> >> >> OK, let me explain in-detail.
> >> >> 
> >> >> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
> >> >> initialization code using
> >> >> spi_flash_validate_params. sf.c acts as interface between spi drivers
> >> >> vs spi-flash code.
> >> >> So the spi-flash initialization code(part of sf_probe) and code in
> >> >> sf_ops are commonly categorized as spi-flash core code and this will
> >> >> not require driver model, so-that the dm drivers will simply use this
> >> >> common code for spi-flash core functionality.
> >> >> 
> >> >> This patch series will separate all the necessary existing code into
> >> >> core and spi-flash vs spi drivers interface code. So at ends
> >> >> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash
> >> >> code so this will acts a spi-flash vs spi drivers interface. (which
> >> >> has dm and non-dm as same as before)
> >> >> - sf_ops is core spi-flash functionality.
> >> >> 
> >> >> On top of this I'm adding actual spi-nor core code, where sf_ops.c
> >> >> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
> >> >> - spi-nor.c: Core SPI NOR
> >> >> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
> >> >> non-dm as same as before)
> >> >> 
> >> >> The reason for adding this spi-nor is to move flash code from
> >> >> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
> >> >> from spi drivers to spi-nor that will be in driver model.
> >> >> 
> >> >> I'm simply adding new core functionality with adding new drivers as
> >> >> dm-driven, I don't think this will not effect/change the driver model
> >> >> growth.
> >> >> 
> >> >>  View of spi-nor framework:
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>                         cmd_sf
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>                         spi_flash
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>                         MTD Core
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>                         sf-uclass
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>                         SPI-NOR
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>         spi-nor-flash           drivers/mtd/spi/*
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>         spi-uclass
> >> >>     
> >> >>     -----------------------------------------------------
> >> >>     
> >> >>         drivers/spi/*
> >> >>     
> >> >>     -----------------------------------------------------
> >> >> 
> >> >> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
> >> >> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface
> >> >> (dm-driven) drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver
> >> >> (dm-driven)
> >> >> 
> >> >> Please let me know for any more comments.
> >> > 
> >> > Perhaps another way of asking this is, what is the plan to remove the
> >> > non-DM code from SF or at least stop adding new features to it.
> >> 
> >> Sorry I didn't understand "remove non-dm code" or either I missed
> >> something here.
> >> 
> >> The plan is not to remove any code intentionally it's about following
> >> feature additions
> >> 1) Tuning up spi-flash framework: Separating Core spi-flash code and
> >> interface code between spi-flash vs spi drivers
> >> 2) Adding MTD core support to spi-flash core (no spi_flash ops -
> >> mtd_ops will use)
> >> 3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
> >> spi-nor controller drivers are part of this like fsl_qspi or etc.
> >> spi-nor controllers and interface code between spi-flash vs
> >> spi-drivers become UCLASS_SPI_NOR
> >> 
> >> Agenda is to add SPI-NOR framework(almost similar to Linux) with
> >> driver model(as UCLASS_SPI_NOR)
> >> 
> >> ------------------------------------------------------------------------
> >> -----------------------
> >> 
> >>       cmd_spi                                      cmd_sf
> >> 
> >> -------|----------------------------------------------------------------
> >> ------------------------
> >> 
> >>         |                                              spi_flash
> >> 
> >> -------|----------------------------------------------------------------
> >> ------------------------
> >> 
> >>         |                                             MTD Core
> >> 
> >> -------|----------------------------------------------------------------
> >> ------------------------
> >> 
> >>         |                                           spi-nor-uclass
> >> 
> >> -------|----------------------------------------------------------------
> >> ------------------------
> >> 
> >>         |                                            SPI-NOR Core
> >>         |                                            (spi-nor.c)
> >> 
> >> -------|----------------------------------------------------------------
> >> -------------------------
> >> 
> >>         |          |=========spi-nor-flash
> >> 
> >> drivers/mtd/spi/fsl_qspi
> >> 
> >>          |          |                   (m25p80.c)
> >>               
> >>               (fsl-quadspi.c)
> >> 
> >> --------|-------V-------------------------------------------------------
> >> ----------------------------
> >> 
> >>          spi-uclass
> >> 
> >> ------------------------------------------------------------------------
> >> --------------------------
> >> 
> >>          drivers/spi/*
> >>  
> >>  -----------------------------------------------------
> >> 
> >> Let me know for any more comments?
> > 
> > This is quite a big topic. Also you are the maintainer, not me. So I
> > don't want to interfere. It's just that I am conscious that my SPI
> > flash conversion was incomplete, in that:
> > 
> > - it still provides the old API (and we need to keep this until every
> > board migrates)
> > - it still uses struct spi_flash as a parameter, when it should really
> > use struct udevice
> > - the concept of chip select number and bus number are still present,
> > but in the end these should just be devices
> > 
> > So I'm keen to keep moving things in the direction of 'fully migrated'.
> > 
> > Adding a new feature like SPI nor seems like a good oppty to encourage
> > people to move over to driver model - .e.g if the new feature is only
> > available with driver model.
> 
> Just park this spi-nor stuff, once I send all patches will discuss more.
> 
> This series is for code re-factorization like separating spi-flash
> core functionality vs spi-driver interface from spi-flash core. Did
> you find any wrong on this?

Why don't we converge toward Linux's spi-nor stack instead ?

^ permalink raw reply	[flat|nested] 41+ messages in thread

* [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer
  2015-10-30 20:30             ` Marek Vasut
@ 2015-10-30 20:33               ` Jagan Teki
  0 siblings, 0 replies; 41+ messages in thread
From: Jagan Teki @ 2015-10-30 20:33 UTC (permalink / raw)
  To: u-boot

On 31 October 2015 at 02:00, Marek Vasut <marex@denx.de> wrote:
> On Friday, October 30, 2015 at 09:02:15 PM, Jagan Teki wrote:
>> Hi Simon,
>>
>> On 30 October 2015 at 04:55, Simon Glass <sjg@chromium.org> wrote:
>> > Hi Jagan,
>> >
>> > On 28 October 2015 at 13:08, Jagan Teki <jteki@openedev.com> wrote:
>> >> Hi Simon,
>> >>
>> >> On 29 October 2015 at 00:17, Simon Glass <sjg@chromium.org> wrote:
>> >> > Hi Jagan,
>> >> >
>> >> > On 19 October 2015 at 03:28, Jagan Teki <jteki@openedev.com> wrote:
>> >> >> Hi Simon,
>> >> >>
>> >> >> On 19 October 2015 at 01:57, Simon Glass <sjg@chromium.org> wrote:
>> >> >>> Hi Jagan,
>> >> >>>
>> >> >>> On 12 October 2015 at 09:00, Jagan Teki <jteki@openedev.com> wrote:
>> >> >>>> Previous version link:
>> >> >>>> http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/233262
>> >> >>>>
>> >> >>>> spi-flash layer need to tune a lot for better code handling and
>> >> >>>> to sync with Linux spi-nor. So below areas got updated in this
>> >> >>>> series. - BAR handling
>> >> >>>> - spi_flash_cmd_wait_ready updates.
>> >> >>>> - Separate core spi-flash handling and spi-flash interface
>> >> >>>>
>> >> >>>>   (interface between spi drivers vs spi-flash layer)
>> >> >>>>
>> >> >>>> Currently I'm working on spi-nor framework for u-boot which
>> >> >>>> is slighly same as Linux spi-nor core with addition of
>> >> >>>> u-boot driver model to it.
>> >> >>>>
>> >> >>>> This series will be starting point to add spi-nor functionalities.
>> >> >>>>
>> >> >>>> TODO:
>> >> >>>> - MTD core addition to spi-flash layer.
>> >> >>>> - spi-nor core addition.
>> >> >>>>
>> >> >>>> Code sizes:
>> >> >>>> After:
>> >> >>>>
>> >> >>>> dm:
>> >> >>>>    text    data     bss     dec     hex filename
>> >> >>>>
>> >> >>>>  354820   12016  221112  587948   8f8ac u-boot
>> >> >>>>
>> >> >>>> non-dm
>> >> >>>>
>> >> >>>>    text    data     bss     dec     hex filename
>> >> >>>>
>> >> >>>>  354317   11876  221124  587317   8f635 u-boot
>> >> >>>>
>> >> >>>> Before:
>> >> >>>> dm
>> >> >>>>
>> >> >>>>    text    data     bss     dec     hex filename
>> >> >>>>
>> >> >>>>  354878   12016  221096  587990   8f8d6 u-boot
>> >> >>>>
>> >> >>>> non-dm
>> >> >>>>
>> >> >>>>    text    data     bss     dec     hex filename
>> >> >>>>
>> >> >>>>  354447   11876  221124  587447   8f6b7 u-boot
>> >> >>>
>> >> >>> I don't think you should be adding new features to the
>> >> >>> non-driver-model SPI flash code. We are supposed to be migrating
>> >> >>> everything to driver model, so it would be better to move your
>> >> >>> boards over, and then work to deprecate and remove the old code.
>> >> >>> Adding new features to it sends the wrong message.
>> >> >>
>> >> >> spi-flash core code doesn't require to add driver model, and cmd_sf
>> >> >> to spi-flash code is already supporting driver model.
>> >> >>
>> >> >> OK, let me explain in-detail.
>> >> >>
>> >> >> Code in sf_probe.c supports both dm and non dm-spi-flash and flash
>> >> >> initialization code using
>> >> >> spi_flash_validate_params. sf.c acts as interface between spi drivers
>> >> >> vs spi-flash code.
>> >> >> So the spi-flash initialization code(part of sf_probe) and code in
>> >> >> sf_ops are commonly categorized as spi-flash core code and this will
>> >> >> not require driver model, so-that the dm drivers will simply use this
>> >> >> common code for spi-flash core functionality.
>> >> >>
>> >> >> This patch series will separate all the necessary existing code into
>> >> >> core and spi-flash vs spi drivers interface code. So at ends
>> >> >> - sf_probe is simply the copy of sf.c and dm and non-dm spi-flash
>> >> >> code so this will acts a spi-flash vs spi drivers interface. (which
>> >> >> has dm and non-dm as same as before)
>> >> >> - sf_ops is core spi-flash functionality.
>> >> >>
>> >> >> On top of this I'm adding actual spi-nor core code, where sf_ops.c
>> >> >> will become spi-nor.c and sf_probe.c will become spi-nor-flash.c.
>> >> >> - spi-nor.c: Core SPI NOR
>> >> >> - spi-nor-flash: spi drivers vs spi-nor interface (which has dm and
>> >> >> non-dm as same as before)
>> >> >>
>> >> >> The reason for adding this spi-nor is to move flash code from
>> >> >> spi-drivers, example fsl_qspi and at the end this fsl_qspi will move
>> >> >> from spi drivers to spi-nor that will be in driver model.
>> >> >>
>> >> >> I'm simply adding new core functionality with adding new drivers as
>> >> >> dm-driven, I don't think this will not effect/change the driver model
>> >> >> growth.
>> >> >>
>> >> >>  View of spi-nor framework:
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>                         cmd_sf
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>                         spi_flash
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>                         MTD Core
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>                         sf-uclass
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>                         SPI-NOR
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>         spi-nor-flash           drivers/mtd/spi/*
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>         spi-uclass
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >>         drivers/spi/*
>> >> >>
>> >> >>     -----------------------------------------------------
>> >> >>
>> >> >> drivers/mtd/spi/spi-nor.c: spi-nor core (not require to add dm)
>> >> >> drivers/mtd/spi/spi-flash-nor.c: spi-nor to spi drivers interface
>> >> >> (dm-driven) drivers/mtd/spi/fsl-quadspi.c: spi-nor controller driver
>> >> >> (dm-driven)
>> >> >>
>> >> >> Please let me know for any more comments.
>> >> >
>> >> > Perhaps another way of asking this is, what is the plan to remove the
>> >> > non-DM code from SF or at least stop adding new features to it.
>> >>
>> >> Sorry I didn't understand "remove non-dm code" or either I missed
>> >> something here.
>> >>
>> >> The plan is not to remove any code intentionally it's about following
>> >> feature additions
>> >> 1) Tuning up spi-flash framework: Separating Core spi-flash code and
>> >> interface code between spi-flash vs spi drivers
>> >> 2) Adding MTD core support to spi-flash core (no spi_flash ops -
>> >> mtd_ops will use)
>> >> 3) Introduce spi-nor core (spi-flash core becomes spi-nor) and new
>> >> spi-nor controller drivers are part of this like fsl_qspi or etc.
>> >> spi-nor controllers and interface code between spi-flash vs
>> >> spi-drivers become UCLASS_SPI_NOR
>> >>
>> >> Agenda is to add SPI-NOR framework(almost similar to Linux) with
>> >> driver model(as UCLASS_SPI_NOR)
>> >>
>> >> ------------------------------------------------------------------------
>> >> -----------------------
>> >>
>> >>       cmd_spi                                      cmd_sf
>> >>
>> >> -------|----------------------------------------------------------------
>> >> ------------------------
>> >>
>> >>         |                                              spi_flash
>> >>
>> >> -------|----------------------------------------------------------------
>> >> ------------------------
>> >>
>> >>         |                                             MTD Core
>> >>
>> >> -------|----------------------------------------------------------------
>> >> ------------------------
>> >>
>> >>         |                                           spi-nor-uclass
>> >>
>> >> -------|----------------------------------------------------------------
>> >> ------------------------
>> >>
>> >>         |                                            SPI-NOR Core
>> >>         |                                            (spi-nor.c)
>> >>
>> >> -------|----------------------------------------------------------------
>> >> -------------------------
>> >>
>> >>         |          |=========spi-nor-flash
>> >>
>> >> drivers/mtd/spi/fsl_qspi
>> >>
>> >>          |          |                   (m25p80.c)
>> >>
>> >>               (fsl-quadspi.c)
>> >>
>> >> --------|-------V-------------------------------------------------------
>> >> ----------------------------
>> >>
>> >>          spi-uclass
>> >>
>> >> ------------------------------------------------------------------------
>> >> --------------------------
>> >>
>> >>          drivers/spi/*
>> >>
>> >>  -----------------------------------------------------
>> >>
>> >> Let me know for any more comments?
>> >
>> > This is quite a big topic. Also you are the maintainer, not me. So I
>> > don't want to interfere. It's just that I am conscious that my SPI
>> > flash conversion was incomplete, in that:
>> >
>> > - it still provides the old API (and we need to keep this until every
>> > board migrates)
>> > - it still uses struct spi_flash as a parameter, when it should really
>> > use struct udevice
>> > - the concept of chip select number and bus number are still present,
>> > but in the end these should just be devices
>> >
>> > So I'm keen to keep moving things in the direction of 'fully migrated'.
>> >
>> > Adding a new feature like SPI nor seems like a good oppty to encourage
>> > people to move over to driver model - .e.g if the new feature is only
>> > available with driver model.
>>
>> Just park this spi-nor stuff, once I send all patches will discuss more.
>>
>> This series is for code re-factorization like separating spi-flash
>> core functionality vs spi-driver interface from spi-flash core. Did
>> you find any wrong on this?
>
> Why don't we converge toward Linux's spi-nor stack instead ?

Yes I'm trying for the same model with dm-driven (UCLASS_SPI_NOR)

thanks!
-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2015-11-18  6:58 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-12 15:00 [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 01/21] spi: zynq_spi: Remove unneeded headers Jagan Teki
2015-10-25  7:32   ` Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 02/21] sf: Return bank_sel, if flash->bank_curr == bank_sel Jagan Teki
2015-10-25  7:33   ` Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 03/21] sf: Add spi_flash_read_bar Jagan Teki
2015-10-25  7:35   ` Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 04/21] sf: Optimize BAR write code Jagan Teki
2015-10-25  7:36   ` Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 05/21] sf: Make flash->flags use for generic usage Jagan Teki
2015-10-25  7:38   ` Jagan Teki
2015-11-16  2:59   ` Bin Meng
2015-11-18  1:14     ` Bin Meng
2015-11-18  1:27       ` Tom Rini
2015-11-18  6:58     ` Jagan Teki
2015-10-12 15:00 ` [U-Boot] [PATCH v4 06/21] sf: Update status reg check in spi_flash_cmd_wait_ready Jagan Teki
2015-10-25  7:39   ` Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 07/21] sf: Add FSR support to spi_flash_cmd_wait_ready Jagan Teki
2015-10-25  7:40   ` Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 08/21] sf: spi_flash_validate_params => spi_flash_scan Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 09/21] sf: Move spi_flash_scan code to sf_ops Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 10/21] sf: Move the read_id " Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 11/21] sf: Move BAR defined code at once place Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 12/21] sf: Use static for file-scope functions Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 13/21] sf: Fix Makefile Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 14/21] sf: Use simple name for register access functions Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 15/21] sf: Use flash function pointers in dm_spi_flash_ops Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 16/21] sf: Flash power up read-only based on idcode0 Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 17/21] sf: Use static for file-scope functions Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 18/21] sf: Remove unneeded header includes Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 19/21] sf: probe: Use spi_flash_scan in dm-spi-flash Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 20/21] sf: Re-factorize spi_flash_probe_tail code Jagan Teki
2015-10-12 15:01 ` [U-Boot] [PATCH v4 21/21] dm-sf: Re-factorize spi_flash_std_probe code Jagan Teki
2015-10-18 20:27 ` [U-Boot] [PATCH v4 00/21] sf: Tunning spi-flash layer Simon Glass
2015-10-19  9:28   ` Jagan Teki
2015-10-28 18:47     ` Simon Glass
2015-10-28 19:08       ` Jagan Teki
2015-10-29 23:25         ` Simon Glass
2015-10-30 20:02           ` Jagan Teki
2015-10-30 20:30             ` Marek Vasut
2015-10-30 20:33               ` Jagan Teki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox