All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
@ 2012-12-31 11:13 Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command Jagannadha Sutradharudu Teki
                   ` (10 more replies)
  0 siblings, 11 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

All these patches are added a support for read and write instruction
for programming/reading SPI flash.

Read and Write instruction are implemented as a command line
arguments for 'sf write' , 'sf read' and 'sf update' commands.

Currently I have added below instructions those are commonly available
on all flash types.
pp - Page Program (existing one)
qpp - Quad-input Page Program
afr - Array Fast Read (existing one)
asr - Array Slow Read
dofr - Dual Output Fast Read
qofr - Quad Output Fast Read
diofr - Dual IO Fast Read
qiofr - Quad IO Fast Read

I have tested mostly of the instruction on real h/w.

This entire implementation will change the current sf framework little bit but
I thought these changes are worth to add.

Request for all your comment, so-that I can move forward.
Please let me know for any issue regarding this new implementation.

Thanks,
Jagan.

Jagannadha Sutradharudu Teki (12):
  cmd_sf: Add wr_inst argument to 'sf write' command
  cmd_sf: Add rd_inst argument to 'sf read' command
  cmd_sf: Add wr_inst argument to 'sf update' command
  cmd_sf: Add rd_inst argument to 'sf update' command
  cmd_sf: Define a functions for parsing read and write instructions
  cmd_sf: Add QPP(Quad-input Page Program) write instruction support
  cmd_sf: Add ASR(Array Slow Read) read instruction support
  cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
  cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
  cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
  cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
  sf: Pass rd_qeb_req variable as 0 for status and config reg reads

 common/cmd_sf.c                      |  198 +++++++++++++++++++++++++++++-----
 drivers/mtd/spi/spi_flash.c          |   40 +++++--
 drivers/mtd/spi/spi_flash_internal.h |   10 +-
 include/spi_flash.h                  |   22 ++--
 include/spi_flash_inst.h             |   39 +++++++
 5 files changed, 257 insertions(+), 52 deletions(-)
 create mode 100644 include/spi_flash_inst.h

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

* [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2013-01-11  2:11   ` Simon Glass
  2012-12-31 11:13 ` [U-Boot] [PATCH 02/12] cmd_sf: Add rd_inst argument to 'sf read' command Jagannadha Sutradharudu Teki
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to add a write instruction(wr_inst)
argument to 'sf write' command.

User will dynamically use the specific write instruction while
programming the flash using 'sf write' command.
Currently added an existing write instruction called pp(Page Program).

Example:
write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction.
u-boot> sf write pp 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c                      |   36 +++++++++++++++++++++++----------
 drivers/mtd/spi/spi_flash.c          |    4 +-
 drivers/mtd/spi/spi_flash_internal.h |    2 +-
 include/spi_flash.h                  |   10 ++++----
 include/spi_flash_inst.h             |   30 ++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 19 deletions(-)
 create mode 100644 include/spi_flash_inst.h

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index b175358..e7843f3 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -9,6 +9,7 @@
 #include <common.h>
 #include <malloc.h>
 #include <spi_flash.h>
+#include <spi_flash_inst.h>
 
 #include <asm/io.h>
 
@@ -234,20 +235,21 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	unsigned long len;
 	void *buf;
 	char *endp;
+	u8 wr_inst;
 	int ret;
 
-	if (argc < 4)
+	if (argc < 5)
 		return -1;
 
-	addr = simple_strtoul(argv[1], &endp, 16);
-	if (*argv[1] == 0 || *endp != 0)
-		return -1;
-	offset = simple_strtoul(argv[2], &endp, 16);
+	addr = simple_strtoul(argv[2], &endp, 16);
 	if (*argv[2] == 0 || *endp != 0)
 		return -1;
-	len = simple_strtoul(argv[3], &endp, 16);
+	offset = simple_strtoul(argv[3], &endp, 16);
 	if (*argv[3] == 0 || *endp != 0)
 		return -1;
+	len = simple_strtoul(argv[4], &endp, 16);
+	if (*argv[4] == 0 || *endp != 0)
+		return -1;
 
 	/* Consistency checking */
 	if (offset + len > flash->size) {
@@ -266,8 +268,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 		ret = spi_flash_update(flash, offset, len, buf);
 	else if (strcmp(argv[0], "read") == 0)
 		ret = spi_flash_read(flash, offset, len, buf);
-	else
-		ret = spi_flash_write(flash, offset, len, buf);
+	else {
+		if (strcmp(argv[1], "pp") == 0)
+			wr_inst = CMD_PAGE_PROGRAM;
+		else {
+			printf("SF: Unknown %s wr_inst on 'sf write'\n",
+					argv[1]);
+			return 1;
+		}
+
+		ret = spi_flash_write(flash, wr_inst, offset, len, buf);
+	}
 
 	unmap_physmem(buf, len);
 
@@ -520,14 +531,17 @@ usage:
 #endif
 
 U_BOOT_CMD(
-	sf,	5,	1,	do_spi_flash,
+	sf,	6,	1,	do_spi_flash,
 	"SPI flash sub-system",
 	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
 	"				  and chip select\n"
 	"sf read addr offset len 	- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr'\n"
-	"sf write addr offset len	- write `len' bytes from memory\n"
-	"				  at `addr' to flash at `offset'\n"
+	"sf write wr_inst addr offset len\n"
+	"				- write `len' bytes from memory\n"
+	"				  at `addr' to flash at `offset' using\n"
+	"				  pp `wr_inst' write instruction\n"
+	"				  pp (Page Program, 02h)\n"
 	"sf erase offset [+]len		- erase `len' bytes from `offset'\n"
 	"				  `+len' round up `len' to block size\n"
 	"sf update addr offset len	- erase and write `len' bytes from memory\n"
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 4dacdc7..8ba2c65 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -65,7 +65,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 	return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
 }
 
-int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
+int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
 		size_t len, const void *buf)
 {
 	unsigned long page_addr, byte_addr, page_size;
@@ -83,7 +83,7 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
 		return ret;
 	}
 
-	cmd[0] = CMD_PAGE_PROGRAM;
+	cmd[0] = wr_inst;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
 
diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
index 15c7ac4..0d416b3 100644
--- a/drivers/mtd/spi/spi_flash_internal.h
+++ b/drivers/mtd/spi/spi_flash_internal.h
@@ -57,7 +57,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
  * Write the requested data out breaking it up into multiple write
  * commands as needed per the write size.
  */
-int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
+int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
 		size_t len, const void *buf);
 
 /*
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 9da9062..9b3a6a1 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -41,8 +41,8 @@ struct spi_flash {
 
 	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		(*write)(struct spi_flash *flash, u8 wr_inst,
+				u32 offset, size_t len, const void *buf);
 	int		(*erase)(struct spi_flash *flash, u32 offset,
 				size_t len);
 };
@@ -57,10 +57,10 @@ static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
 	return flash->read(flash, offset, len, buf);
 }
 
-static inline int spi_flash_write(struct spi_flash *flash, u32 offset,
-		size_t len, const void *buf)
+static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
+		u32 offset, size_t len, const void *buf)
 {
-	return flash->write(flash, offset, len, buf);
+	return flash->write(flash, wr_inst, offset, len, buf);
 }
 
 static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
new file mode 100644
index 0000000..139f45b
--- /dev/null
+++ b/include/spi_flash_inst.h
@@ -0,0 +1,30 @@
+/*
+ * (C) Copyright 2012
+ * Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _SPI_FLASH_INST_H_
+#define _SPI_FLASH_INST_H_
+
+/* Write commands */
+#define CMD_PAGE_PROGRAM		0x02
+
+#endif /* _SPI_FLASH_INST_H_ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 02/12] cmd_sf: Add rd_inst argument to 'sf read' command
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 03/12] cmd_sf: Add wr_inst argument to 'sf update' command Jagannadha Sutradharudu Teki
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to add a read instruction(rd_inst)
argument to 'sf read' command.

User will dynamically use the specific read instruction while
reading the flash using 'sf read' command.
Currently added an existing read instruction called afr(Array Fast Read).

Example:
read 0x2000 length bytes starting at offset 0x0 to memory at 0x10000
using afr read instruction.
u-boot> sf read afr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c                      |   23 +++++++++++++++++------
 drivers/mtd/spi/spi_flash.c          |    4 ++--
 drivers/mtd/spi/spi_flash_internal.h |    2 +-
 include/spi_flash.h                  |   10 +++++-----
 include/spi_flash_inst.h             |    3 +++
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index e7843f3..88b18f8 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -235,7 +235,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	unsigned long len;
 	void *buf;
 	char *endp;
-	u8 wr_inst;
+	u8 wr_inst, rd_inst;
 	int ret;
 
 	if (argc < 5)
@@ -266,9 +266,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 
 	if (strcmp(argv[0], "update") == 0)
 		ret = spi_flash_update(flash, offset, len, buf);
-	else if (strcmp(argv[0], "read") == 0)
-		ret = spi_flash_read(flash, offset, len, buf);
-	else {
+	else if (strcmp(argv[0], "read") == 0) {
+		if (strcmp(argv[1], "afr") == 0)
+			rd_inst = CMD_READ_ARRAY_FAST;
+		else {
+			printf("SF: Unknown %s rd_inst on 'sf read'\n",
+					argv[1]);
+			return 1;
+		}
+
+		ret = spi_flash_read(flash, rd_inst, offset, len, buf);
+	} else {
 		if (strcmp(argv[1], "pp") == 0)
 			wr_inst = CMD_PAGE_PROGRAM;
 		else {
@@ -535,8 +543,11 @@ U_BOOT_CMD(
 	"SPI flash sub-system",
 	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
 	"				  and chip select\n"
-	"sf read addr offset len 	- read `len' bytes starting at\n"
-	"				  `offset' to memory at `addr'\n"
+	"sf read rd_inst addr offset len\n"
+	"				- read `len' bytes starting at\n"
+	"				  `offset' to memory at `addr' using\n"
+	"				  afr `rd_inst' read instruction\n"
+	"				  afr (Array Fast Read, 0bh)\n"
 	"sf write wr_inst addr offset len\n"
 	"				- write `len' bytes from memory\n"
 	"				  at `addr' to flash@`offset' using\n"
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 8ba2c65..0c64ac2 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -135,12 +135,12 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 	return ret;
 }
 
-int spi_flash_cmd_read_fast(struct spi_flash *flash, u32 offset,
+int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u32 offset,
 		size_t len, void *data)
 {
 	u8 cmd[5];
 
-	cmd[0] = CMD_READ_ARRAY_FAST;
+	cmd[0] = rd_inst;
 	spi_flash_addr(offset, cmd);
 	cmd[4] = 0x00;
 
diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
index 0d416b3..dcf8813 100644
--- a/drivers/mtd/spi/spi_flash_internal.h
+++ b/drivers/mtd/spi/spi_flash_internal.h
@@ -43,7 +43,7 @@ int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
 int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
 		size_t cmd_len, void *data, size_t data_len);
 
-int spi_flash_cmd_read_fast(struct spi_flash *flash, u32 offset,
+int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u32 offset,
 		size_t len, void *data);
 
 /*
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 9b3a6a1..6728796 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -39,8 +39,8 @@ struct spi_flash {
 	/* Erase (sector) size */
 	u32		sector_size;
 
-	int		(*read)(struct spi_flash *flash, u32 offset,
-				size_t len, void *buf);
+	int		(*read)(struct spi_flash *flash, u8 rd_inst,
+				u32 offset, size_t len, void *buf);
 	int		(*write)(struct spi_flash *flash, u8 wr_inst,
 				u32 offset, size_t len, const void *buf);
 	int		(*erase)(struct spi_flash *flash, u32 offset,
@@ -51,10 +51,10 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 		unsigned int max_hz, unsigned int spi_mode);
 void spi_flash_free(struct spi_flash *flash);
 
-static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
-		size_t len, void *buf)
+static inline int spi_flash_read(struct spi_flash *flash, u8 rd_inst,
+		u32 offset, size_t len, void *buf)
 {
-	return flash->read(flash, offset, len, buf);
+	return flash->read(flash, rd_inst, offset, len, buf);
 }
 
 static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
index 139f45b..7c1b910 100644
--- a/include/spi_flash_inst.h
+++ b/include/spi_flash_inst.h
@@ -27,4 +27,7 @@
 /* Write commands */
 #define CMD_PAGE_PROGRAM		0x02
 
+/* Read commands */
+#define CMD_READ_ARRAY_FAST		0x0b
+
 #endif /* _SPI_FLASH_INST_H_ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 03/12] cmd_sf: Add wr_inst argument to 'sf update' command
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 02/12] cmd_sf: Add rd_inst argument to 'sf read' command Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 04/12] cmd_sf: Add rd_inst " Jagannadha Sutradharudu Teki
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to add a write instruction(wr_inst)
argument to 'sf update' command.

User will dynamically use the specific write instruction while
programming the flash using 'sf update' command.
Currently added an existing write instruction called pp(Page Program).

Example:
erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction.
u-boot> sf update pp 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 88b18f8..0f4e440 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -141,6 +141,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * If the data being written is the same, then *skipped is incremented by len.
  *
  * @param flash		flash context pointer
+ * @param wr_inst	write instruction
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
@@ -148,8 +149,9 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @param skipped	Count of skipped data (incremented by this function)
  * @return NULL if OK, else a string containing the stage which failed
  */
-static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
-		size_t len, const char *buf, char *cmp_buf, size_t *skipped)
+static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
+		u32 offset, size_t len, const char *buf, char *cmp_buf,
+		size_t *skipped)
 {
 	debug("offset=%#x, sector_size=%#x, len=%#zx\n",
 		offset, flash->sector_size, len);
@@ -163,7 +165,7 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
 	}
 	if (spi_flash_erase(flash, offset, len))
 		return "erase";
-	if (spi_flash_write(flash, offset, len, buf))
+	if (spi_flash_write(flash, wr_inst, offset, len, buf))
 		return "write";
 	return NULL;
 }
@@ -173,12 +175,13 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u32 offset,
  * to change. Existing blocks with the correct data are left unchanged.
  *
  * @param flash		flash context pointer
+ * @param wr_inst	write instruction
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
  * @return 0 if ok, 1 on error
  */
-static int spi_flash_update(struct spi_flash *flash, u32 offset,
+static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u32 offset,
 		size_t len, const char *buf)
 {
 	const char *err_oper = NULL;
@@ -206,8 +209,9 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
 							 start_time));
 				last_update = get_timer(0);
 			}
-			err_oper = spi_flash_update_block(flash, offset, todo,
-					buf, cmp_buf, &skipped);
+			err_oper = spi_flash_update_block(flash, wr_inst,
+					offset, todo, buf, cmp_buf,
+					&skipped);
 		}
 	} else {
 		err_oper = "malloc";
@@ -264,9 +268,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 		return 1;
 	}
 
-	if (strcmp(argv[0], "update") == 0)
-		ret = spi_flash_update(flash, offset, len, buf);
-	else if (strcmp(argv[0], "read") == 0) {
+	if (strcmp(argv[0], "update") == 0) {
+		if (strcmp(argv[1], "pp") == 0)
+			wr_inst = CMD_PAGE_PROGRAM;
+		else {
+			printf("SF: Unknown %s wr_inst on 'sf update'\n",
+					argv[1]);
+			return 1;
+		}
+
+		ret = spi_flash_update(flash, wr_inst, offset, len, buf);
+	} else if (strcmp(argv[0], "read") == 0) {
 		if (strcmp(argv[1], "afr") == 0)
 			rd_inst = CMD_READ_ARRAY_FAST;
 		else {
@@ -555,7 +567,10 @@ U_BOOT_CMD(
 	"				  pp (Page Program, 02h)\n"
 	"sf erase offset [+]len		- erase `len' bytes from `offset'\n"
 	"				  `+len' round up `len' to block size\n"
-	"sf update addr offset len	- erase and write `len' bytes from memory\n"
-	"				  at `addr' to flash at `offset'"
+	"sf update wr_inst addr offset len\n"
+	"				- erase and write `len' bytes from memory\n"
+	"				  at `addr' to flash at `offset' using\n"
+	"				  pp `wr_inst' write instruction\n"
+	"				  pp (Page Program, 02h)"
 	SF_TEST_HELP
 );
-- 
1.7.0.4

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

* [U-Boot] [PATCH 04/12] cmd_sf: Add rd_inst argument to 'sf update' command
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (2 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 03/12] cmd_sf: Add wr_inst argument to 'sf update' command Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions Jagannadha Sutradharudu Teki
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to add a read instruction(rd_inst)
argument to 'sf update' command.

User will dynamically use the specific read instruction while
reading the flash using 'sf update' command.
Currently added an existing read instruction called afr(Array Fast Read).

Example:
erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction and afr read instruction.
u-boot> sf update pp afr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c |   63 +++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 0f4e440..4cfd48a 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -142,6 +142,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  *
  * @param flash		flash context pointer
  * @param wr_inst	write instruction
+ * @param rd_inst	read instruction
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
@@ -150,12 +151,12 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @return NULL if OK, else a string containing the stage which failed
  */
 static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
-		u32 offset, size_t len, const char *buf, char *cmp_buf,
-		size_t *skipped)
+		u8 rd_inst, u32 offset, size_t len, const char *buf,
+		char *cmp_buf, size_t *skipped)
 {
 	debug("offset=%#x, sector_size=%#x, len=%#zx\n",
 		offset, flash->sector_size, len);
-	if (spi_flash_read(flash, offset, len, cmp_buf))
+	if (spi_flash_read(flash, rd_inst, offset, len, cmp_buf))
 		return "read";
 	if (memcmp(cmp_buf, buf, len) == 0) {
 		debug("Skip region %x size %zx: no change\n",
@@ -176,13 +177,14 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
  *
  * @param flash		flash context pointer
  * @param wr_inst	write instruction
+ * @param rd_inst	read instruction
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
  * @return 0 if ok, 1 on error
  */
-static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u32 offset,
-		size_t len, const char *buf)
+static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
+		u32 offset, size_t len, const char *buf)
 {
 	const char *err_oper = NULL;
 	char *cmp_buf;
@@ -210,8 +212,8 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u32 offset,
 				last_update = get_timer(0);
 			}
 			err_oper = spi_flash_update_block(flash, wr_inst,
-					offset, todo, buf, cmp_buf,
-					&skipped);
+					rd_inst, offset, todo, buf,
+					cmp_buf, &skipped);
 		}
 	} else {
 		err_oper = "malloc";
@@ -240,19 +242,29 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	void *buf;
 	char *endp;
 	u8 wr_inst, rd_inst;
+	int update_rd_inst;
 	int ret;
 
-	if (argc < 5)
-		return -1;
+	if (strcmp(argv[0], "update") == 0) {
+		if (argc < 6)
+			return -1;
 
-	addr = simple_strtoul(argv[2], &endp, 16);
-	if (*argv[2] == 0 || *endp != 0)
+		update_rd_inst = 1;
+	} else {
+		if (argc < 5)
+			return -1;
+
+		update_rd_inst = 0;
+	}
+
+	addr = simple_strtoul(argv[update_rd_inst + 2], &endp, 16);
+	if (*argv[update_rd_inst + 2] == 0 || *endp != 0)
 		return -1;
-	offset = simple_strtoul(argv[3], &endp, 16);
-	if (*argv[3] == 0 || *endp != 0)
+	offset = simple_strtoul(argv[update_rd_inst + 3], &endp, 16);
+	if (*argv[update_rd_inst + 3] == 0 || *endp != 0)
 		return -1;
-	len = simple_strtoul(argv[4], &endp, 16);
-	if (*argv[4] == 0 || *endp != 0)
+	len = simple_strtoul(argv[update_rd_inst + 4], &endp, 16);
+	if (*argv[update_rd_inst + 4] == 0 || *endp != 0)
 		return -1;
 
 	/* Consistency checking */
@@ -277,7 +289,16 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 			return 1;
 		}
 
-		ret = spi_flash_update(flash, wr_inst, offset, len, buf);
+		if (strcmp(argv[2], "afr") == 0)
+			rd_inst = CMD_READ_ARRAY_FAST;
+		else {
+			printf("SF: Unknown %s rd_inst on 'sf update'\n",
+					argv[2]);
+			return 1;
+		}
+
+		ret = spi_flash_update(flash, wr_inst, rd_inst,
+					offset, len, buf);
 	} else if (strcmp(argv[0], "read") == 0) {
 		if (strcmp(argv[1], "afr") == 0)
 			rd_inst = CMD_READ_ARRAY_FAST;
@@ -551,7 +572,7 @@ usage:
 #endif
 
 U_BOOT_CMD(
-	sf,	6,	1,	do_spi_flash,
+	sf,	7,	1,	do_spi_flash,
 	"SPI flash sub-system",
 	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
 	"				  and chip select\n"
@@ -567,10 +588,12 @@ U_BOOT_CMD(
 	"				  pp (Page Program, 02h)\n"
 	"sf erase offset [+]len		- erase `len' bytes from `offset'\n"
 	"				  `+len' round up `len' to block size\n"
-	"sf update wr_inst addr offset len\n"
+	"sf update wr_inst rd_inst addr offset len\n"
 	"				- erase and write `len' bytes from memory\n"
 	"				  at `addr' to flash@`offset' using\n"
-	"				  pp `wr_inst' write instruction\n"
-	"				  pp (Page Program, 02h)"
+	"				  pp `wr_inst' write instruction and\n"
+	"				  pp (Page Program, 02h)\n"
+	"				  afr `rd_inst' read instruction\n"
+	"				  afr (Array Fast Read, 0bh)"
 	SF_TEST_HELP
 );
-- 
1.7.0.4

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

* [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (3 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 04/12] cmd_sf: Add rd_inst " Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2013-01-11  2:18   ` Simon Glass
  2012-12-31 11:13 ` [U-Boot] [PATCH 06/12] cmd_sf: Add QPP(Quad-input Page Program) write instruction support Jagannadha Sutradharudu Teki
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides to define a separate functions for parsing read
and write instructions by taking instruction argument from user.

So-that the common functions can used in a different levels for
parsing read and write instructions.

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c |   70 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 4cfd48a..d59ecce 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -234,6 +234,48 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
 	return 0;
 }
 
+/*
+ * This function parsed the write instruction for write operation
+ *
+ * Input:
+ *    arg: specified write instruction from user
+ * Output:
+ *    wr_inst: parsed write instruction for write operation
+ * Return:
+ *    1: for Unknown wr_inst from user
+ *    0: Success
+ */
+static int sf_parse_wr_inst_arg(char *arg, u8 *wr_inst)
+{
+	if (strcmp(arg, "pp") == 0)
+		*wr_inst = CMD_PAGE_PROGRAM;
+	else
+		return 1;
+
+	return 0;
+}
+
+/*
+ * This function parsed the read instruction for read operation
+ *
+ * Input:
+ *    arg: specified read instruction from user
+ * Output:
+ *    rd_inst: parsed read instruction for write operation
+ * Return:
+ *    1: for Unknown rd_inst from user
+ *    0: Success
+ */
+static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst)
+{
+	if (strcmp(arg, "afr") == 0)
+		*rd_inst = CMD_READ_ARRAY_FAST;
+	else
+		return 1;
+
+	return 0;
+}
+
 static int do_spi_flash_read_write(int argc, char * const argv[])
 {
 	unsigned long addr;
@@ -281,41 +323,37 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	}
 
 	if (strcmp(argv[0], "update") == 0) {
-		if (strcmp(argv[1], "pp") == 0)
-			wr_inst = CMD_PAGE_PROGRAM;
-		else {
+		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
+		if (ret) {
 			printf("SF: Unknown %s wr_inst on 'sf update'\n",
 					argv[1]);
-			return 1;
+			return ret;
 		}
 
-		if (strcmp(argv[2], "afr") == 0)
-			rd_inst = CMD_READ_ARRAY_FAST;
-		else {
+		ret = sf_parse_rd_inst_arg(argv[2], &rd_inst);
+		if (ret) {
 			printf("SF: Unknown %s rd_inst on 'sf update'\n",
 					argv[2]);
-			return 1;
+			return ret;
 		}
 
 		ret = spi_flash_update(flash, wr_inst, rd_inst,
 					offset, len, buf);
 	} else if (strcmp(argv[0], "read") == 0) {
-		if (strcmp(argv[1], "afr") == 0)
-			rd_inst = CMD_READ_ARRAY_FAST;
-		else {
+		ret = sf_parse_rd_inst_arg(argv[1], &rd_inst);
+		if (ret) {
 			printf("SF: Unknown %s rd_inst on 'sf read'\n",
 					argv[1]);
-			return 1;
+			return ret;
 		}
 
 		ret = spi_flash_read(flash, rd_inst, offset, len, buf);
 	} else {
-		if (strcmp(argv[1], "pp") == 0)
-			wr_inst = CMD_PAGE_PROGRAM;
-		else {
+		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
+		if (ret) {
 			printf("SF: Unknown %s wr_inst on 'sf write'\n",
 					argv[1]);
-			return 1;
+			return ret;
 		}
 
 		ret = spi_flash_write(flash, wr_inst, offset, len, buf);
-- 
1.7.0.4

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

* [U-Boot] [PATCH 06/12] cmd_sf: Add QPP(Quad-input Page Program) write instruction support
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (4 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 07/12] cmd_sf: Add ASR(Array Slow Read) read " Jagannadha Sutradharudu Teki
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to program a flash using 'qpp'
write instruction(wr_inst) for 'sf write' and 'sf update' commands.

'qpp' will effectively increases the data transfer rate
by up to four times, as compared to the pp( Page Program).

Example:
write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using qpp write instruction.
u-boot> sf write qpp 0x10000 0x0 0x2000

erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using qpp write instruction and afr read instruction.
u-boot> sf update qpp afr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c                      |   41 ++++++++++++++++++++++-----------
 drivers/mtd/spi/spi_flash.c          |   12 ++++++++-
 drivers/mtd/spi/spi_flash_internal.h |    4 +-
 include/spi_flash.h                  |    7 +++--
 include/spi_flash_inst.h             |    1 +
 5 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index d59ecce..b1f19ef 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -143,6 +143,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @param flash		flash context pointer
  * @param wr_inst	write instruction
  * @param rd_inst	read instruction
+ * @param wr_qeb_req	quad enable bit is required for this write operation?
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
@@ -151,8 +152,8 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @return NULL if OK, else a string containing the stage which failed
  */
 static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
-		u8 rd_inst, u32 offset, size_t len, const char *buf,
-		char *cmp_buf, size_t *skipped)
+		u8 rd_inst, u8 wr_qeb_req, u32 offset, size_t len,
+		const char *buf, char *cmp_buf, size_t *skipped)
 {
 	debug("offset=%#x, sector_size=%#x, len=%#zx\n",
 		offset, flash->sector_size, len);
@@ -166,7 +167,7 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
 	}
 	if (spi_flash_erase(flash, offset, len))
 		return "erase";
-	if (spi_flash_write(flash, wr_inst, offset, len, buf))
+	if (spi_flash_write(flash, wr_inst, wr_qeb_req, offset, len, buf))
 		return "write";
 	return NULL;
 }
@@ -178,13 +179,14 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
  * @param flash		flash context pointer
  * @param wr_inst	write instruction
  * @param rd_inst	read instruction
+ * @param wr_qeb_req	quad enable bit is required for this write operation?
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
  * @return 0 if ok, 1 on error
  */
 static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
-		u32 offset, size_t len, const char *buf)
+		u8 wr_qeb_req, u32 offset, size_t len, const char *buf)
 {
 	const char *err_oper = NULL;
 	char *cmp_buf;
@@ -212,7 +214,8 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
 				last_update = get_timer(0);
 			}
 			err_oper = spi_flash_update_block(flash, wr_inst,
-					rd_inst, offset, todo, buf,
+					rd_inst, wr_qeb_req,
+					offset, todo, buf,
 					cmp_buf, &skipped);
 		}
 	} else {
@@ -241,15 +244,21 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
  *    arg: specified write instruction from user
  * Output:
  *    wr_inst: parsed write instruction for write operation
+ *    wr_qeb_req: assign to 1 if this instruction require quad enable bit
+ *		need to set prior to actual write operation
  * Return:
  *    1: for Unknown wr_inst from user
  *    0: Success
  */
-static int sf_parse_wr_inst_arg(char *arg, u8 *wr_inst)
+static int sf_parse_wr_inst_arg(char *arg, u8 *wr_inst, u8 *wr_qeb_req)
 {
-	if (strcmp(arg, "pp") == 0)
+	if (strcmp(arg, "pp") == 0) {
 		*wr_inst = CMD_PAGE_PROGRAM;
-	else
+		*wr_qeb_req = 0;
+	} else if (strcmp(arg, "qpp") == 0) {
+		*wr_inst = CMD_QUAD_PAGE_PROGRAM;
+		*wr_qeb_req = 1;
+	} else
 		return 1;
 
 	return 0;
@@ -284,6 +293,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	void *buf;
 	char *endp;
 	u8 wr_inst, rd_inst;
+	u8 wr_qeb_req;
 	int update_rd_inst;
 	int ret;
 
@@ -323,7 +333,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	}
 
 	if (strcmp(argv[0], "update") == 0) {
-		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
+		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst, &wr_qeb_req);
 		if (ret) {
 			printf("SF: Unknown %s wr_inst on 'sf update'\n",
 					argv[1]);
@@ -338,7 +348,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 		}
 
 		ret = spi_flash_update(flash, wr_inst, rd_inst,
-					offset, len, buf);
+				wr_qeb_req, offset, len, buf);
 	} else if (strcmp(argv[0], "read") == 0) {
 		ret = sf_parse_rd_inst_arg(argv[1], &rd_inst);
 		if (ret) {
@@ -349,14 +359,15 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 
 		ret = spi_flash_read(flash, rd_inst, offset, len, buf);
 	} else {
-		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
+		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst, &wr_qeb_req);
 		if (ret) {
 			printf("SF: Unknown %s wr_inst on 'sf write'\n",
 					argv[1]);
 			return ret;
 		}
 
-		ret = spi_flash_write(flash, wr_inst, offset, len, buf);
+		ret = spi_flash_write(flash, wr_inst, wr_qeb_req,
+				offset, len, buf);
 	}
 
 	unmap_physmem(buf, len);
@@ -622,15 +633,17 @@ U_BOOT_CMD(
 	"sf write wr_inst addr offset len\n"
 	"				- write `len' bytes from memory\n"
 	"				  at `addr' to flash at `offset' using\n"
-	"				  pp `wr_inst' write instruction\n"
+	"				  pp | qpp `wr_inst' write instructions\n"
 	"				  pp (Page Program, 02h)\n"
+	"				  qpp (Quad Page Program, 32h)\n"
 	"sf erase offset [+]len		- erase `len' bytes from `offset'\n"
 	"				  `+len' round up `len' to block size\n"
 	"sf update wr_inst rd_inst addr offset len\n"
 	"				- erase and write `len' bytes from memory\n"
 	"				  at `addr' to flash at `offset' using\n"
-	"				  pp `wr_inst' write instruction and\n"
+	"				  pp | qpp `wr_inst' write instructions and\n"
 	"				  pp (Page Program, 02h)\n"
+	"				  qpp (Quad Page Program, 32h)\n"
 	"				  afr `rd_inst' read instruction\n"
 	"				  afr (Array Fast Read, 0bh)"
 	SF_TEST_HELP
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 0c64ac2..3d57863 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -65,8 +65,8 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
 	return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
 }
 
-int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
-		size_t len, const void *buf)
+int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst,
+		u8 wr_qeb_req, u32 offset, size_t len, const void *buf)
 {
 	unsigned long page_addr, byte_addr, page_size;
 	size_t chunk_len, actual;
@@ -83,6 +83,14 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
 		return ret;
 	}
 
+	if (wr_qeb_req) {
+		ret = spi_flash_set_quad_enable_bit(flash);
+		if (ret) {
+			debug("SF: set quad enable bit failed\n");
+			return ret;
+		}
+	}
+
 	cmd[0] = wr_inst;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		chunk_len = min(len - actual, page_size - byte_addr);
diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
index dcf8813..e36f216 100644
--- a/drivers/mtd/spi/spi_flash_internal.h
+++ b/drivers/mtd/spi/spi_flash_internal.h
@@ -57,8 +57,8 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
  * Write the requested data out breaking it up into multiple write
  * commands as needed per the write size.
  */
-int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
-		size_t len, const void *buf);
+int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst,
+		u8 wr_qeb_req, u32 offset, size_t len, const void *buf);
 
 /*
  * Enable writing on the SPI flash.
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 6728796..31e7d9e 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -42,7 +42,8 @@ struct spi_flash {
 	int		(*read)(struct spi_flash *flash, u8 rd_inst,
 				u32 offset, size_t len, void *buf);
 	int		(*write)(struct spi_flash *flash, u8 wr_inst,
-				u32 offset, size_t len, const void *buf);
+				u8 wr_qeb_req, u32 offset, size_t len,
+				const void *buf);
 	int		(*erase)(struct spi_flash *flash, u32 offset,
 				size_t len);
 };
@@ -58,9 +59,9 @@ static inline int spi_flash_read(struct spi_flash *flash, u8 rd_inst,
 }
 
 static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
-		u32 offset, size_t len, const void *buf)
+		u8 wr_qeb_req, u32 offset, size_t len, const void *buf)
 {
-	return flash->write(flash, wr_inst, offset, len, buf);
+	return flash->write(flash, wr_inst, wr_qeb_req, offset, len, buf);
 }
 
 static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
index 7c1b910..a530842 100644
--- a/include/spi_flash_inst.h
+++ b/include/spi_flash_inst.h
@@ -26,6 +26,7 @@
 
 /* Write commands */
 #define CMD_PAGE_PROGRAM		0x02
+#define CMD_QUAD_PAGE_PROGRAM		0x32
 
 /* Read commands */
 #define CMD_READ_ARRAY_FAST		0x0b
-- 
1.7.0.4

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

* [U-Boot] [PATCH 07/12] cmd_sf: Add ASR(Array Slow Read) read instruction support
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (5 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 06/12] cmd_sf: Add QPP(Quad-input Page Program) write instruction support Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 08/12] cmd_sf: Add DOFR(Dual Output Fast " Jagannadha Sutradharudu Teki
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to read a flash using 'asr'
read instruction(rd_inst) for 'sf read' and 'sf update' commands.

'asr' is similar to afr(Array Fast Read) except that it's
operated under slow speeds.

Example:
read 0x2000 length bytes starting at offset 0x0 to memory at 0x10000
using asr read instruction.
u-boot> sf read asr 0x10000 0x0 0x2000

erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction and asr read instruction.
u-boot> sf update pp asr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c          |   10 +++++++---
 include/spi_flash_inst.h |    1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index b1f19ef..3d9c93e 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -279,6 +279,8 @@ static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst)
 {
 	if (strcmp(arg, "afr") == 0)
 		*rd_inst = CMD_READ_ARRAY_FAST;
+	else if (strcmp(arg, "asr") == 0)
+		*rd_inst = CMD_READ_ARRAY_SLOW;
 	else
 		return 1;
 
@@ -628,8 +630,9 @@ U_BOOT_CMD(
 	"sf read rd_inst addr offset len\n"
 	"				- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr' using\n"
-	"				  afr `rd_inst' read instruction\n"
+	"				  afr | asr `rd_inst' read instructions\n"
 	"				  afr (Array Fast Read, 0bh)\n"
+	"				  asr (Array Slow Read, 02b)\n"
 	"sf write wr_inst addr offset len\n"
 	"				- write `len' bytes from memory\n"
 	"				  at `addr' to flash at `offset' using\n"
@@ -644,7 +647,8 @@ U_BOOT_CMD(
 	"				  pp | qpp `wr_inst' write instructions and\n"
 	"				  pp (Page Program, 02h)\n"
 	"				  qpp (Quad Page Program, 32h)\n"
-	"				  afr `rd_inst' read instruction\n"
-	"				  afr (Array Fast Read, 0bh)"
+	"				  afr | asr `rd_inst' read instructions\n"
+	"				  afr (Array Fast Read, 0bh)\n"
+	"				  asr (Array Slow Read, 02b)"
 	SF_TEST_HELP
 );
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
index a530842..85c8e70 100644
--- a/include/spi_flash_inst.h
+++ b/include/spi_flash_inst.h
@@ -30,5 +30,6 @@
 
 /* Read commands */
 #define CMD_READ_ARRAY_FAST		0x0b
+#define CMD_READ_ARRAY_SLOW		0x03
 
 #endif /* _SPI_FLASH_INST_H_ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 08/12] cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (6 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 07/12] cmd_sf: Add ASR(Array Slow Read) read " Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2012-12-31 11:13 ` [U-Boot] [PATCH 09/12] cmd_sf: Add QOFR(Quad " Jagannadha Sutradharudu Teki
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to read a flash using 'dofr'
read instruction(rd_inst) for 'sf read' and 'sf update' commands.

'dofr' will effectively increases the data transfer rate
by up to two times, as compared to the afr(Array Fast Read).

Example:
read 0x2000 length bytes starting at offset 0x0 to memory at 0x10000
using dofr read instruction.
u-boot> sf read dofr 0x10000 0x0 0x2000

erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction and dofr read instruction.
u-boot> sf update pp dofr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c          |   10 +++++++---
 include/spi_flash_inst.h |    1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 3d9c93e..b971d2a 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -281,6 +281,8 @@ static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst)
 		*rd_inst = CMD_READ_ARRAY_FAST;
 	else if (strcmp(arg, "asr") == 0)
 		*rd_inst = CMD_READ_ARRAY_SLOW;
+	else if (strcmp(arg, "dofr") == 0)
+		*rd_inst = CMD_READ_DUAL_OUTPUT_FAST;
 	else
 		return 1;
 
@@ -630,9 +632,10 @@ U_BOOT_CMD(
 	"sf read rd_inst addr offset len\n"
 	"				- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr' using\n"
-	"				  afr | asr `rd_inst' read instructions\n"
+	"				  afr | asr | dofr `rd_inst' read instructions\n"
 	"				  afr (Array Fast Read, 0bh)\n"
 	"				  asr (Array Slow Read, 02b)\n"
+	"				  dofr (Dual Output Fast Read, 3bh)\n"
 	"sf write wr_inst addr offset len\n"
 	"				- write `len' bytes from memory\n"
 	"				  at `addr' to flash at `offset' using\n"
@@ -647,8 +650,9 @@ U_BOOT_CMD(
 	"				  pp | qpp `wr_inst' write instructions and\n"
 	"				  pp (Page Program, 02h)\n"
 	"				  qpp (Quad Page Program, 32h)\n"
-	"				  afr | asr `rd_inst' read instructions\n"
+	"				  afr | asr | dofr `rd_inst' read instructions\n"
 	"				  afr (Array Fast Read, 0bh)\n"
-	"				  asr (Array Slow Read, 02b)"
+	"				  asr (Array Slow Read, 02b)\n"
+	"				  dofr (Dual Output Fast Read, 3bh)"
 	SF_TEST_HELP
 );
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
index 85c8e70..f8fcf5e 100644
--- a/include/spi_flash_inst.h
+++ b/include/spi_flash_inst.h
@@ -31,5 +31,6 @@
 /* Read commands */
 #define CMD_READ_ARRAY_FAST		0x0b
 #define CMD_READ_ARRAY_SLOW		0x03
+#define CMD_READ_DUAL_OUTPUT_FAST	0x3b
 
 #endif /* _SPI_FLASH_INST_H_ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 09/12] cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (7 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 08/12] cmd_sf: Add DOFR(Dual Output Fast " Jagannadha Sutradharudu Teki
@ 2012-12-31 11:13 ` Jagannadha Sutradharudu Teki
  2013-01-11  2:16 ` [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Simon Glass
  2013-01-11  2:16 ` Simon Glass
  10 siblings, 0 replies; 30+ messages in thread
From: Jagannadha Sutradharudu Teki @ 2012-12-31 11:13 UTC (permalink / raw)
  To: u-boot

This patch provides a support to read a flash using 'qofr'
read instruction(rd_inst) for 'sf read' and 'sf update' commands.

'qofr' will effectively increases the data transfer rate
by up to four times, as compared to the afr(Array Fast Read).

Example:
read 0x2000 length bytes starting at offset 0x0 to memory at 0x10000
using qofr read instruction.
u-boot> sf read qofr 0x10000 0x0 0x2000

erase and write 0x2000 length bytes from memory at 0x10000 address to
flash offset at 0x0 using pp write instruction and qofr read instruction.
u-boot> sf update pp qofr 0x10000 0x0 0x2000

Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c                      |   43 ++++++++++++++++++++++------------
 drivers/mtd/spi/spi_flash.c          |   18 +++++++++++---
 drivers/mtd/spi/spi_flash_internal.h |    6 ++--
 include/spi_flash.h                  |    7 +++--
 include/spi_flash_inst.h             |    1 +
 5 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index b971d2a..a07effd 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -144,6 +144,7 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @param wr_inst	write instruction
  * @param rd_inst	read instruction
  * @param wr_qeb_req	quad enable bit is required for this write operation?
+ * @param rd_qeb_req	quad enable bit is required for this read operation?
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
@@ -152,12 +153,12 @@ static int do_spi_flash_probe(int argc, char * const argv[])
  * @return NULL if OK, else a string containing the stage which failed
  */
 static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
-		u8 rd_inst, u8 wr_qeb_req, u32 offset, size_t len,
-		const char *buf, char *cmp_buf, size_t *skipped)
+		u8 rd_inst, u8 wr_qeb_req, u8 rd_qeb_req, u32 offset,
+		size_t len, const char *buf, char *cmp_buf, size_t *skipped)
 {
 	debug("offset=%#x, sector_size=%#x, len=%#zx\n",
 		offset, flash->sector_size, len);
-	if (spi_flash_read(flash, rd_inst, offset, len, cmp_buf))
+	if (spi_flash_read(flash, rd_inst, rd_qeb_req, offset, len, cmp_buf))
 		return "read";
 	if (memcmp(cmp_buf, buf, len) == 0) {
 		debug("Skip region %x size %zx: no change\n",
@@ -180,13 +181,15 @@ static const char *spi_flash_update_block(struct spi_flash *flash, u8 wr_inst,
  * @param wr_inst	write instruction
  * @param rd_inst	read instruction
  * @param wr_qeb_req	quad enable bit is required for this write operation?
+ * @param rd_qeb_req	quad enable bit is required for this read operation?
  * @param offset	flash offset to write
  * @param len		number of bytes to write
  * @param buf		buffer to write from
  * @return 0 if ok, 1 on error
  */
 static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
-		u8 wr_qeb_req, u32 offset, size_t len, const char *buf)
+		u8 wr_qeb_req, u8 rd_qeb_req, u32 offset,
+		size_t len, const char *buf)
 {
 	const char *err_oper = NULL;
 	char *cmp_buf;
@@ -214,7 +217,7 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
 				last_update = get_timer(0);
 			}
 			err_oper = spi_flash_update_block(flash, wr_inst,
-					rd_inst, wr_qeb_req,
+					rd_inst, wr_qeb_req, rd_qeb_req,
 					offset, todo, buf,
 					cmp_buf, &skipped);
 		}
@@ -271,19 +274,26 @@ static int sf_parse_wr_inst_arg(char *arg, u8 *wr_inst, u8 *wr_qeb_req)
  *    arg: specified read instruction from user
  * Output:
  *    rd_inst: parsed read instruction for write operation
+ *    rd_qeb_req: assign to 1 if this instruction require quad enable bit
+ *		need to set prior to actual read operation
  * Return:
  *    1: for Unknown rd_inst from user
  *    0: Success
  */
-static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst)
+static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst, u8 *rd_qeb_req)
 {
+	*rd_qeb_req = 0;
+
 	if (strcmp(arg, "afr") == 0)
 		*rd_inst = CMD_READ_ARRAY_FAST;
 	else if (strcmp(arg, "asr") == 0)
 		*rd_inst = CMD_READ_ARRAY_SLOW;
 	else if (strcmp(arg, "dofr") == 0)
 		*rd_inst = CMD_READ_DUAL_OUTPUT_FAST;
-	else
+	else if (strcmp(arg, "qofr") == 0) {
+		*rd_inst = CMD_READ_QUAD_OUTPUT_FAST;
+		*rd_qeb_req = 1;
+	} else
 		return 1;
 
 	return 0;
@@ -297,7 +307,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	void *buf;
 	char *endp;
 	u8 wr_inst, rd_inst;
-	u8 wr_qeb_req;
+	u8 wr_qeb_req, rd_qeb_req;
 	int update_rd_inst;
 	int ret;
 
@@ -344,7 +354,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 			return ret;
 		}
 
-		ret = sf_parse_rd_inst_arg(argv[2], &rd_inst);
+		ret = sf_parse_rd_inst_arg(argv[2], &rd_inst, &rd_qeb_req);
 		if (ret) {
 			printf("SF: Unknown %s rd_inst on 'sf update'\n",
 					argv[2]);
@@ -352,16 +362,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 		}
 
 		ret = spi_flash_update(flash, wr_inst, rd_inst,
-				wr_qeb_req, offset, len, buf);
+				wr_qeb_req, rd_qeb_req, offset, len, buf);
 	} else if (strcmp(argv[0], "read") == 0) {
-		ret = sf_parse_rd_inst_arg(argv[1], &rd_inst);
+		ret = sf_parse_rd_inst_arg(argv[1], &rd_inst, &rd_qeb_req);
 		if (ret) {
 			printf("SF: Unknown %s rd_inst on 'sf read'\n",
 					argv[1]);
 			return ret;
 		}
 
-		ret = spi_flash_read(flash, rd_inst, offset, len, buf);
+		ret = spi_flash_read(flash, rd_inst, rd_qeb_req,
+				offset, len, buf);
 	} else {
 		ret = sf_parse_wr_inst_arg(argv[1], &wr_inst, &wr_qeb_req);
 		if (ret) {
@@ -632,10 +643,11 @@ U_BOOT_CMD(
 	"sf read rd_inst addr offset len\n"
 	"				- read `len' bytes starting at\n"
 	"				  `offset' to memory at `addr' using\n"
-	"				  afr | asr | dofr `rd_inst' read instructions\n"
+	"				  afr | asr | dofr | qofr `rd_inst' read instructions\n"
 	"				  afr (Array Fast Read, 0bh)\n"
 	"				  asr (Array Slow Read, 02b)\n"
 	"				  dofr (Dual Output Fast Read, 3bh)\n"
+	"				  qofr (Quad Output Fast Read, 6bh)\n"
 	"sf write wr_inst addr offset len\n"
 	"				- write `len' bytes from memory\n"
 	"				  at `addr' to flash at `offset' using\n"
@@ -650,9 +662,10 @@ U_BOOT_CMD(
 	"				  pp | qpp `wr_inst' write instructions and\n"
 	"				  pp (Page Program, 02h)\n"
 	"				  qpp (Quad Page Program, 32h)\n"
-	"				  afr | asr | dofr `rd_inst' read instructions\n"
+	"				  afr | asr | dofr | qofr `rd_inst' read instructions\n"
 	"				  afr (Array Fast Read, 0bh)\n"
 	"				  asr (Array Slow Read, 02b)\n"
-	"				  dofr (Dual Output Fast Read, 3bh)"
+	"				  dofr (Dual Output Fast Read, 3bh)\n"
+	"				  qofr (Quad Output Fast Read, 6bh)"
 	SF_TEST_HELP
 );
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 3d57863..15ad05c 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -130,21 +130,30 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst,
 	return ret;
 }
 
-int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
+int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd, u8 rd_qeb_req,
 		size_t cmd_len, void *data, size_t data_len)
 {
 	struct spi_slave *spi = flash->spi;
 	int ret;
 
 	spi_claim_bus(spi);
+
+	if (rd_qeb_req) {
+		ret = spi_flash_set_quad_enable_bit(flash);
+		if (ret) {
+			debug("SF: set quad enable bit failed\n");
+			return ret;
+		}
+	}
+
 	ret = spi_flash_cmd_read(spi, cmd, cmd_len, data, data_len);
 	spi_release_bus(spi);
 
 	return ret;
 }
 
-int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u32 offset,
-		size_t len, void *data)
+int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst,
+		u8 rd_qeb_req, u32 offset, size_t len, void *data)
 {
 	u8 cmd[5];
 
@@ -152,7 +161,8 @@ int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u32 offset,
 	spi_flash_addr(offset, cmd);
 	cmd[4] = 0x00;
 
-	return spi_flash_read_common(flash, cmd, sizeof(cmd), data, len);
+	return spi_flash_read_common(flash, cmd, rd_qeb_req,
+				sizeof(cmd), data, len);
 }
 
 int spi_flash_cmd_poll_bit(struct spi_flash *flash, unsigned long timeout,
diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
index e36f216..feefdee 100644
--- a/drivers/mtd/spi/spi_flash_internal.h
+++ b/drivers/mtd/spi/spi_flash_internal.h
@@ -43,8 +43,8 @@ int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
 int spi_flash_cmd_read(struct spi_slave *spi, const u8 *cmd,
 		size_t cmd_len, void *data, size_t data_len);
 
-int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u32 offset,
-		size_t len, void *data);
+int spi_flash_cmd_read_fast(struct spi_flash *flash, u8 rd_inst, u8 rd_qeb_req,
+		u32 offset, size_t len, void *data);
 
 /*
  * Send a multi-byte command to the device followed by (optional)
@@ -96,7 +96,7 @@ int spi_flash_set_quad_enable_bit(struct spi_flash *flash);
  * 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);
+		u8 rd_qeb_req, size_t cmd_len, void *data, size_t data_len);
 
 /* Send a command to the device and wait for some bit to clear itself. */
 int spi_flash_cmd_poll_bit(struct spi_flash *flash, unsigned long timeout,
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 31e7d9e..d3e6ca5 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -40,7 +40,8 @@ struct spi_flash {
 	u32		sector_size;
 
 	int		(*read)(struct spi_flash *flash, u8 rd_inst,
-				u32 offset, size_t len, void *buf);
+				u8 rd_qeb_req, u32 offset, size_t len,
+				void *buf);
 	int		(*write)(struct spi_flash *flash, u8 wr_inst,
 				u8 wr_qeb_req, u32 offset, size_t len,
 				const void *buf);
@@ -53,9 +54,9 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 void spi_flash_free(struct spi_flash *flash);
 
 static inline int spi_flash_read(struct spi_flash *flash, u8 rd_inst,
-		u32 offset, size_t len, void *buf)
+		u8 rd_qeb_req, u32 offset, size_t len, void *buf)
 {
-	return flash->read(flash, rd_inst, offset, len, buf);
+	return flash->read(flash, rd_inst, rd_qeb_req, offset, len, buf);
 }
 
 static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
index f8fcf5e..67b22c8 100644
--- a/include/spi_flash_inst.h
+++ b/include/spi_flash_inst.h
@@ -32,5 +32,6 @@
 #define CMD_READ_ARRAY_FAST		0x0b
 #define CMD_READ_ARRAY_SLOW		0x03
 #define CMD_READ_DUAL_OUTPUT_FAST	0x3b
+#define CMD_READ_QUAD_OUTPUT_FAST	0x6b
 
 #endif /* _SPI_FLASH_INST_H_ */
-- 
1.7.0.4

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

* [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command
  2012-12-31 11:13 ` [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command Jagannadha Sutradharudu Teki
@ 2013-01-11  2:11   ` Simon Glass
       [not found]     ` <CAD6G_RSQXL3RsSuK=4QkRGRNfzuKvU-s7JPrsuB+o5O7GRxJuA@mail.gmail.com>
  0 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2013-01-11  2:11 UTC (permalink / raw)
  To: u-boot

Hi Jagannadha,

On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
<jagannadh.teki@gmail.com> wrote:
> This patch provides a support to add a write instruction(wr_inst)
> argument to 'sf write' command.
>
> User will dynamically use the specific write instruction while
> programming the flash using 'sf write' command.
> Currently added an existing write instruction called pp(Page Program).

I wonder if you might look at using an hyphen option system like:

-p for page program
-x for something else

>
> Example:
> write 0x2000 length bytes from memory at 0x10000 address to
> flash offset at 0x0 using pp write instruction.
> u-boot> sf write pp 0x10000 0x0 0x2000
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> ---
>  common/cmd_sf.c                      |   36 +++++++++++++++++++++++----------
>  drivers/mtd/spi/spi_flash.c          |    4 +-
>  drivers/mtd/spi/spi_flash_internal.h |    2 +-
>  include/spi_flash.h                  |   10 ++++----
>  include/spi_flash_inst.h             |   30 ++++++++++++++++++++++++++++
>  5 files changed, 63 insertions(+), 19 deletions(-)
>  create mode 100644 include/spi_flash_inst.h
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index b175358..e7843f3 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -9,6 +9,7 @@
>  #include <common.h>
>  #include <malloc.h>
>  #include <spi_flash.h>
> +#include <spi_flash_inst.h>
>
>  #include <asm/io.h>
>
> @@ -234,20 +235,21 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>         unsigned long len;
>         void *buf;
>         char *endp;
> +       u8 wr_inst;
>         int ret;
>
> -       if (argc < 4)
> +       if (argc < 5)
>                 return -1;
>
> -       addr = simple_strtoul(argv[1], &endp, 16);
> -       if (*argv[1] == 0 || *endp != 0)
> -               return -1;
> -       offset = simple_strtoul(argv[2], &endp, 16);
> +       addr = simple_strtoul(argv[2], &endp, 16);
>         if (*argv[2] == 0 || *endp != 0)
>                 return -1;
> -       len = simple_strtoul(argv[3], &endp, 16);
> +       offset = simple_strtoul(argv[3], &endp, 16);
>         if (*argv[3] == 0 || *endp != 0)
>                 return -1;
> +       len = simple_strtoul(argv[4], &endp, 16);
> +       if (*argv[4] == 0 || *endp != 0)
> +               return -1;
>
>         /* Consistency checking */
>         if (offset + len > flash->size) {
> @@ -266,8 +268,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>                 ret = spi_flash_update(flash, offset, len, buf);
>         else if (strcmp(argv[0], "read") == 0)
>                 ret = spi_flash_read(flash, offset, len, buf);
> -       else
> -               ret = spi_flash_write(flash, offset, len, buf);
> +       else {
> +               if (strcmp(argv[1], "pp") == 0)
> +                       wr_inst = CMD_PAGE_PROGRAM;
> +               else {
> +                       printf("SF: Unknown %s wr_inst on 'sf write'\n",

Single quotes around %s I think

> +                                       argv[1]);
> +                       return 1;
> +               }
> +
> +               ret = spi_flash_write(flash, wr_inst, offset, len, buf);
> +       }
>
>         unmap_physmem(buf, len);
>
> @@ -520,14 +531,17 @@ usage:
>  #endif
>
>  U_BOOT_CMD(
> -       sf,     5,      1,      do_spi_flash,
> +       sf,     6,      1,      do_spi_flash,
>         "SPI flash sub-system",
>         "probe [[bus:]cs] [hz] [mode]   - init flash device on given SPI bus\n"
>         "                                 and chip select\n"
>         "sf read addr offset len        - read `len' bytes starting at\n"
>         "                                 `offset' to memory at `addr'\n"
> -       "sf write addr offset len       - write `len' bytes from memory\n"
> -       "                                 at `addr' to flash at `offset'\n"
> +       "sf write wr_inst addr offset len\n"

I think wr_inst is optional, so should have [] around it

> +       "                               - write `len' bytes from memory\n"
> +       "                                 at `addr' to flash at `offset' using\n"
> +       "                                 pp `wr_inst' write instruction\n"

options are: pp (only)

W

> +       "                                 pp (Page Program, 02h)\n"
>         "sf erase offset [+]len         - erase `len' bytes from `offset'\n"
>         "                                 `+len' round up `len' to block size\n"
>         "sf update addr offset len      - erase and write `len' bytes from memory\n"
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 4dacdc7..8ba2c65 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -65,7 +65,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>         return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
>  }
>
> -int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
> +int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
>                 size_t len, const void *buf)
>  {
>         unsigned long page_addr, byte_addr, page_size;
> @@ -83,7 +83,7 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
>                 return ret;
>         }
>
> -       cmd[0] = CMD_PAGE_PROGRAM;
> +       cmd[0] = wr_inst;
>         for (actual = 0; actual < len; actual += chunk_len) {
>                 chunk_len = min(len - actual, page_size - byte_addr);
>
> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
> index 15c7ac4..0d416b3 100644
> --- a/drivers/mtd/spi/spi_flash_internal.h
> +++ b/drivers/mtd/spi/spi_flash_internal.h
> @@ -57,7 +57,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>   * Write the requested data out breaking it up into multiple write
>   * commands as needed per the write size.
>   */
> -int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
> +int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
>                 size_t len, const void *buf);
>
>  /*
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 9da9062..9b3a6a1 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -41,8 +41,8 @@ struct spi_flash {
>
>         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             (*write)(struct spi_flash *flash, u8 wr_inst,
> +                               u32 offset, size_t len, const void *buf);

I think wr_inst can just be int? Does it need to be u8? This can
increase code size.

>         int             (*erase)(struct spi_flash *flash, u32 offset,
>                                 size_t len);
>  };
> @@ -57,10 +57,10 @@ static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
>         return flash->read(flash, offset, len, buf);
>  }
>
> -static inline int spi_flash_write(struct spi_flash *flash, u32 offset,
> -               size_t len, const void *buf)
> +static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
> +               u32 offset, size_t len, const void *buf)
>  {
> -       return flash->write(flash, offset, len, buf);
> +       return flash->write(flash, wr_inst, offset, len, buf);
>  }
>
>  static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
> diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
> new file mode 100644
> index 0000000..139f45b
> --- /dev/null
> +++ b/include/spi_flash_inst.h
> @@ -0,0 +1,30 @@
> +/*
> + * (C) Copyright 2012
> + * Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef _SPI_FLASH_INST_H_

Any reason not to put this into spi_flash_internal.h?

> +#define _SPI_FLASH_INST_H_
> +
> +/* Write commands */
> +#define CMD_PAGE_PROGRAM               0x02
> +
> +#endif /* _SPI_FLASH_INST_H_ */
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot


Regards,
Simon

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (8 preceding siblings ...)
  2012-12-31 11:13 ` [U-Boot] [PATCH 09/12] cmd_sf: Add QOFR(Quad " Jagannadha Sutradharudu Teki
@ 2013-01-11  2:16 ` Simon Glass
  2013-01-11  2:16 ` Simon Glass
  10 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2013-01-11  2:16 UTC (permalink / raw)
  To: u-boot

Hi Jagannadha,

On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
<jagannadh.teki@gmail.com> wrote:
> All these patches are added a support for read and write instruction
> for programming/reading SPI flash.
>

I think this is all great and very useful - since no one else has
commented I will have a try.

> Read and Write instruction are implemented as a command line
> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>
> Currently I have added below instructions those are commonly available
> on all flash types.

Maybe you could use flags like -d for dual, -q for quad, -p for page,
-s for slow, -o for output only. So maybe:

-p > pp - Page Program (existing one)
-qp > qpp - Quad-input Page Program
<empty> > afr - Array Fast Read (existing one)
-s > asr - Array Slow Read
-do > dofr - Dual Output Fast Read
-qo > qofr - Quad Output Fast Read
-d > diofr - Dual IO Fast Read
-q > qiofr - Quad IO Fast Read

I worry that your scheme would be hard to remember.


>
> I have tested mostly of the instruction on real h/w.
>
> This entire implementation will change the current sf framework little bit but
> I thought these changes are worth to add.

Yes very much so.

>
> Request for all your comment, so-that I can move forward.
> Please let me know for any issue regarding this new implementation.

Regards,
Simon

>
> Thanks,
> Jagan.
>
> Jagannadha Sutradharudu Teki (12):
>   cmd_sf: Add wr_inst argument to 'sf write' command
>   cmd_sf: Add rd_inst argument to 'sf read' command
>   cmd_sf: Add wr_inst argument to 'sf update' command
>   cmd_sf: Add rd_inst argument to 'sf update' command
>   cmd_sf: Define a functions for parsing read and write instructions
>   cmd_sf: Add QPP(Quad-input Page Program) write instruction support
>   cmd_sf: Add ASR(Array Slow Read) read instruction support
>   cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
>   cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
>   cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
>   cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
>   sf: Pass rd_qeb_req variable as 0 for status and config reg reads
>
>  common/cmd_sf.c                      |  198 +++++++++++++++++++++++++++++-----
>  drivers/mtd/spi/spi_flash.c          |   40 +++++--
>  drivers/mtd/spi/spi_flash_internal.h |   10 +-
>  include/spi_flash.h                  |   22 ++--
>  include/spi_flash_inst.h             |   39 +++++++
>  5 files changed, 257 insertions(+), 52 deletions(-)
>  create mode 100644 include/spi_flash_inst.h
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
                   ` (9 preceding siblings ...)
  2013-01-11  2:16 ` [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Simon Glass
@ 2013-01-11  2:16 ` Simon Glass
  2013-03-02  8:29   ` Jagan Teki
  10 siblings, 1 reply; 30+ messages in thread
From: Simon Glass @ 2013-01-11  2:16 UTC (permalink / raw)
  To: u-boot

Hi Jagannadha,

On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
<jagannadh.teki@gmail.com> wrote:
> All these patches are added a support for read and write instruction
> for programming/reading SPI flash.
>

I think this is all great and very useful - since no one else has
commented I will have a try.

> Read and Write instruction are implemented as a command line
> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>
> Currently I have added below instructions those are commonly available
> on all flash types.

Maybe you could use flags like -d for dual, -q for quad, -p for page,
-s for slow, -o for output only. So maybe:

-p > pp - Page Program (existing one)
-qp > qpp - Quad-input Page Program
<empty> > afr - Array Fast Read (existing one)
-s > asr - Array Slow Read
-do > dofr - Dual Output Fast Read
-qo > qofr - Quad Output Fast Read
-d > diofr - Dual IO Fast Read
-q > qiofr - Quad IO Fast Read

I worry that your scheme would be hard to remember.


>
> I have tested mostly of the instruction on real h/w.
>
> This entire implementation will change the current sf framework little bit but
> I thought these changes are worth to add.

Yes very much so.

>
> Request for all your comment, so-that I can move forward.
> Please let me know for any issue regarding this new implementation.

Regards,
Simon

>
> Thanks,
> Jagan.
>
> Jagannadha Sutradharudu Teki (12):
>   cmd_sf: Add wr_inst argument to 'sf write' command
>   cmd_sf: Add rd_inst argument to 'sf read' command
>   cmd_sf: Add wr_inst argument to 'sf update' command
>   cmd_sf: Add rd_inst argument to 'sf update' command
>   cmd_sf: Define a functions for parsing read and write instructions
>   cmd_sf: Add QPP(Quad-input Page Program) write instruction support
>   cmd_sf: Add ASR(Array Slow Read) read instruction support
>   cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
>   cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
>   cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
>   cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
>   sf: Pass rd_qeb_req variable as 0 for status and config reg reads
>
>  common/cmd_sf.c                      |  198 +++++++++++++++++++++++++++++-----
>  drivers/mtd/spi/spi_flash.c          |   40 +++++--
>  drivers/mtd/spi/spi_flash_internal.h |   10 +-
>  include/spi_flash.h                  |   22 ++--
>  include/spi_flash_inst.h             |   39 +++++++
>  5 files changed, 257 insertions(+), 52 deletions(-)
>  create mode 100644 include/spi_flash_inst.h
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions
  2012-12-31 11:13 ` [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions Jagannadha Sutradharudu Teki
@ 2013-01-11  2:18   ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2013-01-11  2:18 UTC (permalink / raw)
  To: u-boot

Hi Jagannadha,

On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
<jagannadh.teki@gmail.com> wrote:
> This patch provides to define a separate functions for parsing read
> and write instructions by taking instruction argument from user.
>
> So-that the common functions can used in a different levels for
> parsing read and write instructions.
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> ---
>  common/cmd_sf.c |   70 ++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 4cfd48a..d59ecce 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -234,6 +234,48 @@ static int spi_flash_update(struct spi_flash *flash, u8 wr_inst, u8 rd_inst,
>         return 0;
>  }
>
> +/*
> + * This function parsed the write instruction for write operation
> + *
> + * Input:
> + *    arg: specified write instruction from user
> + * Output:
> + *    wr_inst: parsed write instruction for write operation
> + * Return:
> + *    1: for Unknown wr_inst from user
> + *    0: Success
> + */
> +static int sf_parse_wr_inst_arg(char *arg, u8 *wr_inst)
> +{
> +       if (strcmp(arg, "pp") == 0)
> +               *wr_inst = CMD_PAGE_PROGRAM;
> +       else
> +               return 1;
> +
> +       return 0;
> +}
> +
> +/*
> + * This function parsed the read instruction for read operation
> + *
> + * Input:
> + *    arg: specified read instruction from user
> + * Output:
> + *    rd_inst: parsed read instruction for write operation
> + * Return:
> + *    1: for Unknown rd_inst from user
> + *    0: Success
> + */
> +static int sf_parse_rd_inst_arg(char *arg, u8 *rd_inst)
> +{
> +       if (strcmp(arg, "afr") == 0)
> +               *rd_inst = CMD_READ_ARRAY_FAST;
> +       else
> +               return 1;
> +
> +       return 0;
> +}
> +
>  static int do_spi_flash_read_write(int argc, char * const argv[])
>  {
>         unsigned long addr;
> @@ -281,41 +323,37 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>         }
>
>         if (strcmp(argv[0], "update") == 0) {
> -               if (strcmp(argv[1], "pp") == 0)
> -                       wr_inst = CMD_PAGE_PROGRAM;
> -               else {
> +               ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
> +               if (ret) {
>                         printf("SF: Unknown %s wr_inst on 'sf update'\n",
>                                         argv[1]);
> -                       return 1;
> +                       return ret;
>                 }
>
> -               if (strcmp(argv[2], "afr") == 0)
> -                       rd_inst = CMD_READ_ARRAY_FAST;
> -               else {

Are you removing code you added in a previous patch? Why not just
define sf_parse_rd_inst_arg() in the first patch and use it
everywhere?

Also do you need to parse differently for read and write? Would be
nice to avoid that.

> +               ret = sf_parse_rd_inst_arg(argv[2], &rd_inst);
> +               if (ret) {
>                         printf("SF: Unknown %s rd_inst on 'sf update'\n",
>                                         argv[2]);
> -                       return 1;
> +                       return ret;
>                 }
>
>                 ret = spi_flash_update(flash, wr_inst, rd_inst,
>                                         offset, len, buf);
>         } else if (strcmp(argv[0], "read") == 0) {
> -               if (strcmp(argv[1], "afr") == 0)
> -                       rd_inst = CMD_READ_ARRAY_FAST;
> -               else {
> +               ret = sf_parse_rd_inst_arg(argv[1], &rd_inst);
> +               if (ret) {
>                         printf("SF: Unknown %s rd_inst on 'sf read'\n",
>                                         argv[1]);
> -                       return 1;
> +                       return ret;
>                 }
>
>                 ret = spi_flash_read(flash, rd_inst, offset, len, buf);
>         } else {
> -               if (strcmp(argv[1], "pp") == 0)
> -                       wr_inst = CMD_PAGE_PROGRAM;
> -               else {
> +               ret = sf_parse_wr_inst_arg(argv[1], &wr_inst);
> +               if (ret) {
>                         printf("SF: Unknown %s wr_inst on 'sf write'\n",
>                                         argv[1]);
> -                       return 1;
> +                       return ret;
>                 }
>
>                 ret = spi_flash_write(flash, wr_inst, offset, len, buf);
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon

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

* [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command
       [not found]     ` <CAD6G_RSQXL3RsSuK=4QkRGRNfzuKvU-s7JPrsuB+o5O7GRxJuA@mail.gmail.com>
@ 2013-02-14 14:35       ` Simon Glass
  0 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2013-02-14 14:35 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On Wed, Jan 16, 2013 at 2:52 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Simon,
>
> On Fri, Jan 11, 2013 at 7:41 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Jagannadha,
>>
>> On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
>> <jagannadh.teki@gmail.com> wrote:
>>> This patch provides a support to add a write instruction(wr_inst)
>>> argument to 'sf write' command.
>>>
>>> User will dynamically use the specific write instruction while
>>> programming the flash using 'sf write' command.
>>> Currently added an existing write instruction called pp(Page Program).
>>
>> I wonder if you might look at using an hyphen option system like:
>>
>> -p for page program
>> -x for something else
>
> May be required, I will look into this.
>
>>
>>>
>>> Example:
>>> write 0x2000 length bytes from memory at 0x10000 address to
>>> flash offset at 0x0 using pp write instruction.
>>> u-boot> sf write pp 0x10000 0x0 0x2000
>>>
>>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>> ---
>>>  common/cmd_sf.c                      |   36 +++++++++++++++++++++++----------
>>>  drivers/mtd/spi/spi_flash.c          |    4 +-
>>>  drivers/mtd/spi/spi_flash_internal.h |    2 +-
>>>  include/spi_flash.h                  |   10 ++++----
>>>  include/spi_flash_inst.h             |   30 ++++++++++++++++++++++++++++
>>>  5 files changed, 63 insertions(+), 19 deletions(-)
>>>  create mode 100644 include/spi_flash_inst.h
>>>
>>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
>>> index b175358..e7843f3 100644
>>> --- a/common/cmd_sf.c
>>> +++ b/common/cmd_sf.c
>>> @@ -9,6 +9,7 @@
>>>  #include <common.h>
>>>  #include <malloc.h>
>>>  #include <spi_flash.h>
>>> +#include <spi_flash_inst.h>
>>>
>>>  #include <asm/io.h>
>>>
>>> @@ -234,20 +235,21 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>>>         unsigned long len;
>>>         void *buf;
>>>         char *endp;
>>> +       u8 wr_inst;
>>>         int ret;
>>>
>>> -       if (argc < 4)
>>> +       if (argc < 5)
>>>                 return -1;
>>>
>>> -       addr = simple_strtoul(argv[1], &endp, 16);
>>> -       if (*argv[1] == 0 || *endp != 0)
>>> -               return -1;
>>> -       offset = simple_strtoul(argv[2], &endp, 16);
>>> +       addr = simple_strtoul(argv[2], &endp, 16);
>>>         if (*argv[2] == 0 || *endp != 0)
>>>                 return -1;
>>> -       len = simple_strtoul(argv[3], &endp, 16);
>>> +       offset = simple_strtoul(argv[3], &endp, 16);
>>>         if (*argv[3] == 0 || *endp != 0)
>>>                 return -1;
>>> +       len = simple_strtoul(argv[4], &endp, 16);
>>> +       if (*argv[4] == 0 || *endp != 0)
>>> +               return -1;
>>>
>>>         /* Consistency checking */
>>>         if (offset + len > flash->size) {
>>> @@ -266,8 +268,17 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>>>                 ret = spi_flash_update(flash, offset, len, buf);
>>>         else if (strcmp(argv[0], "read") == 0)
>>>                 ret = spi_flash_read(flash, offset, len, buf);
>>> -       else
>>> -               ret = spi_flash_write(flash, offset, len, buf);
>>> +       else {
>>> +               if (strcmp(argv[1], "pp") == 0)
>>> +                       wr_inst = CMD_PAGE_PROGRAM;
>>> +               else {
>>> +                       printf("SF: Unknown %s wr_inst on 'sf write'\n",
>>
>> Single quotes around %s I think
>
> Ok.
>
>>
>>> +                                       argv[1]);
>>> +                       return 1;
>>> +               }
>>> +
>>> +               ret = spi_flash_write(flash, wr_inst, offset, len, buf);
>>> +       }
>>>
>>>         unmap_physmem(buf, len);
>>>
>>> @@ -520,14 +531,17 @@ usage:
>>>  #endif
>>>
>>>  U_BOOT_CMD(
>>> -       sf,     5,      1,      do_spi_flash,
>>> +       sf,     6,      1,      do_spi_flash,
>>>         "SPI flash sub-system",
>>>         "probe [[bus:]cs] [hz] [mode]   - init flash device on given SPI bus\n"
>>>         "                                 and chip select\n"
>>>         "sf read addr offset len        - read `len' bytes starting at\n"
>>>         "                                 `offset' to memory at `addr'\n"
>>> -       "sf write addr offset len       - write `len' bytes from memory\n"
>>> -       "                                 at `addr' to flash at `offset'\n"
>>> +       "sf write wr_inst addr offset len\n"
>>
>> I think wr_inst is optional, so should have [] around it
>>
>>> +       "                               - write `len' bytes from memory\n"
>>> +       "                                 at `addr' to flash at `offset' using\n"
>>> +       "                                 pp `wr_inst' write instruction\n"
>>
>> options are: pp (only)
>>
>> W
>
> I think your idea is to use optional read/write inst for existing
> supported instruction is it?

Yes, please have a think about whether this makes sense.

>
>>
>>> +       "                                 pp (Page Program, 02h)\n"
>>>         "sf erase offset [+]len         - erase `len' bytes from `offset'\n"
>>>         "                                 `+len' round up `len' to block size\n"
>>>         "sf update addr offset len      - erase and write `len' bytes from memory\n"
>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>> index 4dacdc7..8ba2c65 100644
>>> --- a/drivers/mtd/spi/spi_flash.c
>>> +++ b/drivers/mtd/spi/spi_flash.c
>>> @@ -65,7 +65,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>>>         return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
>>>  }
>>>
>>> -int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
>>> +int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
>>>                 size_t len, const void *buf)
>>>  {
>>>         unsigned long page_addr, byte_addr, page_size;
>>> @@ -83,7 +83,7 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
>>>                 return ret;
>>>         }
>>>
>>> -       cmd[0] = CMD_PAGE_PROGRAM;
>>> +       cmd[0] = wr_inst;
>>>         for (actual = 0; actual < len; actual += chunk_len) {
>>>                 chunk_len = min(len - actual, page_size - byte_addr);
>>>
>>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h
>>> index 15c7ac4..0d416b3 100644
>>> --- a/drivers/mtd/spi/spi_flash_internal.h
>>> +++ b/drivers/mtd/spi/spi_flash_internal.h
>>> @@ -57,7 +57,7 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>>>   * Write the requested data out breaking it up into multiple write
>>>   * commands as needed per the write size.
>>>   */
>>> -int spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
>>> +int spi_flash_cmd_write_multi(struct spi_flash *flash, u8 wr_inst, u32 offset,
>>>                 size_t len, const void *buf);
>>>
>>>  /*
>>> diff --git a/include/spi_flash.h b/include/spi_flash.h
>>> index 9da9062..9b3a6a1 100644
>>> --- a/include/spi_flash.h
>>> +++ b/include/spi_flash.h
>>> @@ -41,8 +41,8 @@ struct spi_flash {
>>>
>>>         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             (*write)(struct spi_flash *flash, u8 wr_inst,
>>> +                               u32 offset, size_t len, const void *buf);
>>
>> I think wr_inst can just be int? Does it need to be u8? This can
>> increase code size.
>
> wr_inst is single byte command instruction that why I have used u8.

Sure - but for function arguments it is often better to use native
type sizes (reduces code size). Still in this case I doubt it matters.

>
>>
>>>         int             (*erase)(struct spi_flash *flash, u32 offset,
>>>                                 size_t len);
>>>  };
>>> @@ -57,10 +57,10 @@ static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
>>>         return flash->read(flash, offset, len, buf);
>>>  }
>>>
>>> -static inline int spi_flash_write(struct spi_flash *flash, u32 offset,
>>> -               size_t len, const void *buf)
>>> +static inline int spi_flash_write(struct spi_flash *flash, u8 wr_inst,
>>> +               u32 offset, size_t len, const void *buf)
>>>  {
>>> -       return flash->write(flash, offset, len, buf);
>>> +       return flash->write(flash, wr_inst, offset, len, buf);
>>>  }
>>>
>>>  static inline int spi_flash_erase(struct spi_flash *flash, u32 offset,
>>> diff --git a/include/spi_flash_inst.h b/include/spi_flash_inst.h
>>> new file mode 100644
>>> index 0000000..139f45b
>>> --- /dev/null
>>> +++ b/include/spi_flash_inst.h
>>> @@ -0,0 +1,30 @@
>>> +/*
>>> + * (C) Copyright 2012
>>> + * Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>> + *
>>> + * See file CREDITS for list of people who contributed to this
>>> + * project.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation; either version 2 of
>>> + * the License, or (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>>> + * MA 02111-1307 USA
>>> + */
>>> +
>>> +#ifndef _SPI_FLASH_INST_H_
>>
>> Any reason not to put this into spi_flash_internal.h?
>
> I thought header file from drivers/mtd/spi/spi_flash_internal.h
> couldn't be use directly in non driver directory area.
> am I correct?

Yes.

>
> this is the reason why I have newly added new header file in include
> folder for using it on common folder area.

Then perhaps just put this stuff in include/spi_flash.h? Are you
wanting to avoid it being available to other drivers?

Regards,
Simon

>
> Thanks,
> Jagan.
>
>>
>>> +#define _SPI_FLASH_INST_H_
>>> +
>>> +/* Write commands */
>>> +#define CMD_PAGE_PROGRAM               0x02
>>> +
>>> +#endif /* _SPI_FLASH_INST_H_ */
>>> --
>>> 1.7.0.4
>>>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>> Regards,
>> Simon

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-01-11  2:16 ` Simon Glass
@ 2013-03-02  8:29   ` Jagan Teki
  2013-03-04 21:02     ` Jagan Teki
                       ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Jagan Teki @ 2013-03-02  8:29 UTC (permalink / raw)
  To: u-boot

Hi All,

On Fri, Jan 11, 2013 at 7:46 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Jagannadha,
>
> On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
> <jagannadh.teki@gmail.com> wrote:
>> All these patches are added a support for read and write instruction
>> for programming/reading SPI flash.
>>
>
> I think this is all great and very useful - since no one else has
> commented I will have a try.
>
>> Read and Write instruction are implemented as a command line
>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>
>> Currently I have added below instructions those are commonly available
>> on all flash types.
>
> Maybe you could use flags like -d for dual, -q for quad, -p for page,
> -s for slow, -o for output only. So maybe:
>
> -p > pp - Page Program (existing one)
> -qp > qpp - Quad-input Page Program
> <empty> > afr - Array Fast Read (existing one)
> -s > asr - Array Slow Read
> -do > dofr - Dual Output Fast Read
> -qo > qofr - Quad Output Fast Read
> -d > diofr - Dual IO Fast Read
> -q > qiofr - Quad IO Fast Read
>
> I worry that your scheme would be hard to remember.
>
>
>>
>> I have tested mostly of the instruction on real h/w.
>>
>> This entire implementation will change the current sf framework little bit but
>> I thought these changes are worth to add.
>
> Yes very much so.
>
>>
>> Request for all your comment, so-that I can move forward.
>> Please let me know for any issue regarding this new implementation.
>
> Regards,
> Simon
>
>>
>> Thanks,
>> Jagan.
>>
>> Jagannadha Sutradharudu Teki (12):
>>   cmd_sf: Add wr_inst argument to 'sf write' command
>>   cmd_sf: Add rd_inst argument to 'sf read' command
>>   cmd_sf: Add wr_inst argument to 'sf update' command
>>   cmd_sf: Add rd_inst argument to 'sf update' command
>>   cmd_sf: Define a functions for parsing read and write instructions
>>   cmd_sf: Add QPP(Quad-input Page Program) write instruction support
>>   cmd_sf: Add ASR(Array Slow Read) read instruction support
>>   cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
>>   cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
>>   cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
>>   cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
>>   sf: Pass rd_qeb_req variable as 0 for status and config reg reads
>>
>>  common/cmd_sf.c                      |  198 +++++++++++++++++++++++++++++-----
>>  drivers/mtd/spi/spi_flash.c          |   40 +++++--
>>  drivers/mtd/spi/spi_flash_internal.h |   10 +-
>>  include/spi_flash.h                  |   22 ++--
>>  include/spi_flash_inst.h             |   39 +++++++
>>  5 files changed, 257 insertions(+), 52 deletions(-)
>>  create mode 100644 include/spi_flash_inst.h
>>

Since these changes I have sent long back, I am just re-modified the
framework to
add new features at the same time with backward comparability for
current commands.

Current command setup:
sf write
sf read
sf update

Changed command set: [no changes in the argument count]
sf write     ---  current command
sf write.pp --  same as sf write
sf write.qp -- quad program

sf read   -- current read
sf read.af --- array flast read, same as sf read
sf read.as -- array slow read
sf read.do --- dual out
sf read.qo -- quad out
sf read.dio -- dual io
sf read.qio -- quad io

sf update  -- current update
sf update.pp.af -- write page program, read array fast, same as sf update
sf update.pp.as - write page program, read array slow
sf update.pp.do - write page program, read dual out
sf update.pp.qo - write page program, read quad out
sf update.pp.dio - write page program, read dual io
sf update.pp.qio - write page program, read quad io
sf update.qp.af - write quad program, read array fast
sf update.qp.as - write quad program, read array slow
sf update.qp.do - write quad program, read dual out
sf update.qp.qo - write quad program, read quad out
sf update.qp.dio - write quad program, read dual io
sf update.qp.qio - write quad program, read quad io

Though it seems to be lengthy, but may useful with lot of combinations
from user.
My intention is to use the existing argument count with changes in the
command set.

Request for your inputs.

Thanks,
Jagan.

>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-02  8:29   ` Jagan Teki
@ 2013-03-04 21:02     ` Jagan Teki
  2013-03-04 21:08     ` Tom Rini
  2013-03-08 23:55     ` Simon Glass
  2 siblings, 0 replies; 30+ messages in thread
From: Jagan Teki @ 2013-03-04 21:02 UTC (permalink / raw)
  To: u-boot

Hi All,

Any suggestions please.

Thanks,
Jagan.

On Sat, Mar 2, 2013 at 1:59 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi All,
>
> On Fri, Jan 11, 2013 at 7:46 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Jagannadha,
>>
>> On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
>> <jagannadh.teki@gmail.com> wrote:
>>> All these patches are added a support for read and write instruction
>>> for programming/reading SPI flash.
>>>
>>
>> I think this is all great and very useful - since no one else has
>> commented I will have a try.
>>
>>> Read and Write instruction are implemented as a command line
>>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>>
>>> Currently I have added below instructions those are commonly available
>>> on all flash types.
>>
>> Maybe you could use flags like -d for dual, -q for quad, -p for page,
>> -s for slow, -o for output only. So maybe:
>>
>> -p > pp - Page Program (existing one)
>> -qp > qpp - Quad-input Page Program
>> <empty> > afr - Array Fast Read (existing one)
>> -s > asr - Array Slow Read
>> -do > dofr - Dual Output Fast Read
>> -qo > qofr - Quad Output Fast Read
>> -d > diofr - Dual IO Fast Read
>> -q > qiofr - Quad IO Fast Read
>>
>> I worry that your scheme would be hard to remember.
>>
>>
>>>
>>> I have tested mostly of the instruction on real h/w.
>>>
>>> This entire implementation will change the current sf framework little bit but
>>> I thought these changes are worth to add.
>>
>> Yes very much so.
>>
>>>
>>> Request for all your comment, so-that I can move forward.
>>> Please let me know for any issue regarding this new implementation.
>>
>> Regards,
>> Simon
>>
>>>
>>> Thanks,
>>> Jagan.
>>>
>>> Jagannadha Sutradharudu Teki (12):
>>>   cmd_sf: Add wr_inst argument to 'sf write' command
>>>   cmd_sf: Add rd_inst argument to 'sf read' command
>>>   cmd_sf: Add wr_inst argument to 'sf update' command
>>>   cmd_sf: Add rd_inst argument to 'sf update' command
>>>   cmd_sf: Define a functions for parsing read and write instructions
>>>   cmd_sf: Add QPP(Quad-input Page Program) write instruction support
>>>   cmd_sf: Add ASR(Array Slow Read) read instruction support
>>>   cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
>>>   cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
>>>   cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
>>>   cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
>>>   sf: Pass rd_qeb_req variable as 0 for status and config reg reads
>>>
>>>  common/cmd_sf.c                      |  198 +++++++++++++++++++++++++++++-----
>>>  drivers/mtd/spi/spi_flash.c          |   40 +++++--
>>>  drivers/mtd/spi/spi_flash_internal.h |   10 +-
>>>  include/spi_flash.h                  |   22 ++--
>>>  include/spi_flash_inst.h             |   39 +++++++
>>>  5 files changed, 257 insertions(+), 52 deletions(-)
>>>  create mode 100644 include/spi_flash_inst.h
>>>
>
> Since these changes I have sent long back, I am just re-modified the
> framework to
> add new features at the same time with backward comparability for
> current commands.
>
> Current command setup:
> sf write
> sf read
> sf update
>
> Changed command set: [no changes in the argument count]
> sf write     ---  current command
> sf write.pp --  same as sf write
> sf write.qp -- quad program
>
> sf read   -- current read
> sf read.af --- array flast read, same as sf read
> sf read.as -- array slow read
> sf read.do --- dual out
> sf read.qo -- quad out
> sf read.dio -- dual io
> sf read.qio -- quad io
>
> sf update  -- current update
> sf update.pp.af -- write page program, read array fast, same as sf update
> sf update.pp.as - write page program, read array slow
> sf update.pp.do - write page program, read dual out
> sf update.pp.qo - write page program, read quad out
> sf update.pp.dio - write page program, read dual io
> sf update.pp.qio - write page program, read quad io
> sf update.qp.af - write quad program, read array fast
> sf update.qp.as - write quad program, read array slow
> sf update.qp.do - write quad program, read dual out
> sf update.qp.qo - write quad program, read quad out
> sf update.qp.dio - write quad program, read dual io
> sf update.qp.qio - write quad program, read quad io
>
> Though it seems to be lengthy, but may useful with lot of combinations
> from user.
> My intention is to use the existing argument count with changes in the
> command set.
>
> Request for your inputs.
>
> Thanks,
> Jagan.
>
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-02  8:29   ` Jagan Teki
  2013-03-04 21:02     ` Jagan Teki
@ 2013-03-04 21:08     ` Tom Rini
  2013-03-05 17:00       ` Jagan Teki
  2013-03-08 23:55     ` Simon Glass
  2 siblings, 1 reply; 30+ messages in thread
From: Tom Rini @ 2013-03-04 21:08 UTC (permalink / raw)
  To: u-boot

On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki wrote:

[snip]
> Since these changes I have sent long back, I am just re-modified the
> framework to
> add new features at the same time with backward comparability for
> current commands.
> 
> Current command setup:
> sf write
> sf read
> sf update
> 
> Changed command set: [no changes in the argument count]
> sf write     ---  current command
> sf write.pp --  same as sf write
> sf write.qp -- quad program
> 
> sf read   -- current read
> sf read.af --- array flast read, same as sf read
> sf read.as -- array slow read
> sf read.do --- dual out
> sf read.qo -- quad out
> sf read.dio -- dual io
> sf read.qio -- quad io
> 
> sf update  -- current update
> sf update.pp.af -- write page program, read array fast, same as sf update
> sf update.pp.as - write page program, read array slow
> sf update.pp.do - write page program, read dual out
> sf update.pp.qo - write page program, read quad out
> sf update.pp.dio - write page program, read dual io
> sf update.pp.qio - write page program, read quad io
> sf update.qp.af - write quad program, read array fast
> sf update.qp.as - write quad program, read array slow
> sf update.qp.do - write quad program, read dual out
> sf update.qp.qo - write quad program, read quad out
> sf update.qp.dio - write quad program, read dual io
> sf update.qp.qio - write quad program, read quad io
> 
> Though it seems to be lengthy, but may useful with lot of combinations
> from user.
> My intention is to use the existing argument count with changes in the
> command set.

Are there cases where for the current device we're operating on that can
handle more than one of these, aside from fast or slow?  And do we
really need to offer both fast and slow?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130304/2f30d4c9/attachment.pgp>

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-04 21:08     ` Tom Rini
@ 2013-03-05 17:00       ` Jagan Teki
  2013-03-05 17:08         ` Tom Rini
  0 siblings, 1 reply; 30+ messages in thread
From: Jagan Teki @ 2013-03-05 17:00 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On Tue, Mar 5, 2013 at 2:38 AM, Tom Rini <trini@ti.com> wrote:
> On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki wrote:
>
> [snip]
>> Since these changes I have sent long back, I am just re-modified the
>> framework to
>> add new features at the same time with backward comparability for
>> current commands.
>>
>> Current command setup:
>> sf write
>> sf read
>> sf update
>>
>> Changed command set: [no changes in the argument count]
>> sf write     ---  current command
>> sf write.pp --  same as sf write
>> sf write.qp -- quad program
>>
>> sf read   -- current read
>> sf read.af --- array flast read, same as sf read
>> sf read.as -- array slow read
>> sf read.do --- dual out
>> sf read.qo -- quad out
>> sf read.dio -- dual io
>> sf read.qio -- quad io
>>
>> sf update  -- current update
>> sf update.pp.af -- write page program, read array fast, same as sf update
>> sf update.pp.as - write page program, read array slow
>> sf update.pp.do - write page program, read dual out
>> sf update.pp.qo - write page program, read quad out
>> sf update.pp.dio - write page program, read dual io
>> sf update.pp.qio - write page program, read quad io
>> sf update.qp.af - write quad program, read array fast
>> sf update.qp.as - write quad program, read array slow
>> sf update.qp.do - write quad program, read dual out
>> sf update.qp.qo - write quad program, read quad out
>> sf update.qp.dio - write quad program, read dual io
>> sf update.qp.qio - write quad program, read quad io
>>
>> Though it seems to be lengthy, but may useful with lot of combinations
>> from user.
>> My intention is to use the existing argument count with changes in the
>> command set.
>
> Are there cases where for the current device we're operating on that can
> handle more than one of these, aside from fast or slow?  And do we
> really need to offer both fast and slow?

Yes as per as I know spansion, numonyx and winbond flashes are supporting
all the operation modes that I listed above.

These are very generic w.r.t above flashes.or I can say these are commonly
available modes.

Thanks,
Jagan.

>
> --
> Tom

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 17:00       ` Jagan Teki
@ 2013-03-05 17:08         ` Tom Rini
  2013-03-05 17:21           ` Jagan Teki
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Rini @ 2013-03-05 17:08 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/05/2013 12:00 PM, Jagan Teki wrote:
> Hi Tom,
> 
> On Tue, Mar 5, 2013 at 2:38 AM, Tom Rini <trini@ti.com> wrote:
>> On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki wrote:
>> 
>> [snip]
>>> Since these changes I have sent long back, I am just
>>> re-modified the framework to add new features at the same time
>>> with backward comparability for current commands.
>>> 
>>> Current command setup: sf write sf read sf update
>>> 
>>> Changed command set: [no changes in the argument count] sf
>>> write     ---  current command sf write.pp --  same as sf
>>> write sf write.qp -- quad program
>>> 
>>> sf read   -- current read sf read.af --- array flast read, same
>>> as sf read sf read.as -- array slow read sf read.do --- dual
>>> out sf read.qo -- quad out sf read.dio -- dual io sf read.qio
>>> -- quad io
>>> 
>>> sf update  -- current update sf update.pp.af -- write page
>>> program, read array fast, same as sf update sf update.pp.as -
>>> write page program, read array slow sf update.pp.do - write
>>> page program, read dual out sf update.pp.qo - write page
>>> program, read quad out sf update.pp.dio - write page program,
>>> read dual io sf update.pp.qio - write page program, read quad
>>> io sf update.qp.af - write quad program, read array fast sf
>>> update.qp.as - write quad program, read array slow sf
>>> update.qp.do - write quad program, read dual out sf
>>> update.qp.qo - write quad program, read quad out sf
>>> update.qp.dio - write quad program, read dual io sf
>>> update.qp.qio - write quad program, read quad io
>>> 
>>> Though it seems to be lengthy, but may useful with lot of
>>> combinations from user. My intention is to use the existing
>>> argument count with changes in the command set.
>> 
>> Are there cases where for the current device we're operating on
>> that can handle more than one of these, aside from fast or slow?
>> And do we really need to offer both fast and slow?
> 
> Yes as per as I know spansion, numonyx and winbond flashes are
> supporting all the operation modes that I listed above.
> 
> These are very generic w.r.t above flashes.or I can say these are
> commonly available modes.

And when hooked up via whichever SPI controller you have, all of those
modes are also available and we can't really "guess" about using a
faster mode?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRNiaFAAoJENk4IS6UOR1WMykQAJy9vm+WJIDxVvUdP1jEBksH
mhOCcaSN7H7cbDxVvJ/9PBcN/9204X7Bf5Rg3k24rLO3QKIwJrtKP3hK9vQhGZRD
ikaS6tqOY9auJlww7ZZSbA8Kh7WbzXQ3h8TNUl/tKTU/CqqhXnZyA0N6ML547HQA
wn1+qRMSwduPZ7w0/276Ec2fRAhEK0IhXQ7KLBFaR8mman5sUB/IaRuE5oEtewaV
rpnOiwoUkih3lSq3oT51ooJFtQR4q8LvH0v272dMjFteFqjirpEdsTRY8MB1l+pb
GIGEFq/WHGkbrpzwEed9miTY1MhTZHQSRULErfgfsGck96a84J6CqGH1Se1GbRIP
5qjX4n9FyRyRMJ94wNju4Q3GMO9H04cQrxt+DACKPNAj0XoEkWZOa740fhn3rcaJ
a+pbEja9c0PiPzoSsO4Pa3jTmvsvUcwNGlBxubdcIEDENmii63aeH8LzaOCSKTkO
3ckE9pIzBJO+Oe5CW/7x9srGGSTsbyNgKVbgfTesS0DQYzxJ401o7Cq7hbojqTI9
TXy8RoJDbnxuIupm43K4Ci1dRZGX2g6/XrCexv7KIyiWgN1TQJwhJKXl74OH1RhL
4ZetFaJOHbGavaKGZK3ZTouQ3IjfYrnZ1FcijVW36fZredzHVFgG5HxGCE1VgKv7
vRG65lCA3cSh7jyu8BYH
=jrxq
-----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 17:08         ` Tom Rini
@ 2013-03-05 17:21           ` Jagan Teki
  2013-03-05 17:45             ` Tom Rini
  0 siblings, 1 reply; 30+ messages in thread
From: Jagan Teki @ 2013-03-05 17:21 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On Tue, Mar 5, 2013 at 10:38 PM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 03/05/2013 12:00 PM, Jagan Teki wrote:
>> Hi Tom,
>>
>> On Tue, Mar 5, 2013 at 2:38 AM, Tom Rini <trini@ti.com> wrote:
>>> On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki wrote:
>>>
>>> [snip]
>>>> Since these changes I have sent long back, I am just
>>>> re-modified the framework to add new features at the same time
>>>> with backward comparability for current commands.
>>>>
>>>> Current command setup: sf write sf read sf update
>>>>
>>>> Changed command set: [no changes in the argument count] sf
>>>> write     ---  current command sf write.pp --  same as sf
>>>> write sf write.qp -- quad program
>>>>
>>>> sf read   -- current read sf read.af --- array flast read, same
>>>> as sf read sf read.as -- array slow read sf read.do --- dual
>>>> out sf read.qo -- quad out sf read.dio -- dual io sf read.qio
>>>> -- quad io
>>>>
>>>> sf update  -- current update sf update.pp.af -- write page
>>>> program, read array fast, same as sf update sf update.pp.as -
>>>> write page program, read array slow sf update.pp.do - write
>>>> page program, read dual out sf update.pp.qo - write page
>>>> program, read quad out sf update.pp.dio - write page program,
>>>> read dual io sf update.pp.qio - write page program, read quad
>>>> io sf update.qp.af - write quad program, read array fast sf
>>>> update.qp.as - write quad program, read array slow sf
>>>> update.qp.do - write quad program, read dual out sf
>>>> update.qp.qo - write quad program, read quad out sf
>>>> update.qp.dio - write quad program, read dual io sf
>>>> update.qp.qio - write quad program, read quad io
>>>>
>>>> Though it seems to be lengthy, but may useful with lot of
>>>> combinations from user. My intention is to use the existing
>>>> argument count with changes in the command set.
>>>
>>> Are there cases where for the current device we're operating on
>>> that can handle more than one of these, aside from fast or slow?
>>> And do we really need to offer both fast and slow?
>>
>> Yes as per as I know spansion, numonyx and winbond flashes are
>> supporting all the operation modes that I listed above.
>>
>> These are very generic w.r.t above flashes.or I can say these are
>> commonly available modes.
>
> And when hooked up via whichever SPI controller you have, all of those
> modes are also available and we can't really "guess" about using a
> faster mode?
I didn't get you. In general, u-boot should provide all the possible
operation modes
and the choice should be up to user.
Please elaborate as I seem to be missing something.

Thanks,
Jagan.

>
> - --
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJRNiaFAAoJENk4IS6UOR1WMykQAJy9vm+WJIDxVvUdP1jEBksH
> mhOCcaSN7H7cbDxVvJ/9PBcN/9204X7Bf5Rg3k24rLO3QKIwJrtKP3hK9vQhGZRD
> ikaS6tqOY9auJlww7ZZSbA8Kh7WbzXQ3h8TNUl/tKTU/CqqhXnZyA0N6ML547HQA
> wn1+qRMSwduPZ7w0/276Ec2fRAhEK0IhXQ7KLBFaR8mman5sUB/IaRuE5oEtewaV
> rpnOiwoUkih3lSq3oT51ooJFtQR4q8LvH0v272dMjFteFqjirpEdsTRY8MB1l+pb
> GIGEFq/WHGkbrpzwEed9miTY1MhTZHQSRULErfgfsGck96a84J6CqGH1Se1GbRIP
> 5qjX4n9FyRyRMJ94wNju4Q3GMO9H04cQrxt+DACKPNAj0XoEkWZOa740fhn3rcaJ
> a+pbEja9c0PiPzoSsO4Pa3jTmvsvUcwNGlBxubdcIEDENmii63aeH8LzaOCSKTkO
> 3ckE9pIzBJO+Oe5CW/7x9srGGSTsbyNgKVbgfTesS0DQYzxJ401o7Cq7hbojqTI9
> TXy8RoJDbnxuIupm43K4Ci1dRZGX2g6/XrCexv7KIyiWgN1TQJwhJKXl74OH1RhL
> 4ZetFaJOHbGavaKGZK3ZTouQ3IjfYrnZ1FcijVW36fZredzHVFgG5HxGCE1VgKv7
> vRG65lCA3cSh7jyu8BYH
> =jrxq
> -----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 17:21           ` Jagan Teki
@ 2013-03-05 17:45             ` Tom Rini
  2013-03-05 18:34               ` Jagan Teki
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Rini @ 2013-03-05 17:45 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/05/2013 12:21 PM, Jagan Teki wrote:
> Hi Tom,
> 
> On Tue, Mar 5, 2013 at 10:38 PM, Tom Rini <trini@ti.com> wrote: On
> 03/05/2013 12:00 PM, Jagan Teki wrote:
>>>> Hi Tom,
>>>> 
>>>> On Tue, Mar 5, 2013 at 2:38 AM, Tom Rini <trini@ti.com>
>>>> wrote:
>>>>> On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki
>>>>> wrote:
>>>>> 
>>>>> [snip]
>>>>>> Since these changes I have sent long back, I am just 
>>>>>> re-modified the framework to add new features at the same
>>>>>> time with backward comparability for current commands.
>>>>>> 
>>>>>> Current command setup: sf write sf read sf update
>>>>>> 
>>>>>> Changed command set: [no changes in the argument count]
>>>>>> sf write     ---  current command sf write.pp --  same as
>>>>>> sf write sf write.qp -- quad program
>>>>>> 
>>>>>> sf read   -- current read sf read.af --- array flast
>>>>>> read, same as sf read sf read.as -- array slow read sf
>>>>>> read.do --- dual out sf read.qo -- quad out sf read.dio
>>>>>> -- dual io sf read.qio -- quad io
>>>>>> 
>>>>>> sf update  -- current update sf update.pp.af -- write
>>>>>> page program, read array fast, same as sf update sf
>>>>>> update.pp.as - write page program, read array slow sf
>>>>>> update.pp.do - write page program, read dual out sf
>>>>>> update.pp.qo - write page program, read quad out sf
>>>>>> update.pp.dio - write page program, read dual io sf
>>>>>> update.pp.qio - write page program, read quad io sf
>>>>>> update.qp.af - write quad program, read array fast sf 
>>>>>> update.qp.as - write quad program, read array slow sf 
>>>>>> update.qp.do - write quad program, read dual out sf 
>>>>>> update.qp.qo - write quad program, read quad out sf 
>>>>>> update.qp.dio - write quad program, read dual io sf 
>>>>>> update.qp.qio - write quad program, read quad io
>>>>>> 
>>>>>> Though it seems to be lengthy, but may useful with lot
>>>>>> of combinations from user. My intention is to use the
>>>>>> existing argument count with changes in the command set.
>>>>> 
>>>>> Are there cases where for the current device we're
>>>>> operating on that can handle more than one of these, aside
>>>>> from fast or slow? And do we really need to offer both fast
>>>>> and slow?
>>>> 
>>>> Yes as per as I know spansion, numonyx and winbond flashes
>>>> are supporting all the operation modes that I listed above.
>>>> 
>>>> These are very generic w.r.t above flashes.or I can say these
>>>> are commonly available modes.
> 
> And when hooked up via whichever SPI controller you have, all of
> those modes are also available and we can't really "guess" about
> using a faster mode?
>> I didn't get you. In general, u-boot should provide all the
>> possible operation modes and the choice should be up to user. 
>> Please elaborate as I seem to be missing something.

Unless I'm missing something (or we're talking about different
things), you have a SPI flash that do quad (or dual) read/write.  But
it also has to be connected to a controller that talks quad (or dual).

So is there any way we can make sf read/write/update be the fastest
supported (and why do we want fast and slow quad programming?) by the
controller and chip?  I can see having to do "sf read.qp.qf ..." and
so forth leading to some less than desired user interaction.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRNi8rAAoJENk4IS6UOR1WkAsP/3T7bdlRJAWtYFuUFoqF/Ofu
4LZZGcab75WwW7xUlHxct7oH8j4DE3c+PYT+RmxdpbhW7Av6VQJ9acDWRJE6upao
3M4WvTlDD2uyUEyMeWQWtgdcLNMP5eJcZ1CHP7MKYklW2qsdIbCZ4SnqseDqCIyb
7u2PPtCNWSVIn6tpihaM7hEho8KWa1YH7nnOfqF7V2629Tyah9IxX83VFLyP4Ujz
k/qRenQQN7CcKsyaSEnDNH3ZWH1FN0asrXZyxOcEUVaIca+pq4Z7QtmufS2I+e/b
DV2HUmI5UsC0USQrmaD21ais4taz4hZVNowEGcvw8Go0AnLkB7r+epksFr+Qf/T0
jm1XT3+pNEN4kt+Rgsuf+97ovOlNBWvIGmrYO104aDVg3+IVc8lkeb7STAKIAXah
d+WAA8kq36O0gj0enR4wsf96bzmn0aVUqBtuKmdwFP+W1U+q1Es/JodFW5niKMWn
i2VEMFZ0Oww+7VzWcIg5tT5t2FtkLkHRg+7QkFP0BIvd1+7rKpuEE+ntd9UBjFpr
ZzaC65D/aPtv9LvmTqOfRlk6nGxXmFJQqWbd0sv5hFEPp4pHMlkr8queKdRItW1E
dYu98kvcETdAI3AfgLcsyzjNOzPODRdlUiUmCDhcHW2Waxby7pmreeEdSJoHGeZt
QszhTVaNWgVq0qAFQVIc
=1CRc
-----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 17:45             ` Tom Rini
@ 2013-03-05 18:34               ` Jagan Teki
  2013-03-05 19:59                 ` Tom Rini
  0 siblings, 1 reply; 30+ messages in thread
From: Jagan Teki @ 2013-03-05 18:34 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On Tue, Mar 5, 2013 at 11:15 PM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 03/05/2013 12:21 PM, Jagan Teki wrote:
>> Hi Tom,
>>
>> On Tue, Mar 5, 2013 at 10:38 PM, Tom Rini <trini@ti.com> wrote: On
>> 03/05/2013 12:00 PM, Jagan Teki wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On Tue, Mar 5, 2013 at 2:38 AM, Tom Rini <trini@ti.com>
>>>>> wrote:
>>>>>> On Sat, Mar 02, 2013 at 01:59:38PM +0530, Jagan Teki
>>>>>> wrote:
>>>>>>
>>>>>> [snip]
>>>>>>> Since these changes I have sent long back, I am just
>>>>>>> re-modified the framework to add new features at the same
>>>>>>> time with backward comparability for current commands.
>>>>>>>
>>>>>>> Current command setup: sf write sf read sf update
>>>>>>>
>>>>>>> Changed command set: [no changes in the argument count]
>>>>>>> sf write     ---  current command sf write.pp --  same as
>>>>>>> sf write sf write.qp -- quad program
>>>>>>>
>>>>>>> sf read   -- current read sf read.af --- array flast
>>>>>>> read, same as sf read sf read.as -- array slow read sf
>>>>>>> read.do --- dual out sf read.qo -- quad out sf read.dio
>>>>>>> -- dual io sf read.qio -- quad io
>>>>>>>
>>>>>>> sf update  -- current update sf update.pp.af -- write
>>>>>>> page program, read array fast, same as sf update sf
>>>>>>> update.pp.as - write page program, read array slow sf
>>>>>>> update.pp.do - write page program, read dual out sf
>>>>>>> update.pp.qo - write page program, read quad out sf
>>>>>>> update.pp.dio - write page program, read dual io sf
>>>>>>> update.pp.qio - write page program, read quad io sf
>>>>>>> update.qp.af - write quad program, read array fast sf
>>>>>>> update.qp.as - write quad program, read array slow sf
>>>>>>> update.qp.do - write quad program, read dual out sf
>>>>>>> update.qp.qo - write quad program, read quad out sf
>>>>>>> update.qp.dio - write quad program, read dual io sf
>>>>>>> update.qp.qio - write quad program, read quad io
>>>>>>>
>>>>>>> Though it seems to be lengthy, but may useful with lot
>>>>>>> of combinations from user. My intention is to use the
>>>>>>> existing argument count with changes in the command set.
>>>>>>
>>>>>> Are there cases where for the current device we're
>>>>>> operating on that can handle more than one of these, aside
>>>>>> from fast or slow? And do we really need to offer both fast
>>>>>> and slow?
>>>>>
>>>>> Yes as per as I know spansion, numonyx and winbond flashes
>>>>> are supporting all the operation modes that I listed above.
>>>>>
>>>>> These are very generic w.r.t above flashes.or I can say these
>>>>> are commonly available modes.
>>
>> And when hooked up via whichever SPI controller you have, all of
>> those modes are also available and we can't really "guess" about
>> using a faster mode?
>>> I didn't get you. In general, u-boot should provide all the
>>> possible operation modes and the choice should be up to user.
>>> Please elaborate as I seem to be missing something.
>
> Unless I'm missing something (or we're talking about different
> things), you have a SPI flash that do quad (or dual) read/write.  But
> it also has to be connected to a controller that talks quad (or dual).
>
> So is there any way we can make sf read/write/update be the fastest
> supported (and why do we want fast and slow quad programming?) by the
> controller and chip?  I can see having to do "sf read.qp.qf ..." and
> so forth leading to some less than desired user interaction.

Okay.. what I understand is -
Currently, all the flash supported commands are provided to user as
choice. what what i suppose to implement.
He needs to choose the best option based on what the controller
and chip connected supports.
I guess your idea is to dynamically perform the fastest read/write
command based
on fastest command supported by controller and chip. I don't know how
to support this.

Thanks,
Jagan.

>
> - --
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJRNi8rAAoJENk4IS6UOR1WkAsP/3T7bdlRJAWtYFuUFoqF/Ofu
> 4LZZGcab75WwW7xUlHxct7oH8j4DE3c+PYT+RmxdpbhW7Av6VQJ9acDWRJE6upao
> 3M4WvTlDD2uyUEyMeWQWtgdcLNMP5eJcZ1CHP7MKYklW2qsdIbCZ4SnqseDqCIyb
> 7u2PPtCNWSVIn6tpihaM7hEho8KWa1YH7nnOfqF7V2629Tyah9IxX83VFLyP4Ujz
> k/qRenQQN7CcKsyaSEnDNH3ZWH1FN0asrXZyxOcEUVaIca+pq4Z7QtmufS2I+e/b
> DV2HUmI5UsC0USQrmaD21ais4taz4hZVNowEGcvw8Go0AnLkB7r+epksFr+Qf/T0
> jm1XT3+pNEN4kt+Rgsuf+97ovOlNBWvIGmrYO104aDVg3+IVc8lkeb7STAKIAXah
> d+WAA8kq36O0gj0enR4wsf96bzmn0aVUqBtuKmdwFP+W1U+q1Es/JodFW5niKMWn
> i2VEMFZ0Oww+7VzWcIg5tT5t2FtkLkHRg+7QkFP0BIvd1+7rKpuEE+ntd9UBjFpr
> ZzaC65D/aPtv9LvmTqOfRlk6nGxXmFJQqWbd0sv5hFEPp4pHMlkr8queKdRItW1E
> dYu98kvcETdAI3AfgLcsyzjNOzPODRdlUiUmCDhcHW2Waxby7pmreeEdSJoHGeZt
> QszhTVaNWgVq0qAFQVIc
> =1CRc
> -----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 18:34               ` Jagan Teki
@ 2013-03-05 19:59                 ` Tom Rini
  2013-03-08 12:22                   ` Michal Simek
  0 siblings, 1 reply; 30+ messages in thread
From: Tom Rini @ 2013-03-05 19:59 UTC (permalink / raw)
  To: u-boot

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/05/2013 01:34 PM, Jagan Teki wrote:

>> Okay.. what I understand is - Currently, all the flash supported
>> commands are provided to user as choice. what what i suppose to
>> implement. He needs to choose the best option based on what the
>> controller and chip connected supports. I guess your idea is to
>> dynamically perform the fastest read/write command based on
>> fastest command supported by controller and chip. I don't know
>> how to support this.

Dang, OK.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIbBAEBAgAGBQJRNk65AAoJENk4IS6UOR1Wg1wP92uphDCgIEaxfykmLfEOWM0Z
XnWUFEkFi5Qe2Q2HpxVl9sjPXLr2/YHSFvWWTuPPWakXDt1OlgTU4erD3Fl2eQfQ
Sz0xiP8MbE8MdL8gQ7mGsZYb7ENLBL8yiGmP4P6Bonm6fKPGZK8LyZMMIfI8mm3R
X7TQKuJsTrZ8lUNZMuh9chumAVyWJzP5vsnCCsJ26yX75eQ2J8jKo4K3nkwMX5GM
SdGzKtRhr8niB93hZ3Dym2l8PylJL6Vvxj2Do4RUOVKL1D7MtiGThRVNlTKEih7h
rc3zRjWZqoMaF6kpUwz7ffZrPEerckoA6JkEE9hx+TJwcs43TXC2Gn4IaNyp38pu
XCQYauQPHPB/5gYSIdoF1o+USUFIvDITFd6YXYT4KIrOVsUslQCMKHayozetWdQ+
3M0x0Xp3z53kn1YUBDKZ4z7juw2FdwTrZAxuXky8ET/zO8AU1gFaQivSk+YxNH4h
yPVFknCg1r1ehpPetin7NxxASUBw57J06xOHITStod6PlD2dLv5U1c4I1QMTFjgj
5pCYdQYpe45+J/D+jnwYhQtVncjIYUHLJxm19DYxYR4dXsn2eIWjsYFl4DnqI0n3
l+teqn6H5tGW5FyRtQ7wHipSdJKZwyKrkX4Gm4U/dA5WTKxiWUGhDytN8OzqRtmw
1vnfvgQdBsub1euZa/s=
=SYEM
-----END PGP SIGNATURE-----

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-05 19:59                 ` Tom Rini
@ 2013-03-08 12:22                   ` Michal Simek
  0 siblings, 0 replies; 30+ messages in thread
From: Michal Simek @ 2013-03-08 12:22 UTC (permalink / raw)
  To: u-boot

2013/3/5 Tom Rini <trini@ti.com>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 03/05/2013 01:34 PM, Jagan Teki wrote:
>
>>> Okay.. what I understand is - Currently, all the flash supported
>>> commands are provided to user as choice. what what i suppose to
>>> implement. He needs to choose the best option based on what the
>>> controller and chip connected supports. I guess your idea is to
>>> dynamically perform the fastest read/write command based on
>>> fastest command supported by controller and chip. I don't know
>>> how to support this.
>
> Dang, OK.

Any update on this?


Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-03-02  8:29   ` Jagan Teki
  2013-03-04 21:02     ` Jagan Teki
  2013-03-04 21:08     ` Tom Rini
@ 2013-03-08 23:55     ` Simon Glass
  2 siblings, 0 replies; 30+ messages in thread
From: Simon Glass @ 2013-03-08 23:55 UTC (permalink / raw)
  To: u-boot

Hi,

On Sat, Mar 2, 2013 at 12:29 AM, Jagan Teki <jagannadh.teki@gmail.com>wrote:

> Hi All,
>
> On Fri, Jan 11, 2013 at 7:46 AM, Simon Glass <sjg@chromium.org> wrote:
> > Hi Jagannadha,
> >
> > On Mon, Dec 31, 2012 at 3:13 AM, Jagannadha Sutradharudu Teki
> > <jagannadh.teki@gmail.com> wrote:
> >> All these patches are added a support for read and write instruction
> >> for programming/reading SPI flash.
> >>
> >
> > I think this is all great and very useful - since no one else has
> > commented I will have a try.
> >
> >> Read and Write instruction are implemented as a command line
> >> arguments for 'sf write' , 'sf read' and 'sf update' commands.
> >>
> >> Currently I have added below instructions those are commonly available
> >> on all flash types.
> >
> > Maybe you could use flags like -d for dual, -q for quad, -p for page,
> > -s for slow, -o for output only. So maybe:
> >
> > -p > pp - Page Program (existing one)
> > -qp > qpp - Quad-input Page Program
> > <empty> > afr - Array Fast Read (existing one)
> > -s > asr - Array Slow Read
> > -do > dofr - Dual Output Fast Read
> > -qo > qofr - Quad Output Fast Read
> > -d > diofr - Dual IO Fast Read
> > -q > qiofr - Quad IO Fast Read
> >
> > I worry that your scheme would be hard to remember.
> >
> >
> >>
> >> I have tested mostly of the instruction on real h/w.
> >>
> >> This entire implementation will change the current sf framework little
> bit but
> >> I thought these changes are worth to add.
> >
> > Yes very much so.
> >
> >>
> >> Request for all your comment, so-that I can move forward.
> >> Please let me know for any issue regarding this new implementation.
> >
> > Regards,
> > Simon
> >
> >>
> >> Thanks,
> >> Jagan.
> >>
> >> Jagannadha Sutradharudu Teki (12):
> >>   cmd_sf: Add wr_inst argument to 'sf write' command
> >>   cmd_sf: Add rd_inst argument to 'sf read' command
> >>   cmd_sf: Add wr_inst argument to 'sf update' command
> >>   cmd_sf: Add rd_inst argument to 'sf update' command
> >>   cmd_sf: Define a functions for parsing read and write instructions
> >>   cmd_sf: Add QPP(Quad-input Page Program) write instruction support
> >>   cmd_sf: Add ASR(Array Slow Read) read instruction support
> >>   cmd_sf: Add DOFR(Dual Output Fast Read) read instruction support
> >>   cmd_sf: Add QOFR(Quad Output Fast Read) read instruction support
> >>   cmd_sf: Add DIOFR(Dual IO Fast Read) read instruction support
> >>   cmd_sf: Add QIOFR(Quad IO Fast Read) read instruction support
> >>   sf: Pass rd_qeb_req variable as 0 for status and config reg reads
> >>
> >>  common/cmd_sf.c                      |  198
> +++++++++++++++++++++++++++++-----
> >>  drivers/mtd/spi/spi_flash.c          |   40 +++++--
> >>  drivers/mtd/spi/spi_flash_internal.h |   10 +-
> >>  include/spi_flash.h                  |   22 ++--
> >>  include/spi_flash_inst.h             |   39 +++++++
> >>  5 files changed, 257 insertions(+), 52 deletions(-)
> >>  create mode 100644 include/spi_flash_inst.h
> >>
>
> Since these changes I have sent long back, I am just re-modified the
> framework to
> add new features at the same time with backward comparability for
> current commands.
>
> Current command setup:
> sf write
> sf read
> sf update
>
> Changed command set: [no changes in the argument count]
> sf write     ---  current command
> sf write.pp --  same as sf write
> sf write.qp -- quad program
>
> sf read   -- current read
> sf read.af --- array flast read, same as sf read
> sf read.as -- array slow read
> sf read.do --- dual out
> sf read.qo -- quad out
> sf read.dio -- dual io
> sf read.qio -- quad io
>
> sf update  -- current update
> sf update.pp.af -- write page program, read array fast, same as sf update
> sf update.pp.as - write page program, read array slow
> sf update.pp.do - write page program, read dual out
> sf update.pp.qo - write page program, read quad out
> sf update.pp.dio - write page program, read dual io
> sf update.pp.qio - write page program, read quad io
> sf update.qp.af - write quad program, read array fast
> sf update.qp.as - write quad program, read array slow
> sf update.qp.do - write quad program, read dual out
> sf update.qp.qo - write quad program, read quad out
> sf update.qp.dio - write quad program, read dual io
> sf update.qp.qio - write quad program, read quad io
>
> Though it seems to be lengthy, but may useful with lot of combinations
> from user.
> My intention is to use the existing argument count with changes in the
> command set.
>
> Request for your inputs.
>


I think your new scheme sounds reasonable. Regarding the capability of
drivers to support these commands, I suppose you are going to need to add
new spi_slave flags which drivers can set, to indicate that each mode is
available? Still, the question is whether these modes are selected at 'sf
probe' time, or at 'sf read/write' time?

The latter is more flexible and makes it easier for cases where you must
use one more for read and another for write (no need to probe again in
between). But you will then need to add flags to spi_setup_slave() for
these modes, right?

Regards,
Simon


>
> Thanks,
> Jagan.
>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
>

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-01-16  7:47 ` Jagan Teki
@ 2013-01-16 19:51   ` Langer Thomas
  0 siblings, 0 replies; 30+ messages in thread
From: Langer Thomas @ 2013-01-16 19:51 UTC (permalink / raw)
  To: u-boot

Hello Jagan,

Jagan Teki wrote on?2013-01-16:

> Hi Thomas,
> 
> On Thu, Jan 10, 2013 at 8:03 PM, Langer Thomas (LQDE RD ST PON SW)
> <thomas.langer@lantiq.com> wrote:
>> Hello Jagan,
>> 
>>> All these patches are added a support for read and write instruction
>>> for programming/reading SPI flash.
>> 
>> I have written some weeks ago that I would really appreciate the
>> support of dual and quad IO accesses for serial flashes. I just  think,
>> this is not an acceptable way to do this.
>> 
> 
> First of all thank you very much for your comments.
> 
>> It is important to know for all, who do not know the details on the hardware
>> requirements of this feature, that these new transfers require a special SPI
>> controller! A standard SPI hardware, which is being used on most (maybe all)
>> existing boards, cannot use the dual or quad IO features at all!
> 
> I am some how unclear about your  concern.
> 
> Basically I have added this code that is purely specific to mtd flash
> layer, means
> the entire code is alters the functionalists of flash parts(stmicro,
> spansion, winbond..etc).
> 
> whether the respective SOC SPI/QSPI controller driver can support
> these transfer or not, may be
> we couldn't take care at this point of time.
> 
> Please comment if I am wrong with my analyses.

You are right, the MTD/SF layer does not know this. But in your board config you are
able to set a compile option to enable this feature.

It is an important rule for U-Boot, that new code must be added in a way that the code
size is not increased as long as the feature is disabled!

See here (Note 4): http://www.denx.de/wiki/view/U-Boot/Patches#Notes
In the current form, your patches increase the code size for all boards with serial flash!

> 
>> 
>> And in addition, I still haven't seen any change, which indicates the required
>> transfer mode to the SPI layer! How do you tell your SPI driver, which part
>> of the transfer should be done in dual or quad mode?
> 
> I don't think so, but the SPI/QSPI controller  driver should aware
> respective read/write/erase instructions
> transfer changes those are trying to pass from mtd flash. .
> 
> Suppose if the flash used QUAD PP , then the respective h/w SPI/QSPI
> controller must have a support to do the QUAD PP transfers (by adding
> QUAD_PP command transfer).
> 
> I think this job is specific to respective h/w driver, they need to
> add these transfers support.
> currently I have used a zynq qspi controller for testing these
> instruction by adding respective
> instructions are this driver.
> 
> http://git.xilinx.com/?p=u-boot-
> xlnx.git;a=blob;f=drivers/spi/zynq_qspi.c;h=642241d0fc25d04c06ecbeb5d01ba
> 0 6108307c62;hb=master
> 

I don't think, this is a good idea! You are mixing the SPI driver with details of the flashes!
What happens, if you connect something, which is not a flash, to this SPI controller?
And then try to send data, which is detected as "qpp" or any other quad/dual io instruction?
I assume, this will not work!

I would expect the SPI as transparent interface. The SPI driver should provide an interface
to specify the use of 1/2/4 signals for a transfer or part of it.
And the flash driver then should use this extended interface to specify the required parts,
as it is his job!

For example, with your driver, it would be difficult to support the Macronix MX25L12835F,
which allows to enable their "QPI" mode, which then expects some command, with the
same opcode as in "None-QPI" mode, have to be send in parallel.
And Macronix is also known to use different opcodes than others for some instructions.
Does this mean, for supporting this flash, you would have to extend your SPI driver first?

I don't expect something like this to be accepted, either in u-boot nor in the kernel.
Your driver has some complex handling inside for things, which are only valid for
serial flashes! Please don't do this!

Please define and use a clear interface, where the flash driver can tell the spi driver,
what kind of transfer should be used (1/2/4) for which phase (command/address/data).
In u-boot, I would use the "flags" argument of the "spi_xfer" function for this.

I would assume, you will get more flexibility, as your spi driver will get smaller and less
complex and it does not need to be changed, if some other flash type should be supported.
This will then be part of the specific flash driver! (you see??)

>> 
>>> 
>>> Read and Write instruction are implemented as a command line
>>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>> 
>>> Currently I have added below instructions those are commonly available
>>> on all flash types.
>>> pp - Page Program (existing one)
>>> qpp - Quad-input Page Program
>>> afr - Array Fast Read (existing one)
>>> asr - Array Slow Read
>>> dofr - Dual Output Fast Read
>>> qofr - Quad Output Fast Read
>>> diofr - Dual IO Fast Read
>>> qiofr - Quad IO Fast Read
>>> 
>>> I have tested mostly of the instruction on real h/w.
>>> 
>>> This entire implementation will change the current sf framework little
>>> bit but I thought these changes are worth to add.
>> 
>> This means, all your patches adding new code, which has no benefit for
>> existing boards, but changing the commands in an incompatible way,
>> which forces all users to adapt their definitions! I don't think this
>> will be accepted, as I would not.
>> 
>> The biggest question is: Do somebody really need the flexibility to
>> select the used instruction at this level? If you have a platform,
>> which contains an extended SPI controller and has also a supported
>> flash, I expect it would be okay to always use the dual or quad
>> instructions! And this can be completely handled by the lower level
>> functions, no need to expose this to the command line!
> 
> Yes, basically if some don't want/they don't support the respective
> instruction at their h/w or
> QSPI/SPI controller level certainly they couldn't use.
> instead they may use the respective supported instructions at command
> level, I suspect this could be
> a selective features from users point of view.
> 
>> 
>> I think, first of all, you should add some config options (e.g.
>> CONFIG_SYS_SF_DUAL / CONFIG_SYS_SF_QUAD) to indicate, that the platform
>> / board will support these. Then, during "sf probe", there should be a
>> detection, if the flash will support this (depending on the type) and
>> is enabled (checking the config bit for quad IO). Depending on this,
>> either specific functions for read/write could be assigned or some
>> other data, which indicate the usable instructions, can be set.
> 
> May be your taking about run-time detection of specific instruction
> supported by respective flash.
> Actually i don't have any idea how to detect whether the flash
> supports  pp, qpp, fr, sr, dor, qor?
> 
> Please pass any information if there is a flexible way to detect this feature.

I expect, that these flashes either have a specific ID and some flags can be added to the
tables, which are used to identify them already. Or some newer flashes might support
the JEDEC Standard JESD216: "Serial Flash Discoverable Parameters (SFDP)"?

For the quad-io mode, the corresponding bit from the configuration register should 
be used!

> 
> But currently these(pp, qpp, afr, asr, dor, qor, dior, qior)
> instructions are commonly supported once from
> spansion, numonyx and winbond flashes.this is the reason for me to
> give these instructions support at this point of time.

But it needs a spi driver with specific adaption to these commands,
instead of a generic interface, which can be adapted for other spi drivers also.

> 
> Thanks,
> Jagan.
> 

Best Regards,
Thomas

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-01-10 14:33 Langer Thomas
  2013-01-11  2:28 ` Simon Glass
@ 2013-01-16  7:47 ` Jagan Teki
  2013-01-16 19:51   ` Langer Thomas
  1 sibling, 1 reply; 30+ messages in thread
From: Jagan Teki @ 2013-01-16  7:47 UTC (permalink / raw)
  To: u-boot

Hi Thomas,

On Thu, Jan 10, 2013 at 8:03 PM, Langer Thomas (LQDE RD ST PON SW)
<thomas.langer@lantiq.com> wrote:
> Hello Jagan,
>
>> All these patches are added a support for read and write instruction
>> for programming/reading SPI flash.
>
> I have written some weeks ago that I would really appreciate the support of
> dual and quad IO accesses for serial flashes. I just  think, this is not an acceptable
> way to do this.
>

First of all thank you very much for your comments.

> It is important to know for all, who do not know the details on the hardware
> requirements of this feature, that these new transfers require a special SPI
> controller! A standard SPI hardware, which is being used on most (maybe all)
> existing boards, cannot use the dual or quad IO features at all!

I am some how unclear about your  concern.

Basically I have added this code that is purely specific to mtd flash
layer, means
the entire code is alters the functionalists of flash parts(stmicro,
spansion, winbond..etc).

whether the respective SOC SPI/QSPI controller driver can support
these transfer or not, may be
we couldn't take care at this point of time.

Please comment if I am wrong with my analyses.

>
> And in addition, I still haven't seen any change, which indicates the required
> transfer mode to the SPI layer! How do you tell your SPI driver, which part
> of the transfer should be done in dual or quad mode?

I don't think so, but the SPI/QSPI controller  driver should aware
respective read/write/erase instructions
transfer changes those are trying to pass from mtd flash. .

Suppose if the flash used QUAD PP , then the respective h/w SPI/QSPI
controller must
have a support to do the QUAD PP transfers (by adding QUAD_PP command transfer).

I think this job is specific to respective h/w driver, they need to
add these transfers support.
currently I have used a zynq qspi controller for testing these
instruction by adding respective
instructions are this driver.

http://git.xilinx.com/?p=u-boot-xlnx.git;a=blob;f=drivers/spi/zynq_qspi.c;h=642241d0fc25d04c06ecbeb5d01ba06108307c62;hb=master

>
>>
>> Read and Write instruction are implemented as a command line
>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>
>> Currently I have added below instructions those are commonly available
>> on all flash types.
>> pp - Page Program (existing one)
>> qpp - Quad-input Page Program
>> afr - Array Fast Read (existing one)
>> asr - Array Slow Read
>> dofr - Dual Output Fast Read
>> qofr - Quad Output Fast Read
>> diofr - Dual IO Fast Read
>> qiofr - Quad IO Fast Read
>>
>> I have tested mostly of the instruction on real h/w.
>>
>> This entire implementation will change the current sf framework little bit but
>> I thought these changes are worth to add.
>
> This means, all your patches adding new code, which has no benefit for existing
> boards, but changing the commands in an incompatible way, which forces all users
> to adapt their definitions!
> I don't think this will be accepted, as I would not.
>
> The biggest question is: Do somebody really need the flexibility to select the
> used instruction at this level?
> If you have a platform, which contains an extended SPI controller and has also a
> supported flash, I expect it would be okay to always use the dual or quad instructions!
> And this can be completely handled by the lower level functions, no need to expose
> this to the command line!

Yes, basically if some don't want/they don't support the respective
instruction at their h/w or
QSPI/SPI controller level certainly they couldn't use.
instead they may use the respective supported instructions at command
level, I suspect this could be
a selective features from users point of view.

>
> I think, first of all, you should add some config options (e.g. CONFIG_SYS_SF_DUAL /
> CONFIG_SYS_SF_QUAD) to indicate, that the platform / board will support these.
> Then, during "sf probe", there should be a detection, if the flash will support this
> (depending on the type) and is enabled (checking the config bit for quad IO).
> Depending on this, either specific functions for read/write could be assigned or
> some other data, which indicate the usable instructions, can be set.

May be your taking about run-time detection of specific instruction
supported by respective flash.
Actually i don't have any idea how to detect whether the flash
supports  pp, qpp, fr, sr, dor, qor?

Please pass any information if there is a flexible way to detect this feature.

But currently these(pp, qpp, afr, asr, dor, qor, dior, qior)
instructions are commonly supported once from
spansion, numonyx and winbond flashes.this is the reason for me to
give these instructions support at this point of time.

Thanks,
Jagan.

>
> As it depends on the flash type and manufacturer, how the lower sequence must
> look like (for the command/address/data phases, it might be 1-1-4, 1-4-4 or even 4-4-4),
> this should be flexible to be assigned from the detection code.
> For example, please check the differences between Spansion and Macronix!
>
>>
>> Request for all your comment, so-that I can move forward.
>> Please let me know for any issue regarding this new implementation.
>>
>> Thanks,
>> Jagan.
>>
>
> Please think about my comments. I just want to avoid to bloat the code with something,
> which is currently not usable for existing boards (as far as I know) and not flexible enough
> to support different, manufacturer specific command sets.
>
> Best Regards,
> Thomas
>

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
  2013-01-10 14:33 Langer Thomas
@ 2013-01-11  2:28 ` Simon Glass
  2013-01-16  7:47 ` Jagan Teki
  1 sibling, 0 replies; 30+ messages in thread
From: Simon Glass @ 2013-01-11  2:28 UTC (permalink / raw)
  To: u-boot

Hi,

On Thu, Jan 10, 2013 at 6:33 AM, Langer Thomas (LQDE RD ST PON SW)
<thomas.langer@lantiq.com> wrote:
> Hello Jagan,
>
>> All these patches are added a support for read and write instruction
>> for programming/reading SPI flash.

Hmmm just saw this comment.

>
> I have written some weeks ago that I would really appreciate the support of
> dual and quad IO accesses for serial flashes. I just  think, this is not an acceptable
> way to do this.
>
> It is important to know for all, who do not know the details on the hardware
> requirements of this feature, that these new transfers require a special SPI
> controller! A standard SPI hardware, which is being used on most (maybe all)
> existing boards, cannot use the dual or quad IO features at all!
>
> And in addition, I still haven't seen any change, which indicates the required
> transfer mode to the SPI layer! How do you tell your SPI driver, which part
> of the transfer should be done in dual or quad mode?
>
>>
>> Read and Write instruction are implemented as a command line
>> arguments for 'sf write' , 'sf read' and 'sf update' commands.
>>
>> Currently I have added below instructions those are commonly available
>> on all flash types.
>> pp - Page Program (existing one)
>> qpp - Quad-input Page Program
>> afr - Array Fast Read (existing one)
>> asr - Array Slow Read
>> dofr - Dual Output Fast Read
>> qofr - Quad Output Fast Read
>> diofr - Dual IO Fast Read
>> qiofr - Quad IO Fast Read
>>
>> I have tested mostly of the instruction on real h/w.
>>
>> This entire implementation will change the current sf framework little bit but
>> I thought these changes are worth to add.
>
> This means, all your patches adding new code, which has no benefit for existing
> boards, but changing the commands in an incompatible way, which forces all users
> to adapt their definitions!
> I don't think this will be accepted, as I would not.
>
> The biggest question is: Do somebody really need the flexibility to select the
> used instruction at this level?
> If you have a platform, which contains an extended SPI controller and has also a
> supported flash, I expect it would be okay to always use the dual or quad instructions!
> And this can be completely handled by the lower level functions, no need to expose
> this to the command line!
>
> I think, first of all, you should add some config options (e.g. CONFIG_SYS_SF_DUAL /
> CONFIG_SYS_SF_QUAD) to indicate, that the platform / board will support these.
> Then, during "sf probe", there should be a detection, if the flash will support this
> (depending on the type) and is enabled (checking the config bit for quad IO).
> Depending on this, either specific functions for read/write could be assigned or
> some other data, which indicate the usable instructions, can be set.

I suspect the SPI device will need to provide some flags to indicate
that it supports it, and then we will probably need some new SPI_...
flags in spi.h to select the required behaviour?

>
> As it depends on the flash type and manufacturer, how the lower sequence must
> look like (for the command/address/data phases, it might be 1-1-4, 1-4-4 or even 4-4-4),
> this should be flexible to be assigned from the detection code.
> For example, please check the differences between Spansion and Macronix!

Well maybe we could deal with that later? I think it would be good to
get some basic framework in there, and people can then adapt for their
chips, etc.

>
>>
>> Request for all your comment, so-that I can move forward.
>> Please let me know for any issue regarding this new implementation.
>>
>> Thanks,
>> Jagan.
>>
>
> Please think about my comments. I just want to avoid to bloat the code with something,
> which is currently not usable for existing boards (as far as I know) and not flexible enough
> to support different, manufacturer specific command sets.

I'm sure some boards can use it. I'm interested in knowing which board
this was tested on.

I sent a few patches to make it easier to extend SPI and SPI flash
interfaces without breaking old drivers.

http://patchwork.ozlabs.org/patch/208226/
http://patchwork.ozlabs.org/patch/208228/

Regards,
Simon

>
> Best Regards,
> Thomas
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions
@ 2013-01-10 14:33 Langer Thomas
  2013-01-11  2:28 ` Simon Glass
  2013-01-16  7:47 ` Jagan Teki
  0 siblings, 2 replies; 30+ messages in thread
From: Langer Thomas @ 2013-01-10 14:33 UTC (permalink / raw)
  To: u-boot

Hello Jagan,

> All these patches are added a support for read and write instruction
> for programming/reading SPI flash.

I have written some weeks ago that I would really appreciate the support of 
dual and quad IO accesses for serial flashes. I just  think, this is not an acceptable
way to do this.

It is important to know for all, who do not know the details on the hardware
requirements of this feature, that these new transfers require a special SPI
controller! A standard SPI hardware, which is being used on most (maybe all) 
existing boards, cannot use the dual or quad IO features at all! 

And in addition, I still haven't seen any change, which indicates the required 
transfer mode to the SPI layer! How do you tell your SPI driver, which part
of the transfer should be done in dual or quad mode?

> 
> Read and Write instruction are implemented as a command line
> arguments for 'sf write' , 'sf read' and 'sf update' commands.
> 
> Currently I have added below instructions those are commonly available
> on all flash types.
> pp - Page Program (existing one)
> qpp - Quad-input Page Program
> afr - Array Fast Read (existing one)
> asr - Array Slow Read
> dofr - Dual Output Fast Read
> qofr - Quad Output Fast Read
> diofr - Dual IO Fast Read
> qiofr - Quad IO Fast Read
> 
> I have tested mostly of the instruction on real h/w.
> 
> This entire implementation will change the current sf framework little bit but
> I thought these changes are worth to add.

This means, all your patches adding new code, which has no benefit for existing 
boards, but changing the commands in an incompatible way, which forces all users
to adapt their definitions!
I don't think this will be accepted, as I would not.

The biggest question is: Do somebody really need the flexibility to select the
used instruction at this level?
If you have a platform, which contains an extended SPI controller and has also a 
supported flash, I expect it would be okay to always use the dual or quad instructions!
And this can be completely handled by the lower level functions, no need to expose
this to the command line!

I think, first of all, you should add some config options (e.g. CONFIG_SYS_SF_DUAL / 
CONFIG_SYS_SF_QUAD) to indicate, that the platform / board will support these.
Then, during "sf probe", there should be a detection, if the flash will support this
(depending on the type) and is enabled (checking the config bit for quad IO).
Depending on this, either specific functions for read/write could be assigned or
some other data, which indicate the usable instructions, can be set.

As it depends on the flash type and manufacturer, how the lower sequence must
look like (for the command/address/data phases, it might be 1-1-4, 1-4-4 or even 4-4-4),
this should be flexible to be assigned from the detection code.
For example, please check the differences between Spansion and Macronix!

> 
> Request for all your comment, so-that I can move forward.
> Please let me know for any issue regarding this new implementation.
> 
> Thanks,
> Jagan.
> 

Please think about my comments. I just want to avoid to bloat the code with something,
which is currently not usable for existing boards (as far as I know) and not flexible enough
to support different, manufacturer specific command sets.

Best Regards,
Thomas

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

end of thread, other threads:[~2013-03-08 23:55 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-31 11:13 [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 01/12] cmd_sf: Add wr_inst argument to 'sf write' command Jagannadha Sutradharudu Teki
2013-01-11  2:11   ` Simon Glass
     [not found]     ` <CAD6G_RSQXL3RsSuK=4QkRGRNfzuKvU-s7JPrsuB+o5O7GRxJuA@mail.gmail.com>
2013-02-14 14:35       ` Simon Glass
2012-12-31 11:13 ` [U-Boot] [PATCH 02/12] cmd_sf: Add rd_inst argument to 'sf read' command Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 03/12] cmd_sf: Add wr_inst argument to 'sf update' command Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 04/12] cmd_sf: Add rd_inst " Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 05/12] cmd_sf: Define a functions for parsing read and write instructions Jagannadha Sutradharudu Teki
2013-01-11  2:18   ` Simon Glass
2012-12-31 11:13 ` [U-Boot] [PATCH 06/12] cmd_sf: Add QPP(Quad-input Page Program) write instruction support Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 07/12] cmd_sf: Add ASR(Array Slow Read) read " Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 08/12] cmd_sf: Add DOFR(Dual Output Fast " Jagannadha Sutradharudu Teki
2012-12-31 11:13 ` [U-Boot] [PATCH 09/12] cmd_sf: Add QOFR(Quad " Jagannadha Sutradharudu Teki
2013-01-11  2:16 ` [U-Boot] [PATCH 00/12] cmd_sf: Add support for read and write instructions Simon Glass
2013-01-11  2:16 ` Simon Glass
2013-03-02  8:29   ` Jagan Teki
2013-03-04 21:02     ` Jagan Teki
2013-03-04 21:08     ` Tom Rini
2013-03-05 17:00       ` Jagan Teki
2013-03-05 17:08         ` Tom Rini
2013-03-05 17:21           ` Jagan Teki
2013-03-05 17:45             ` Tom Rini
2013-03-05 18:34               ` Jagan Teki
2013-03-05 19:59                 ` Tom Rini
2013-03-08 12:22                   ` Michal Simek
2013-03-08 23:55     ` Simon Glass
2013-01-10 14:33 Langer Thomas
2013-01-11  2:28 ` Simon Glass
2013-01-16  7:47 ` Jagan Teki
2013-01-16 19:51   ` Langer Thomas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.