[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Wenyou Yang]

This series of patches are based and have been tested on the 'master'
branch of the u-boot.git tree.

Tests were passed with a sama5d2 xplained board which embeds both SPI and
QSPI controllers.

The following tests have been passed:

- QSPI0 + Macronix MX25L25673G:
  + probe: OK
  + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
  + Page Program 1-1-4 at offset 0x10000: OK
    The Macronix datasheet tells that only Page Program 1-4-4 is
    supported, not Page Program 1-1-4, however it worked, I don't know
    why...

- QSPI0 + Microchip SST26
  + probe: OK
  + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
  + Page Program 1-1-1 at offset 0x10000: OK
    SST26 memories support Page Program 1-4-4 but with the op code of
    Page Program 1-1-4, which is not standard so I don't use it.

- QSPI0 + Adesto AT25DF321A
  + probe: OK
  + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
  + Page Program 1-1-1 at offset 0x10000: OK

- SPI0 + Adesto AT25DF321A
  + probe: OK
  + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
  + Page Program 1-1-1 at offest 0x6000: OK

- SPI1 + Atmel AT45
  + probe: OK
  + Read at offset 0 and other than 0: OK
  + Write at offset 0 and other than 0: OK

During my tests, I used:
  - setenv/saveenv, reboot, printenv
  or
  - sf probe, sf read, sf write, sf erase and sf update.

Changes in v3:
 - Add the include <spi.h> to fix build error for corvus_defconfig.

Changes in v2:
 - Rebase on the latest u-boot/master(2710d54f5).

Cyrille Pitchen (8):
  spi: add support of SPI flash commands
  sf: describe all SPI flash commands with 'struct spi_flash_command'
  sf: select the relevant SPI flash protocol for read and write commands
  sf: differentiate Page Program 1-1-4 and 1-4-4
  sf: add 'addr_len' member to 'struct spi_flash'
  sf: add new option to support SPI flash above 16MiB
  sf: add support to Microchip SST26 QSPI memories
  sf: add driver for Atmel QSPI controller

 drivers/mtd/spi/Kconfig         |  16 +-
 drivers/mtd/spi/sf.c            |  78 ++++++--
 drivers/mtd/spi/sf_dataflash.c  | 119 ++++++------
 drivers/mtd/spi/sf_internal.h   |  48 +++--
 drivers/mtd/spi/spi_flash.c     | 341 +++++++++++++++++++++++----------
 drivers/mtd/spi/spi_flash_ids.c |   5 +
 drivers/spi/Kconfig             |   7 +
 drivers/spi/Makefile            |   1 +
 drivers/spi/atmel_qspi.c        | 404 ++++++++++++++++++++++++++++++++++++++++
 drivers/spi/atmel_qspi.h        | 169 +++++++++++++++++
 drivers/spi/spi-uclass.c        |  40 ++++
 drivers/spi/spi.c               |  13 ++
 include/spi.h                   | 168 +++++++++++++++++
 include/spi_flash.h             |   7 +
 14 files changed, 1226 insertions(+), 190 deletions(-)
 create mode 100644 drivers/spi/atmel_qspi.c
 create mode 100644 drivers/spi/atmel_qspi.h

-- 
2.13.0

[U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

This patch introduces 'struct spi_flash_command' and functions
spi_is_flash_command_supported() / spi_exec_flash_command().

The 'struct spi_flash_command' describes all the relevant parameters to
execute any SPI flash command:
- the instruction op code
- the number of bytes used to send the address: 0, 3 or 4 bytes
- the number of mode and wait-state clock cycles, also called dummy cycles
- the number and values of data bytes to be sent or received
- the SPI x-y-z protocol [1]
- the flash command type [2]

[1] SPI x-y-z protocol:
- x is the number of I/O lines used to send the instruction op code.
- y is the number of I/O lines used during address, mode and wait-state
  clock cycles.
- z is the number of I/O lines used to send or received data.

[2] Flash command type:
The flash command type is provided to differenciate "memory"
read/write/erase operations from "flash internal register" read/write
operations. Indeed some SPI controller drivers handle those command type
in different ways.
However SPI controller drivers should not check the value of the
instruction op code to guess the actual kind of flash command to perform.
Many instruction op codes are SPI flash manufacturer specific and only
drivers/mtd/spi/spi_flash.c should have the knowledge of all of them.

Besides, more and more QSPI controllers, like those of TI and Candence,
have special way to support (Fast) Read operations using some
"memory like" area mapped into the system bus. Hence, if those drivers
choose to override the default implementation of
spi_is_flash_command_supported() so that their own functions return true
only for a "memory read" flash command type, then spi_exec_flash_command()
might be used to implement the read from the "memory like" area mapped
into the system bus.
It means that spi_exec_flash_command() could be used to supersede the
actual flash->memory_map mechanism; spi_is_flash_command_supported() /
spi_exec_flash_command() being more generic and covering more use cases.

For instance, the Atmel QSPI hardware controller uses its "memory like"
area mapped ino the system to perform not only (Fast) Read operations but
actually all other types of flash commands. Hence the regular SPI API
based on the spi_xfer() function is not suited to support the Atmel QSPI
controller.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/spi/spi-uclass.c |  40 +++++++++++
 drivers/spi/spi.c        |  13 ++++
 include/spi.h            | 168 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)

diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index e06a603ab1..5de4510363 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -91,6 +91,30 @@ int dm_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	return spi_get_ops(bus)->xfer(dev, bitlen, dout, din, flags);
 }
 
+bool dm_spi_is_flash_command_supported(struct udevice *dev,
+				       const struct spi_flash_command *cmd)
+{
+	struct udevice *bus = dev->parent;
+	struct dm_spi_ops *ops = spi_get_ops(bus);
+
+	if (ops->is_flash_command_supported)
+		return ops->is_flash_command_supported(dev, cmd);
+
+	return false;
+}
+
+int dm_spi_exec_flash_command(struct udevice *dev,
+			      const struct spi_flash_command *cmd)
+{
+	struct udevice *bus = dev->parent;
+	struct dm_spi_ops *ops = spi_get_ops(bus);
+
+	if (ops->exec_flash_command)
+		return ops->exec_flash_command(dev, cmd);
+
+	return -EINVAL;
+}
+
 int spi_claim_bus(struct spi_slave *slave)
 {
 	return dm_spi_claim_bus(slave->dev);
@@ -107,6 +131,18 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
 	return dm_spi_xfer(slave->dev, bitlen, dout, din, flags);
 }
 
+bool spi_is_flash_command_supported(struct spi_slave *slave,
+				    const struct spi_flash_command *cmd)
+{
+	return dm_spi_is_flash_command_supported(slave->dev, cmd);
+}
+
+int spi_exec_flash_command(struct spi_slave *slave,
+			   const struct spi_flash_command *cmd)
+{
+	return dm_spi_exec_flash_command(slave->dev, cmd);
+}
+
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 static int spi_child_post_bind(struct udevice *dev)
 {
@@ -144,6 +180,10 @@ static int spi_post_probe(struct udevice *bus)
 		ops->set_mode += gd->reloc_off;
 	if (ops->cs_info)
 		ops->cs_info += gd->reloc_off;
+	if (ops->is_flash_command_supported)
+		ops->is_flash_command_supported += gd->reloc_off;
+	if (ops->exec_flash_command)
+		ops->exec_flash_command += gd->reloc_off;
 #endif
 
 	return 0;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7d81fbd7f8..e47acdc9e4 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
 #include <spi.h>
@@ -58,3 +59,15 @@ struct spi_slave *spi_base_setup_slave_fdt(const void *blob, int busnum,
 	return spi_setup_slave(busnum, cs, max_hz, mode);
 }
 #endif
+
+__weak bool spi_is_flash_command_supported(struct spi_slave *slave,
+					   const struct spi_flash_command *cmd)
+{
+	return false;
+}
+
+__weak int spi_exec_flash_command(struct spi_slave *slave,
+				  const struct spi_flash_command *cmd)
+{
+	return -EINVAL;
+}
diff --git a/include/spi.h b/include/spi.h
index 8c4b882c54..a090266b52 100644
--- a/include/spi.h
+++ b/include/spi.h
@@ -10,6 +10,8 @@
 #ifndef _SPI_H_
 #define _SPI_H_
 
+#include <linux/string.h> /* memset() */
+
 /* SPI mode flags */
 #define SPI_CPHA	BIT(0)			/* clock phase */
 #define SPI_CPOL	BIT(1)			/* clock polarity */
@@ -64,6 +66,116 @@ struct dm_spi_slave_platdata {
 #endif /* CONFIG_DM_SPI */
 
 /**
+ * enum spi_flash_protocol - SPI flash command protocol
+ */
+#define SPI_FPROTO_INST_SHIFT	16
+#define SPI_FPROTO_INST_MASK	GENMASK(23, 16)
+#define SPI_FPROTO_INST(nbits)					\
+	((((unsigned long)(nbits)) << SPI_FPROTO_INST_SHIFT) &	\
+	 SPI_FPROTO_INST_MASK)
+
+#define SPI_FPROTO_ADDR_SHIFT	8
+#define SPI_FPROTO_ADDR_MASK	GENMASK(15, 8)
+#define SPI_FPROTO_ADDR(nbits)					\
+	((((unsigned long)(nbits)) << SPI_FPROTO_ADDR_SHIFT) &	\
+	 SPI_FPROTO_ADDR_MASK)
+
+#define SPI_FPROTO_DATA_SHIFT	0
+#define SPI_FPROTO_DATA_MASK	GENMASK(7, 0)
+#define SPI_FPROTO_DATA(nbits)					\
+	((((unsigned long)(nbits)) << SPI_FPROTO_DATA_SHIFT) &	\
+	 SPI_FPROTO_DATA_MASK)
+
+#define SPI_FPROTO(inst_nbits, addr_nbits, data_nbits)	\
+	(SPI_FPROTO_INST(inst_nbits) |			\
+	 SPI_FPROTO_ADDR(addr_nbits) |			\
+	 SPI_FPROTO_DATA(data_nbits))
+
+enum spi_flash_protocol {
+	SPI_FPROTO_1_1_1 = SPI_FPROTO(1, 1, 1),
+	SPI_FPROTO_1_1_2 = SPI_FPROTO(1, 1, 2),
+	SPI_FPROTO_1_1_4 = SPI_FPROTO(1, 1, 4),
+	SPI_FPROTO_1_2_2 = SPI_FPROTO(1, 2, 2),
+	SPI_FPROTO_1_4_4 = SPI_FPROTO(1, 4, 4),
+	SPI_FPROTO_2_2_2 = SPI_FPROTO(2, 2, 2),
+	SPI_FPROTO_4_4_4 = SPI_FPROTO(4, 4, 4),
+};
+
+static inline
+u8 spi_flash_protocol_get_inst_nbits(enum spi_flash_protocol proto)
+{
+	return ((unsigned long)(proto & SPI_FPROTO_INST_MASK)) >>
+		SPI_FPROTO_INST_SHIFT;
+}
+
+static inline
+u8 spi_flash_protocol_get_addr_nbits(enum spi_flash_protocol proto)
+{
+	return ((unsigned long)(proto & SPI_FPROTO_ADDR_MASK)) >>
+		SPI_FPROTO_ADDR_SHIFT;
+}
+
+static inline
+u8 spi_flash_protocol_get_data_nbits(enum spi_flash_protocol proto)
+{
+	return ((unsigned long)(proto & SPI_FPROTO_DATA_MASK)) >>
+		SPI_FPROTO_DATA_SHIFT;
+}
+
+/**
+ * struct spi_flash_command - SPI flash command structure
+ *
+ * @instr:		Opcode sent to the SPI slave during instr clock cycles.
+ * @mode:		Value sent to the SPI slave during mode clock cycles.
+ * @num_mode_cycles:	Number of mode clock cycles.
+ * @num_wait_states:	Number of wait-state clock cycles.
+ * @addr_len:		Number of bytes sent during address clock cycles:
+ *			should be 0, 3, or 4.
+ * @addr:		Value sent to the SPI slave during address clock cycles.
+ * @data_len:		Number of bytes to be sent during data clock cycles.
+ * @tx_data:		Data sent to the SPI slave during data clock cycles.
+ * @rx_data:		Data read from the SPI slave during data clock cycles.
+ */
+struct spi_flash_command {
+	enum spi_flash_protocol proto;
+	u8 flags;
+#define SPI_FCMD_TYPE		GENMASK(2, 0)
+#define SPI_FCMD_READ		(0x0U << 0)
+#define SPI_FCMD_WRITE		(0x1U << 0)
+#define SPI_FCMD_ERASE		(0x2U << 0)
+#define SPI_FCMD_READ_REG	(0x3U << 0)
+#define SPI_FCMD_WRITE_REG	(0x4U << 0)
+
+	u8 inst;
+	u8 mode;
+	u8 num_mode_cycles;
+	u8 num_wait_states;
+	u8 addr_len;
+	u32 addr;
+	size_t data_len;
+	const void *tx_data;
+	void *rx_data;
+};
+
+/**
+ * Initialize a 'struct spi_flash_command'.
+ *
+ * @cmd:		Pointer to the 'struct spi_flash_command' to initialize.
+ * @instr:		Instruction opcode.
+ * @addr_len:		Number of address bytes.
+ */
+static inline void
+spi_flash_command_init(struct spi_flash_command *cmd,
+		       u8 inst, u8 addr_len, u8 flags)
+{
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->proto = SPI_FPROTO_1_1_1;
+	cmd->inst = inst;
+	cmd->addr_len = addr_len;
+	cmd->flags = flags;
+}
+
+/**
  * struct spi_slave - Representation of a SPI slave
  *
  * For driver model this is the per-child data used by the SPI bus. It can
@@ -252,6 +364,24 @@ int spi_set_wordlen(struct spi_slave *slave, unsigned int wordlen);
 int  spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout,
 		void *din, unsigned long flags);
 
+/**
+ * Check whether the given SPI flash command is supported
+ *
+ * @slave:	The SPI slave
+ * @cmd:	The SPI flash command to check.
+ */
+bool spi_is_flash_command_supported(struct spi_slave *slave,
+				    const struct spi_flash_command *cmd);
+
+/**
+ * Execute SPI flash command
+ *
+ * @slave:	The SPI slave which will execute the give SPI flash command.
+ * @cmd:	The SPI flash command to execute.
+ */
+int spi_exec_flash_command(struct spi_slave *slave,
+			   const struct spi_flash_command *cmd);
+
 /* Copy memory mapped data */
 void spi_flash_copy_mmap(void *data, void *offset, size_t len);
 
@@ -464,6 +594,26 @@ struct dm_spi_ops {
 	 *	   is invalid, other -ve value on error
 	 */
 	int (*cs_info)(struct udevice *bus, uint cs, struct spi_cs_info *info);
+
+	/**
+	 * Check whether the given SPI flash command is supported.
+	 *
+	 * @bus:	The SPI bus
+	 * @cmd:	The SPI flash command to check.
+	 * @return:	true if supported, false otherwise
+	 */
+	bool (*is_flash_command_supported)(struct udevice *bus,
+					   const struct spi_flash_command *cmd);
+
+	/**
+	 * Execute a SPI flash command
+	 *
+	 * @bus:	The SPI bus
+	 * @cmd:	The SPI flash command to execute.
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*exec_flash_command)(struct udevice *bus,
+				  const struct spi_flash_command *cmd);
 };
 
 struct dm_spi_emul_ops {
@@ -651,6 +801,24 @@ void dm_spi_release_bus(struct udevice *dev);
 int dm_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		const void *dout, void *din, unsigned long flags);
 
+/**
+ * Check whether the given SPI flash command is supported.
+ *
+ * @dev:	The SPI slave device
+ * @cmd:	The SPI flash command
+ */
+bool dm_spi_is_flash_command_supported(struct udevice *dev,
+				       const struct spi_flash_command *cmd);
+
+/**
+ * Execute the given SPI flash command.
+ *
+ * @dev:	The SPI slave device
+ * @cmd:	The SPI flash command
+ */
+int dm_spi_exec_flash_command(struct udevice *dev,
+			      const struct spi_flash_command *cmd);
+
 /* Access the operations for a SPI device */
 #define spi_get_ops(dev)	((struct dm_spi_ops *)(dev)->driver->ops)
 #define spi_emul_get_ops(dev)	((struct dm_spi_emul_ops *)(dev)->driver->ops)
-- 
2.13.0

[U-Boot] [PATCH v3 2/8] sf: describe all SPI flash commands with 'struct spi_flash_command'

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

Now that the SPI sub-system API has been extended with
'struct spi_flash_command' and spi_is_flash_command_supported() /
spi_exec_flash_command() functions, we update the SPI FLASH sub-system to
use this new API.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mtd/spi/sf.c           |  78 +++++++++++++----
 drivers/mtd/spi/sf_dataflash.c | 119 +++++++++++++-------------
 drivers/mtd/spi/sf_internal.h  |  24 +++---
 drivers/mtd/spi/spi_flash.c    | 184 +++++++++++++++++++++++------------------
 4 files changed, 236 insertions(+), 169 deletions(-)

diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c
index d5e175ca00..6178b0aa98 100644
--- a/drivers/mtd/spi/sf.c
+++ b/drivers/mtd/spi/sf.c
@@ -9,46 +9,88 @@
 
 #include <common.h>
 #include <spi.h>
+#include <spi_flash.h>
 
-static int spi_flash_read_write(struct spi_slave *spi,
-				const u8 *cmd, size_t cmd_len,
-				const u8 *data_out, u8 *data_in,
-				size_t data_len)
+#include "sf_internal.h"
+
+static void spi_flash_addr(u32 addr, u8 addr_len, u8 *cmd_buf)
 {
+	u8 i;
+
+	for (i = 0; i < addr_len; i++)
+		cmd_buf[i] = addr >> ((addr_len - 1 - i) * 8);
+}
+
+static u8 spi_compute_num_dummy_bytes(enum spi_flash_protocol proto,
+				      u8 num_dummy_clock_cycles)
+{
+	int shift = fls(spi_flash_protocol_get_addr_nbits(proto)) - 1;
+
+	if (shift < 0)
+		shift = 0;
+	return (num_dummy_clock_cycles << shift) >> 3;
+}
+
+static int spi_flash_exec(struct spi_flash *flash,
+			  const struct spi_flash_command *cmd)
+{
+	struct spi_slave *spi = flash->spi;
+	u8 cmd_buf[SPI_FLASH_CMD_LEN];
+	size_t cmd_len, num_dummy_bytes;
 	unsigned long flags = SPI_XFER_BEGIN;
 	int ret;
 
-	if (data_len == 0)
+	if (spi_is_flash_command_supported(spi, cmd))
+		return spi_exec_flash_command(spi, cmd);
+
+	if (cmd->data_len == 0)
 		flags |= SPI_XFER_END;
 
-	ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, flags);
+	cmd_buf[0] = cmd->inst;
+	spi_flash_addr(cmd->addr, cmd->addr_len, cmd_buf + 1);
+	cmd_len = 1 + cmd->addr_len;
+
+	num_dummy_bytes = spi_compute_num_dummy_bytes(cmd->proto,
+						      cmd->num_mode_cycles +
+						      cmd->num_wait_states);
+	memset(cmd_buf + cmd_len, 0xff, num_dummy_bytes);
+	cmd_len += num_dummy_bytes;
+
+	ret = spi_xfer(spi, cmd_len * 8, cmd_buf, NULL, flags);
 	if (ret) {
 		debug("SF: Failed to send command (%zu bytes): %d\n",
 		      cmd_len, ret);
-	} else if (data_len != 0) {
-		ret = spi_xfer(spi, data_len * 8, data_out, data_in,
-					SPI_XFER_END);
+	} else if (cmd->data_len != 0) {
+		ret = spi_xfer(spi, cmd->data_len * 8,
+			       cmd->tx_data, cmd->rx_data,
+			       SPI_XFER_END);
 		if (ret)
 			debug("SF: Failed to transfer %zu bytes of data: %d\n",
-			      data_len, ret);
+			      cmd->data_len, ret);
 	}
 
 	return ret;
 }
 
-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(struct spi_flash *flash,
+		       const struct spi_flash_command *cmd)
 {
-	return spi_flash_read_write(spi, cmd, cmd_len, NULL, data, data_len);
+	return spi_flash_exec(flash, cmd);
 }
 
-int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len)
+int spi_flash_cmd(struct spi_flash *flash, u8 instr, void *response, size_t len)
 {
-	return spi_flash_cmd_read(spi, &cmd, 1, response, len);
+	struct spi_flash_command cmd;
+	u8 flags = (response && len) ? SPI_FCMD_READ_REG : SPI_FCMD_WRITE_REG;
+
+	spi_flash_command_init(&cmd, instr, 0, flags);
+	cmd.data_len = len;
+	cmd.rx_data = response;
+	return spi_flash_exec(flash, &cmd);
 }
 
-int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
-		const void *data, size_t data_len)
+int spi_flash_cmd_write(struct spi_flash *flash,
+			const struct spi_flash_command *cmd)
 {
-	return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
+	return spi_flash_exec(flash, cmd);
 }
diff --git a/drivers/mtd/spi/sf_dataflash.c b/drivers/mtd/spi/sf_dataflash.c
index bcddfa0755..b2166ad4e5 100644
--- a/drivers/mtd/spi/sf_dataflash.c
+++ b/drivers/mtd/spi/sf_dataflash.c
@@ -73,7 +73,7 @@ struct dataflash {
 };
 
 /* Return the status of the DataFlash device */
-static inline int dataflash_status(struct spi_slave *spi)
+static inline int dataflash_status(struct spi_flash *spi_flash)
 {
 	int ret;
 	u8 status;
@@ -81,7 +81,7 @@ static inline int dataflash_status(struct spi_slave *spi)
 	 * NOTE:  at45db321c over 25 MHz wants to write
 	 * a dummy byte after the opcode...
 	 */
-	ret = spi_flash_cmd(spi, OP_READ_STATUS, &status, 1);
+	ret = spi_flash_cmd(spi_flash, OP_READ_STATUS, &status, 1);
 	return ret ? -EIO : status;
 }
 
@@ -90,7 +90,7 @@ static inline int dataflash_status(struct spi_slave *spi)
  * This usually takes 5-20 msec or so; more for sector erase.
  * ready: return > 0
  */
-static int dataflash_waitready(struct spi_slave *spi)
+static int dataflash_waitready(struct spi_flash *spi_flash)
 {
 	int status;
 	int timeout = 2 * CONFIG_SYS_HZ;
@@ -98,7 +98,7 @@ static int dataflash_waitready(struct spi_slave *spi)
 
 	timebase = get_timer(0);
 	do {
-		status = dataflash_status(spi);
+		status = dataflash_status(spi_flash);
 		if (status < 0)
 			status = 0;
 
@@ -114,11 +114,11 @@ static int dataflash_waitready(struct spi_slave *spi)
 /* Erase pages of flash */
 static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
 {
+	struct spi_flash_command	cmd;
 	struct dataflash	*dataflash;
 	struct spi_flash	*spi_flash;
 	struct spi_slave	*spi;
 	unsigned		blocksize;
-	uint8_t			*command;
 	uint32_t		rem;
 	int			status;
 
@@ -128,9 +128,6 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
 
 	blocksize = spi_flash->page_size << 3;
 
-	memset(dataflash->command, 0 , sizeof(dataflash->command));
-	command = dataflash->command;
-
 	debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
 
 	div_u64_rem(len, spi_flash->page_size, &rem);
@@ -146,6 +143,8 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
 		return status;
 	}
 
+	spi_flash_command_init(&cmd, OP_ERASE_BLOCK, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_ERASE);
 	while (len > 0) {
 		unsigned int	pageaddr;
 		int		do_block;
@@ -157,23 +156,24 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
 		do_block = (pageaddr & 0x7) == 0 && len >= blocksize;
 		pageaddr = pageaddr << dataflash->page_offset;
 
-		command[0] = do_block ? OP_ERASE_BLOCK : OP_ERASE_PAGE;
-		command[1] = (uint8_t)(pageaddr >> 16);
-		command[2] = (uint8_t)(pageaddr >> 8);
-		command[3] = 0;
+		cmd.inst = do_block ? OP_ERASE_BLOCK : OP_ERASE_PAGE;
+		cmd.addr = pageaddr & 0x00FFFF00;
 
 		debug("%s ERASE %s: (%x) %x %x %x [%d]\n",
 		      dev->name, do_block ? "block" : "page",
-		      command[0], command[1], command[2], command[3],
+		      cmd.inst,
+		      (cmd.addr >> 16) & 0xff,
+		      (cmd.addr >>  8) & 0xff,
+		      (cmd.addr >>  0) & 0xff,
 		      pageaddr);
 
-		status = spi_flash_cmd_write(spi, command, 4, NULL, 0);
+		status = spi_flash_cmd_write(spi_flash, &cmd);
 		if (status < 0) {
 			debug("%s: erase send command error!\n", dev->name);
 			return -EIO;
 		}
 
-		status = dataflash_waitready(spi);
+		status = dataflash_waitready(spi_flash);
 		if (status < 0) {
 			debug("%s: erase waitready error!\n", dev->name);
 			return status;
@@ -202,23 +202,18 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
 static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
 			      void *buf)
 {
+	struct spi_flash_command	cmd;
 	struct dataflash	*dataflash;
 	struct spi_flash	*spi_flash;
 	struct spi_slave	*spi;
 	unsigned int		addr;
-	uint8_t			*command;
 	int			status;
 
 	dataflash = dev_get_priv(dev);
 	spi_flash = dev_get_uclass_priv(dev);
 	spi = spi_flash->spi;
 
-	memset(dataflash->command, 0 , sizeof(dataflash->command));
-	command = dataflash->command;
-
-	debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
-	debug("READ: (%x) %x %x %x\n",
-	      command[0], command[1], command[2], command[3]);
+	debug("%s: read addr=0x%x len 0x%x\n", dev->name, offset, len);
 
 	/* Calculate flash page/byte address */
 	addr = (((unsigned)offset / spi_flash->page_size)
@@ -236,13 +231,15 @@ static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
 	 * the peak rate available.  Some chips support commands with
 	 * fewer "don't care" bytes.  Both buffers stay unchanged.
 	 */
-	command[0] = OP_READ_CONTINUOUS;
-	command[1] = (uint8_t)(addr >> 16);
-	command[2] = (uint8_t)(addr >> 8);
-	command[3] = (uint8_t)(addr >> 0);
+	spi_flash_command_init(&cmd, OP_READ_CONTINUOUS, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_READ);
+	cmd.addr = addr;
+	cmd.num_wait_states = 4 * 8; /* 4 "don't care" bytes */
+	cmd.data_len = len;
+	cmd.rx_data = buf;
 
 	/* plus 4 "don't care" bytes, command len: 4 + 4 "don't care" bytes */
-	status = spi_flash_cmd_read(spi, command, 8, buf, len);
+	status = spi_flash_cmd_read(spi_flash, &cmd);
 
 	spi_release_bus(spi);
 
@@ -258,10 +255,10 @@ static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
 int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 			const void *buf)
 {
+	struct spi_flash_command	cmd;
 	struct dataflash	*dataflash;
 	struct spi_flash	*spi_flash;
 	struct spi_slave	*spi;
-	uint8_t			*command;
 	unsigned int		pageaddr, addr, to, writelen;
 	size_t			remaining = len;
 	u_char			*writebuf = (u_char *)buf;
@@ -271,9 +268,6 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 	spi_flash = dev_get_uclass_priv(dev);
 	spi = spi_flash->spi;
 
-	memset(dataflash->command, 0 , sizeof(dataflash->command));
-	command = dataflash->command;
-
 	debug("%s: write 0x%x..0x%x\n", dev->name, offset, (offset + len));
 
 	pageaddr = ((unsigned)offset / spi_flash->page_size);
@@ -289,6 +283,8 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 		return status;
 	}
 
+	spi_flash_command_init(&cmd, OP_TRANSFER_BUF1, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_WRITE);
 	while (remaining > 0) {
 		debug("write @ %d:%d len=%d\n", pageaddr, to, writelen);
 
@@ -313,22 +309,25 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 
 		/* (1) Maybe transfer partial page to Buffer1 */
 		if (writelen != spi_flash->page_size) {
-			command[0] = OP_TRANSFER_BUF1;
-			command[1] = (addr & 0x00FF0000) >> 16;
-			command[2] = (addr & 0x0000FF00) >> 8;
-			command[3] = 0;
+			cmd.inst = OP_TRANSFER_BUF1;
+			cmd.addr = (addr & 0x00FFFF00);
+			cmd.data_len = 0;
+			cmd.tx_data = NULL;
 
 			debug("TRANSFER: (%x) %x %x %x\n",
-			      command[0], command[1], command[2], command[3]);
+			      cmd.inst,
+			      (cmd.addr >> 16) & 0xff,
+			      (cmd.addr >>  8) & 0xff,
+			      (cmd.addr >>  0) & 0xff);
 
-			status = spi_flash_cmd_write(spi, command, 4, NULL, 0);
+			status = spi_flash_cmd_write(spi_flash, &cmd);
 			if (status < 0) {
 				debug("%s: write(<pagesize) command error!\n",
 				      dev->name);
 				return -EIO;
 			}
 
-			status = dataflash_waitready(spi);
+			status = dataflash_waitready(spi_flash);
 			if (status < 0) {
 				debug("%s: write(<pagesize) waitready error!\n",
 				      dev->name);
@@ -338,22 +337,24 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 
 		/* (2) Program full page via Buffer1 */
 		addr += to;
-		command[0] = OP_PROGRAM_VIA_BUF1;
-		command[1] = (addr & 0x00FF0000) >> 16;
-		command[2] = (addr & 0x0000FF00) >> 8;
-		command[3] = (addr & 0x000000FF);
+		cmd.inst = OP_PROGRAM_VIA_BUF1;
+		cmd.addr = addr;
+		cmd.data_len = writelen;
+		cmd.tx_data = writebuf;
 
 		debug("PROGRAM: (%x) %x %x %x\n",
-		      command[0], command[1], command[2], command[3]);
+		      cmd.inst,
+		      (cmd.addr >> 16) & 0xff,
+		      (cmd.addr >>  8) & 0xff,
+		      (cmd.addr >>  0) & 0xff);
 
-		status = spi_flash_cmd_write(spi, command,
-					     4, writebuf, writelen);
+		status = spi_flash_cmd_write(spi_flash, &cmd);
 		if (status < 0) {
 			debug("%s: write send command error!\n", dev->name);
 			return -EIO;
 		}
 
-		status = dataflash_waitready(spi);
+		status = dataflash_waitready(spi_flash);
 		if (status < 0) {
 			debug("%s: write waitready error!\n", dev->name);
 			return status;
@@ -362,16 +363,18 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
 #ifdef CONFIG_SPI_DATAFLASH_WRITE_VERIFY
 		/* (3) Compare to Buffer1 */
 		addr = pageaddr << dataflash->page_offset;
-		command[0] = OP_COMPARE_BUF1;
-		command[1] = (addr & 0x00FF0000) >> 16;
-		command[2] = (addr & 0x0000FF00) >> 8;
-		command[3] = 0;
+		cmd.inst = OP_COMPARE_BUF1;
+		cmd.addr = addr & 0x00FFFF00;
+		cmd.data_len = writelen;
+		cmd.tx_data = writebuf;
 
 		debug("COMPARE: (%x) %x %x %x\n",
-		      command[0], command[1], command[2], command[3]);
+		      cmd.inst,
+		      (cmd.addr >> 16) & 0xff,
+		      (cmd.addr >>  8) & 0xff,
+		      (cmd.addr >>  0) & 0xff);
 
-		status = spi_flash_cmd_write(spi, command,
-					     4, writebuf, writelen);
+		status = spi_flash_cmd_write(spi, &cmd);
 		if (status < 0) {
 			debug("%s: write(compare) send command error!\n",
 			      dev->name);
@@ -496,7 +499,7 @@ static struct flash_info dataflash_data[] = {
 	{ "at45db642d",  0x1f2800, 8192, 1024, 10, SUP_POW2PS | IS_POW2PS},
 };
 
-static struct flash_info *jedec_probe(struct spi_slave *spi)
+static struct flash_info *jedec_probe(struct spi_flash *spi_flash)
 {
 	int			tmp;
 	uint8_t			id[5];
@@ -513,7 +516,7 @@ static struct flash_info *jedec_probe(struct spi_slave *spi)
 	 * That's not an error; only rev C and newer chips handle it, and
 	 * only Atmel sells these chips.
 	 */
-	tmp = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
+	tmp = spi_flash_cmd(spi_flash, CMD_READ_ID, id, sizeof(id));
 	if (tmp < 0) {
 		printf("dataflash: error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
@@ -532,7 +535,7 @@ static struct flash_info *jedec_probe(struct spi_slave *spi)
 			tmp++, info++) {
 		if (info->jedec_id == jedec) {
 			if (info->flags & SUP_POW2PS) {
-				status = dataflash_status(spi);
+				status = dataflash_status(spi_flash);
 				if (status < 0) {
 					debug("dataflash: status error %d\n",
 					      status);
@@ -596,7 +599,7 @@ static int spi_dataflash_probe(struct udevice *dev)
 	 * Both support the security register, though with different
 	 * write procedures.
 	 */
-	info = jedec_probe(spi);
+	info = jedec_probe(spi_flash);
 	if (IS_ERR(info))
 		goto err_jedec_probe;
 	if (info != NULL) {
@@ -611,7 +614,7 @@ static int spi_dataflash_probe(struct udevice *dev)
 	* Older chips support only legacy commands, identifing
 	* capacity using bits in the status byte.
 	*/
-	status = dataflash_status(spi);
+	status = dataflash_status(spi_flash);
 	if (status <= 0 || status == 0xff) {
 		printf("dataflash: read status error %d\n", status);
 		if (status == 0 || status == 0xff)
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 839cdbe1b0..5c551089d6 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -27,7 +27,7 @@ enum spi_nor_option_flags {
 };
 
 #define SPI_FLASH_3B_ADDR_LEN		3
-#define SPI_FLASH_CMD_LEN		(1 + SPI_FLASH_3B_ADDR_LEN)
+#define SPI_FLASH_CMD_LEN		(1 + SPI_FLASH_3B_ADDR_LEN + 16)
 #define SPI_FLASH_16MB_BOUN		0x1000000
 
 /* CFI Manufacture ID's */
@@ -137,21 +137,21 @@ struct spi_flash_info {
 extern const struct spi_flash_info spi_flash_ids[];
 
 /* Send a single-byte command to the device and read the response */
-int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
+int spi_flash_cmd(struct spi_flash *flash, u8 instr, void *response, size_t len);
 
 /*
  * Send a multi-byte command to the device and read the response. Used
  * for flash array reads, etc.
  */
-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(struct spi_flash *flash,
+		       const struct spi_flash_command *cmd);
 
 /*
  * Send a multi-byte command to the device followed by (optional)
  * data. Used for programming the flash array, etc.
  */
-int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
-		const void *data, size_t data_len);
+int spi_flash_cmd_write(struct spi_flash *flash,
+			const struct spi_flash_command *cmd);
 
 
 /* Flash erase(sectors) operation, support all possible erase commands */
@@ -169,13 +169,13 @@ int stm_is_locked(struct spi_flash *flash, u32 ofs, size_t len);
 /* Enable writing on the SPI flash */
 static inline int spi_flash_cmd_write_enable(struct spi_flash *flash)
 {
-	return spi_flash_cmd(flash->spi, CMD_WRITE_ENABLE, NULL, 0);
+	return spi_flash_cmd(flash, CMD_WRITE_ENABLE, NULL, 0);
 }
 
 /* Disable writing on the SPI flash */
 static inline int spi_flash_cmd_write_disable(struct spi_flash *flash)
 {
-	return spi_flash_cmd(flash->spi, CMD_WRITE_DISABLE, NULL, 0);
+	return spi_flash_cmd(flash, CMD_WRITE_DISABLE, NULL, 0);
 }
 
 /*
@@ -186,8 +186,8 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash)
  * - spi_flash_wait_till_ready
  * - SPI release
  */
-int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, const void *buf, size_t buf_len);
+int spi_flash_write_common(struct spi_flash *flash,
+			   const struct spi_flash_command *cmd);
 
 /*
  * Flash write operation, support all possible write commands.
@@ -201,8 +201,8 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
  * Same as spi_flash_cmd_read() except it also claims/releases the SPI
  * bus. Used as common part of the ->read() operation.
  */
-int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, void *data, size_t data_len);
+int spi_flash_read_common(struct spi_flash *flash,
+			  const struct spi_flash_command *cmd);
 
 /* Flash read operation, support all possible read commands */
 int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 0034a28d5f..80541d0ab4 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -22,21 +22,15 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static void spi_flash_addr(u32 addr, u8 *cmd)
-{
-	/* cmd[0] is actual command */
-	cmd[1] = addr >> 16;
-	cmd[2] = addr >> 8;
-	cmd[3] = addr >> 0;
-}
-
 static int read_sr(struct spi_flash *flash, u8 *rs)
 {
+	struct spi_flash_command cmd;
 	int ret;
-	u8 cmd;
 
-	cmd = CMD_READ_STATUS;
-	ret = spi_flash_read_common(flash, &cmd, 1, rs, 1);
+	spi_flash_command_init(&cmd, CMD_READ_STATUS, 0, SPI_FCMD_READ_REG);
+	cmd.data_len = 1;
+	cmd.rx_data = rs;
+	ret = spi_flash_read_common(flash, &cmd);
 	if (ret < 0) {
 		debug("SF: fail to read status register\n");
 		return ret;
@@ -47,10 +41,13 @@ static int read_sr(struct spi_flash *flash, u8 *rs)
 
 static int read_fsr(struct spi_flash *flash, u8 *fsr)
 {
+	struct spi_flash_command cmd;
 	int ret;
-	const u8 cmd = CMD_FLAG_STATUS;
 
-	ret = spi_flash_read_common(flash, &cmd, 1, fsr, 1);
+	spi_flash_command_init(&cmd, CMD_FLAG_STATUS, 0, SPI_FCMD_READ_REG);
+	cmd.data_len = 1;
+	cmd.rx_data = fsr;
+	ret = spi_flash_read_common(flash, &cmd);
 	if (ret < 0) {
 		debug("SF: fail to read flag status register\n");
 		return ret;
@@ -61,11 +58,13 @@ static int read_fsr(struct spi_flash *flash, u8 *fsr)
 
 static int write_sr(struct spi_flash *flash, u8 ws)
 {
-	u8 cmd;
+	struct spi_flash_command cmd;
 	int ret;
 
-	cmd = CMD_WRITE_STATUS;
-	ret = spi_flash_write_common(flash, &cmd, 1, &ws, 1);
+	spi_flash_command_init(&cmd, CMD_WRITE_STATUS, 0, SPI_FCMD_WRITE_REG);
+	cmd.data_len = 1;
+	cmd.tx_data = &ws;
+	ret = spi_flash_write_common(flash, &cmd);
 	if (ret < 0) {
 		debug("SF: fail to write status register\n");
 		return ret;
@@ -77,11 +76,13 @@ static int write_sr(struct spi_flash *flash, u8 ws)
 #if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
 static int read_cr(struct spi_flash *flash, u8 *rc)
 {
+	struct spi_flash_command cmd;
 	int ret;
-	u8 cmd;
 
-	cmd = CMD_READ_CONFIG;
-	ret = spi_flash_read_common(flash, &cmd, 1, rc, 1);
+	spi_flash_command_init(&cmd, CMD_READ_CONFIG, 0, SPI_FCMD_READ_REG);
+	cmd.data_len = 1;
+	cmd.rx_data = rc;
+	ret = spi_flash_read_common(flash, &cmd);
 	if (ret < 0) {
 		debug("SF: fail to read config register\n");
 		return ret;
@@ -92,17 +93,19 @@ static int read_cr(struct spi_flash *flash, u8 *rc)
 
 static int write_cr(struct spi_flash *flash, u8 wc)
 {
+	struct spi_flash_command cmd;
 	u8 data[2];
-	u8 cmd;
 	int ret;
 
 	ret = read_sr(flash, &data[0]);
 	if (ret < 0)
 		return ret;
 
-	cmd = CMD_WRITE_STATUS;
+	spi_flash_command_init(&cmd, CMD_WRITE_STATUS, 0, SPI_FCMD_WRITE_REG);
+	cmd.data_len = 2;
+	cmd.tx_data = data;
 	data[1] = wc;
-	ret = spi_flash_write_common(flash, &cmd, 1, &data, 2);
+	ret = spi_flash_write_common(flash, &cmd);
 	if (ret) {
 		debug("SF: fail to write config register\n");
 		return ret;
@@ -115,15 +118,19 @@ static int write_cr(struct spi_flash *flash, u8 wc)
 #ifdef CONFIG_SPI_FLASH_BAR
 static int write_bar(struct spi_flash *flash, u32 offset)
 {
-	u8 cmd, bank_sel;
+	struct spi_flash_command cmd;
+	u8 bank_sel;
 	int ret;
 
 	bank_sel = offset / (SPI_FLASH_16MB_BOUN << flash->shift);
 	if (bank_sel == flash->bank_curr)
 		goto bar_end;
 
-	cmd = flash->bank_write_cmd;
-	ret = spi_flash_write_common(flash, &cmd, 1, &bank_sel, 1);
+	spi_flash_command_init(&cmd, flash->bank_write_cmd, 0,
+			       SPI_FCMD_WRITE_REG);
+	cmd.data_len = 1;
+	cmd.tx_data = &bank_sel;
+	ret = spi_flash_write_common(flash, &cmd);
 	if (ret < 0) {
 		debug("SF: fail to write bank register\n");
 		return ret;
@@ -136,6 +143,7 @@ bar_end:
 
 static int read_bar(struct spi_flash *flash, const struct spi_flash_info *info)
 {
+	struct spi_flash_command cmd;
 	u8 curr_bank = 0;
 	int ret;
 
@@ -152,8 +160,11 @@ static int read_bar(struct spi_flash *flash, const struct spi_flash_info *info)
 		flash->bank_write_cmd = CMD_EXTNADDR_WREAR;
 	}
 
-	ret = spi_flash_read_common(flash, &flash->bank_read_cmd, 1,
-				    &curr_bank, 1);
+	spi_flash_command_init(&cmd, flash->bank_read_cmd, 0,
+			       SPI_FCMD_READ_REG);
+	cmd.data_len = 1;
+	cmd.rx_data = &curr_bank;
+	ret = spi_flash_read_common(flash, &cmd);
 	if (ret) {
 		debug("SF: fail to read bank addr register\n");
 		return ret;
@@ -250,14 +261,14 @@ static int spi_flash_wait_till_ready(struct spi_flash *flash,
 	return -ETIMEDOUT;
 }
 
-int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, const void *buf, size_t buf_len)
+int spi_flash_write_common(struct spi_flash *flash,
+			   const struct spi_flash_command *cmd)
 {
 	struct spi_slave *spi = flash->spi;
 	unsigned long timeout = SPI_FLASH_PROG_TIMEOUT;
 	int ret;
 
-	if (buf == NULL)
+	if (cmd->tx_data == NULL)
 		timeout = SPI_FLASH_PAGE_ERASE_TIMEOUT;
 
 	ret = spi_claim_bus(spi);
@@ -272,7 +283,7 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
 		return ret;
 	}
 
-	ret = spi_flash_cmd_write(spi, cmd, cmd_len, buf, buf_len);
+	ret = spi_flash_cmd_write(flash, cmd);
 	if (ret < 0) {
 		debug("SF: write cmd failed\n");
 		return ret;
@@ -293,8 +304,8 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
 
 int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 {
+	struct spi_flash_command cmd;
 	u32 erase_size, erase_addr;
-	u8 cmd[SPI_FLASH_CMD_LEN];
 	int ret = -1;
 
 	erase_size = flash->erase_size;
@@ -311,7 +322,8 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 		}
 	}
 
-	cmd[0] = flash->erase_cmd;
+	spi_flash_command_init(&cmd, flash->erase_cmd, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_ERASE);
 	while (len) {
 		erase_addr = offset;
 
@@ -324,12 +336,15 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 		if (ret < 0)
 			return ret;
 #endif
-		spi_flash_addr(erase_addr, cmd);
+		cmd.addr = erase_addr;
 
-		debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1],
-		      cmd[2], cmd[3], erase_addr);
+		debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd.inst,
+		      (cmd.addr >> 16) & 0xff,
+		      (cmd.addr >>  8) & 0xff,
+		      (cmd.addr >>  0) & 0xff,
+		      erase_addr);
 
-		ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0);
+		ret = spi_flash_write_common(flash, &cmd);
 		if (ret < 0) {
 			debug("SF: erase failed\n");
 			break;
@@ -346,10 +361,10 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		size_t len, const void *buf)
 {
 	struct spi_slave *spi = flash->spi;
+	struct spi_flash_command cmd;
 	unsigned long byte_addr, page_size;
 	u32 write_addr;
 	size_t chunk_len, actual;
-	u8 cmd[SPI_FLASH_CMD_LEN];
 	int ret = -1;
 
 	page_size = flash->page_size;
@@ -362,7 +377,8 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		}
 	}
 
-	cmd[0] = flash->write_cmd;
+	spi_flash_command_init(&cmd, flash->write_cmd, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_WRITE);
 	for (actual = 0; actual < len; actual += chunk_len) {
 		write_addr = offset;
 
@@ -382,13 +398,18 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 			chunk_len = min(chunk_len,
 					(size_t)spi->max_write_size);
 
-		spi_flash_addr(write_addr, cmd);
+		cmd.addr = write_addr;
+		cmd.data_len = chunk_len;
+		cmd.tx_data = buf + actual;
 
 		debug("SF: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = %zu\n",
-		      buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
+		      buf + actual, cmd.inst,
+		      (cmd.addr >> 16) & 0xff,
+		      (cmd.addr >>  8) & 0xff,
+		      (cmd.addr >>  0) & 0xff,
+		      chunk_len);
 
-		ret = spi_flash_write_common(flash, cmd, sizeof(cmd),
-					buf + actual, chunk_len);
+		ret = spi_flash_write_common(flash, &cmd);
 		if (ret < 0) {
 			debug("SF: write failed\n");
 			break;
@@ -400,8 +421,8 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 	return ret;
 }
 
-int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
-		size_t cmd_len, void *data, size_t data_len)
+int spi_flash_read_common(struct spi_flash *flash,
+			  const struct spi_flash_command *cmd)
 {
 	struct spi_slave *spi = flash->spi;
 	int ret;
@@ -412,7 +433,7 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 		return ret;
 	}
 
-	ret = spi_flash_cmd_read(spi, cmd, cmd_len, data, data_len);
+	ret = spi_flash_cmd_read(flash, cmd);
 	if (ret < 0) {
 		debug("SF: read cmd failed\n");
 		return ret;
@@ -440,7 +461,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		size_t len, void *data)
 {
 	struct spi_slave *spi = flash->spi;
-	u8 *cmd, cmdsz;
+	struct spi_flash_command cmd;
 	u32 remain_len, read_len, read_addr;
 	int bank_sel = 0;
 	int ret = -1;
@@ -459,14 +480,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		return 0;
 	}
 
-	cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
-	cmd = calloc(1, cmdsz);
-	if (!cmd) {
-		debug("SF: Failed to allocate cmd\n");
-		return -ENOMEM;
-	}
-
-	cmd[0] = flash->read_cmd;
+	spi_flash_command_init(&cmd, flash->read_cmd, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_READ);
+	cmd.num_wait_states = flash->dummy_byte * 8;
 	while (len) {
 		read_addr = offset;
 
@@ -487,9 +503,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		else
 			read_len = remain_len;
 
-		spi_flash_addr(read_addr, cmd);
-
-		ret = spi_flash_read_common(flash, cmd, cmdsz, data, read_len);
+		cmd.addr = read_addr;
+		cmd.data_len = read_len;
+		cmd.rx_data = data;
+		ret = spi_flash_read_common(flash, &cmd);
 		if (ret < 0) {
 			debug("SF: read failed\n");
 			break;
@@ -500,30 +517,33 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		data += read_len;
 	}
 
-	free(cmd);
 	return ret;
 }
 
 #ifdef CONFIG_SPI_FLASH_SST
 static int sst_byte_write(struct spi_flash *flash, u32 offset, const void *buf)
 {
-	struct spi_slave *spi = flash->spi;
+	struct spi_flash_command cmd;
 	int ret;
-	u8 cmd[4] = {
-		CMD_SST_BP,
-		offset >> 16,
-		offset >> 8,
-		offset,
-	};
+	u8 sr = 0xFFu;
+
+	spi_flash_command_init(&cmd, CMD_SST_BP, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_WRITE);
+	cmd.addr = offset;
+	cmd.data_len = 1;
+	cmd.tx_data = buf;
 
+#ifdef DEBUG
+	read_sr(flash, &sr);
+#endif
 	debug("BP[%02x]: 0x%p => cmd = { 0x%02x 0x%06x }\n",
-	      spi_w8r8(spi, CMD_READ_STATUS), buf, cmd[0], offset);
+	      sr, buf, cmd.inst, offset);
 
 	ret = spi_flash_cmd_write_enable(flash);
 	if (ret)
 		return ret;
 
-	ret = spi_flash_cmd_write(spi, cmd, sizeof(cmd), buf, 1);
+	ret = spi_flash_cmd_write(flash, &cmd);
 	if (ret)
 		return ret;
 
@@ -534,9 +554,10 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 		const void *buf)
 {
 	struct spi_slave *spi = flash->spi;
-	size_t actual, cmd_len;
+	struct spi_flash_command cmd;
+	size_t actual;
 	int ret;
-	u8 cmd[4];
+	u8 sr = 0xFFu;
 
 	ret = spi_claim_bus(spi);
 	if (ret) {
@@ -557,19 +578,20 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 	if (ret)
 		goto done;
 
-	cmd_len = 4;
-	cmd[0] = CMD_SST_AAI_WP;
-	cmd[1] = offset >> 16;
-	cmd[2] = offset >> 8;
-	cmd[3] = offset;
+	spi_flash_command_init(&cmd, CMD_SST_AAI_WP, SPI_FLASH_3B_ADDR_LEN,
+			       SPI_FCMD_WRITE);
+	cmd.addr = offset;
+	cmd.data_len = 2;
 
 	for (; actual < len - 1; actual += 2) {
+#ifdef DEBUG
+		read_sr(flash, &sr);
+#endif
 		debug("WP[%02x]: 0x%p => cmd = { 0x%02x 0x%06x }\n",
-		      spi_w8r8(spi, CMD_READ_STATUS), buf + actual,
-		      cmd[0], offset);
+		      sr, buf + actual, cmd.inst, offset);
 
-		ret = spi_flash_cmd_write(spi, cmd, cmd_len,
-					buf + actual, 2);
+		cmd.tx_data = buf + actual;
+		ret = spi_flash_cmd_write(flash, &cmd);
 		if (ret) {
 			debug("SF: sst word program failed\n");
 			break;
@@ -579,7 +601,7 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 		if (ret)
 			break;
 
-		cmd_len = 1;
+		cmd.addr_len = 0;
 		offset += 2;
 	}
 
@@ -869,7 +891,7 @@ static const struct spi_flash_info *spi_flash_read_id(struct spi_flash *flash)
 	u8				id[SPI_FLASH_MAX_ID_LEN];
 	const struct spi_flash_info	*info;
 
-	tmp = spi_flash_cmd(flash->spi, CMD_READ_ID, id, SPI_FLASH_MAX_ID_LEN);
+	tmp = spi_flash_cmd(flash, CMD_READ_ID, id, SPI_FLASH_MAX_ID_LEN);
 	if (tmp < 0) {
 		printf("SF: error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
-- 
2.13.0

[U-Boot] [PATCH v3 3/8] sf: select the relevant SPI flash protocol for read and write commands

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

SPI controller drivers should not check the instruction op code byte to
guess which SPI x-y-z protocol is to be used for Fast Read or Page Program
operations.

Indeed, the op code values are not so reliable. For instance, the 32h op
code is generally used for Page Program 1-1-4 operations. However
Microchip SST26 memories use this 32h op code for their Page Program 1-4-4
operations. There are many other examples of those SPI flash manufacturer
quirks.

Instead, the SPI FLASH sub-system now fills the 'proto' member
of 'struct spi_flash_command' with flash->read_proto for Fast Read
operations and flash->write_proto for Page Program operations.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3:
 - Add the include <spi.h> to fix build error for corvus_defconfig.

Changes in v2: None

 drivers/mtd/spi/spi_flash.c | 24 ++++++++++++++++--------
 include/spi_flash.h         |  5 +++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 80541d0ab4..588e5729b4 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -379,6 +379,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 
 	spi_flash_command_init(&cmd, flash->write_cmd, SPI_FLASH_3B_ADDR_LEN,
 			       SPI_FCMD_WRITE);
+	cmd.proto = flash->write_proto;
 	for (actual = 0; actual < len; actual += chunk_len) {
 		write_addr = offset;
 
@@ -482,6 +483,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 
 	spi_flash_command_init(&cmd, flash->read_cmd, SPI_FLASH_3B_ADDR_LEN,
 			       SPI_FCMD_READ);
+	cmd.proto = flash->read_proto;
 	cmd.num_wait_states = flash->dummy_byte * 8;
 	while (len) {
 		read_addr = offset;
@@ -1044,24 +1046,30 @@ int spi_flash_scan(struct spi_flash *flash)
 
 	/* Look for read commands */
 	flash->read_cmd = CMD_READ_ARRAY_FAST;
-	if (spi->mode & SPI_RX_SLOW)
+	flash->read_proto = SPI_FPROTO_1_1_1;
+	if (spi->mode & SPI_RX_SLOW) {
 		flash->read_cmd = CMD_READ_ARRAY_SLOW;
-	else if (spi->mode & SPI_RX_QUAD && info->flags & RD_QUAD)
+	} else if (spi->mode & SPI_RX_QUAD && info->flags & RD_QUAD) {
 		flash->read_cmd = CMD_READ_QUAD_OUTPUT_FAST;
-	else if (spi->mode & SPI_RX_DUAL && info->flags & RD_DUAL)
+		flash->read_proto = SPI_FPROTO_1_1_4;
+	} else if (spi->mode & SPI_RX_DUAL && info->flags & RD_DUAL) {
 		flash->read_cmd = CMD_READ_DUAL_OUTPUT_FAST;
+		flash->read_proto = SPI_FPROTO_1_1_2;
+	}
 
 	/* Look for write commands */
-	if (info->flags & WR_QPP && spi->mode & SPI_TX_QUAD)
+	if (info->flags & WR_QPP && spi->mode & SPI_TX_QUAD) {
 		flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
-	else
+		flash->write_proto = SPI_FPROTO_1_1_4;
+	} else {
 		/* Go for default supported write cmd */
 		flash->write_cmd = CMD_PAGE_PROGRAM;
+		flash->write_proto = SPI_FPROTO_1_1_1;
+	}
 
 	/* Set the quad enable bit - only for quad commands */
-	if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
-	    (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
-	    (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
+	if (spi_flash_protocol_get_data_nbits(flash->read_proto) == 4 ||
+	    spi_flash_protocol_get_data_nbits(flash->write_proto) == 4) {
 		ret = set_quad_mode(flash, info);
 		if (ret) {
 			debug("SF: Fail to set QEB for %02x\n",
diff --git a/include/spi_flash.h b/include/spi_flash.h
index be2fe3f84c..688a1708fd 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -11,6 +11,7 @@
 #define _SPI_FLASH_H_
 
 #include <dm.h>	/* Because we dereference struct udevice here */
+#include <spi.h>
 #include <linux/types.h>
 
 #ifndef CONFIG_SF_DEFAULT_SPEED
@@ -48,6 +49,8 @@ struct spi_slave;
  * @read_cmd:		Read cmd - Array Fast, Extn read and quad read.
  * @write_cmd:		Write cmd - page and quad program.
  * @dummy_byte:		Dummy cycles for read operation.
+ * @read_proto:		SPI x-y-z protocol for flash read ops
+ * @write_proto:	SPI x-y-z protocol for flash write ops
  * @memory_map:		Address of read-only SPI flash access
  * @flash_lock:		lock a region of the SPI Flash
  * @flash_unlock:	unlock a region of the SPI Flash
@@ -83,6 +86,8 @@ struct spi_flash {
 	u8 read_cmd;
 	u8 write_cmd;
 	u8 dummy_byte;
+	enum spi_flash_protocol read_proto;
+	enum spi_flash_protocol write_proto;
 
 	void *memory_map;
 
-- 
2.13.0

[U-Boot] [PATCH v3 4/8] sf: differentiate Page Program 1-1-4 and 1-4-4

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

This patch simply renames the ambiguous CMD_QUAD_PAGE_PROGRAM macro
into the more explicit CMD_PAGE_PROGRAM_1_1_4.
Also it defines the CMD_PAGE_PROGRAM_1_4_4 macro to the standard 38h op
code.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mtd/spi/sf_internal.h | 3 ++-
 drivers/mtd/spi/spi_flash.c   | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 5c551089d6..8b8c951bcc 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -48,7 +48,8 @@ enum spi_nor_option_flags {
 #define CMD_PAGE_PROGRAM		0x02
 #define CMD_WRITE_DISABLE		0x04
 #define CMD_WRITE_ENABLE		0x06
-#define CMD_QUAD_PAGE_PROGRAM		0x32
+#define CMD_PAGE_PROGRAM_1_1_4		0x32
+#define CMD_PAGE_PROGRAM_1_4_4		0x38
 
 /* Read commands */
 #define CMD_READ_ARRAY_SLOW		0x03
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 588e5729b4..8712e5eef0 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -1059,7 +1059,7 @@ int spi_flash_scan(struct spi_flash *flash)
 
 	/* Look for write commands */
 	if (info->flags & WR_QPP && spi->mode & SPI_TX_QUAD) {
-		flash->write_cmd = CMD_QUAD_PAGE_PROGRAM;
+		flash->write_cmd = CMD_PAGE_PROGRAM_1_1_4;
 		flash->write_proto = SPI_FPROTO_1_1_4;
 	} else {
 		/* Go for default supported write cmd */
-- 
2.13.0

[U-Boot] [PATCH v3 5/8] sf: add 'addr_len' member to 'struct spi_flash'

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

This is a transitional patch to prepare the SPI FLASH sub-system to
support the 4-byte address instruction set later.
For now, flash->addr_len is always set to SPI_FLASH_3B_ADDR_LEN.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mtd/spi/spi_flash.c | 13 ++++++++-----
 include/spi_flash.h         |  2 ++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 8712e5eef0..d6942d57b2 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -322,7 +322,7 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
 		}
 	}
 
-	spi_flash_command_init(&cmd, flash->erase_cmd, SPI_FLASH_3B_ADDR_LEN,
+	spi_flash_command_init(&cmd, flash->erase_cmd, flash->addr_len,
 			       SPI_FCMD_ERASE);
 	while (len) {
 		erase_addr = offset;
@@ -377,7 +377,7 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
 		}
 	}
 
-	spi_flash_command_init(&cmd, flash->write_cmd, SPI_FLASH_3B_ADDR_LEN,
+	spi_flash_command_init(&cmd, flash->write_cmd, flash->addr_len,
 			       SPI_FCMD_WRITE);
 	cmd.proto = flash->write_proto;
 	for (actual = 0; actual < len; actual += chunk_len) {
@@ -481,7 +481,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		return 0;
 	}
 
-	spi_flash_command_init(&cmd, flash->read_cmd, SPI_FLASH_3B_ADDR_LEN,
+	spi_flash_command_init(&cmd, flash->read_cmd, flash->addr_len,
 			       SPI_FCMD_READ);
 	cmd.proto = flash->read_proto;
 	cmd.num_wait_states = flash->dummy_byte * 8;
@@ -529,7 +529,7 @@ static int sst_byte_write(struct spi_flash *flash, u32 offset, const void *buf)
 	int ret;
 	u8 sr = 0xFFu;
 
-	spi_flash_command_init(&cmd, CMD_SST_BP, SPI_FLASH_3B_ADDR_LEN,
+	spi_flash_command_init(&cmd, CMD_SST_BP, flash->addr_len,
 			       SPI_FCMD_WRITE);
 	cmd.addr = offset;
 	cmd.data_len = 1;
@@ -580,7 +580,7 @@ int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 	if (ret)
 		goto done;
 
-	spi_flash_command_init(&cmd, CMD_SST_AAI_WP, SPI_FLASH_3B_ADDR_LEN,
+	spi_flash_command_init(&cmd, CMD_SST_AAI_WP, flash->addr_len,
 			       SPI_FCMD_WRITE);
 	cmd.addr = offset;
 	cmd.data_len = 2;
@@ -1102,6 +1102,9 @@ int spi_flash_scan(struct spi_flash *flash)
 		flash->flags |= SNOR_F_USE_FSR;
 #endif
 
+	/* Set the address length */
+	flash->addr_len = SPI_FLASH_3B_ADDR_LEN;
+
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
 	ret = read_bar(flash, info);
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 688a1708fd..09a81fdd76 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -45,6 +45,7 @@ struct spi_slave;
  * @bank_read_cmd:	Bank read cmd
  * @bank_write_cmd:	Bank write cmd
  * @bank_curr:		Current flash bank
+ * @addr_len:		Number of bytes for the address
  * @erase_cmd:		Erase cmd 4K, 32K, 64K
  * @read_cmd:		Read cmd - Array Fast, Extn read and quad read.
  * @write_cmd:		Write cmd - page and quad program.
@@ -82,6 +83,7 @@ struct spi_flash {
 	u8 bank_write_cmd;
 	u8 bank_curr;
 #endif
+	u8 addr_len;
 	u8 erase_cmd;
 	u8 read_cmd;
 	u8 write_cmd;
-- 
2.13.0

[U-Boot] [PATCH v3 6/8] sf: add new option to support SPI flash above 16MiB

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

The patch provides an alternative method to support SPI flash size greater
than 16MiB (128Mib).

Indeed using the Base Address Register (BAR) is stateful. Hence, once the
BAR has been modified, if a spurious CPU reset occurs with no reset/power
off at the SPI flash side, early boot loarders may try to read from offset
0 but fails because of the new BAR value.

On the other hand, using the 4-byte address instruction set is stateless.
When supported by the SPI flash memory, it allows us to access memory data
area above 16MiB without changing the internal state of this SPI flash
memory. Then if a spirious reboot occurs, early boot loaders can still
access data from offset 0.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mtd/spi/Kconfig       | 16 +++++++-
 drivers/mtd/spi/sf_internal.h | 18 +++++++++
 drivers/mtd/spi/spi_flash.c   | 92 ++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
index 5700859ff2..985aefd96c 100644
--- a/drivers/mtd/spi/Kconfig
+++ b/drivers/mtd/spi/Kconfig
@@ -34,14 +34,28 @@ config SPI_FLASH
 
 	  If unsure, say N
 
+choice
+	prompt "Support SPI flash above 16MiB"
+	depends on SPI_FLASH
+	optional
+
 config SPI_FLASH_BAR
 	bool "SPI flash Bank/Extended address register support"
-	depends on SPI_FLASH
 	help
 	  Enable the SPI flash Bank/Extended address register support.
 	  Bank/Extended address registers are used to access the flash
 	  which has size > 16MiB in 3-byte addressing.
 
+config SPI_FLASH_4BAIS
+	bool "SPI flash 4-byte address instruction set support"
+	help
+	  Convert the selected 3-byte address op codes into their associated
+	  4-byte address op codes. Using this instruction set does not change
+	  the internal state of the SPI flash device.
+
+endchoice
+
+
 config SF_DUAL_FLASH
 	bool "SPI DUAL flash memory support"
 	depends on SPI_FLASH
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 8b8c951bcc..30994f9f46 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -27,6 +27,7 @@ enum spi_nor_option_flags {
 };
 
 #define SPI_FLASH_3B_ADDR_LEN		3
+#define SPI_FLASH_4B_ADDR_LEN		4
 #define SPI_FLASH_CMD_LEN		(1 + SPI_FLASH_3B_ADDR_LEN + 16)
 #define SPI_FLASH_16MB_BOUN		0x1000000
 
@@ -64,6 +65,19 @@ enum spi_nor_option_flags {
 #define CMD_READ_CONFIG			0x35
 #define CMD_FLAG_STATUS			0x70
 
+/* 4-byte address instruction set */
+#define CMD_READ_ARRAY_SLOW_4B		0x13
+#define CMD_READ_ARRAY_FAST_4B		0x0c
+#define CMD_READ_DUAL_OUTPUT_FAST_4B	0x3c
+#define CMD_READ_DUAL_IO_FAST_4B	0xbc
+#define CMD_READ_QUAD_OUTPUT_FAST_4B	0x6c
+#define CMD_READ_QUAD_IO_FAST_4B	0xec
+#define CMD_PAGE_PROGRAM_4B		0x12
+#define CMD_PAGE_PROGRAM_1_1_4_4B	0x34
+#define CMD_PAGE_PROGRAM_1_4_4_4B	0x3e
+#define CMD_ERASE_4K_4B			0x21
+#define CMD_ERASE_64K_4B		0xdc
+
 /* Bank addr access commands */
 #ifdef CONFIG_SPI_FLASH_BAR
 # define CMD_BANKADDR_BRWR		0x17
@@ -133,6 +147,10 @@ struct spi_flash_info {
 #define RD_QUADIO		BIT(6)	/* use Quad IO Read */
 #define RD_DUALIO		BIT(7)	/* use Dual IO Read */
 #define RD_FULL			(RD_QUAD | RD_DUAL | RD_QUADIO | RD_DUALIO)
+#define NO_4BAIS		BIT(8)	/*
+					 * 4-byte address instruction set
+					 * NOT supported
+					 */
 };
 
 extern const struct spi_flash_info spi_flash_ids[];
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index d6942d57b2..89ceae2221 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -176,6 +176,67 @@ bar_end:
 }
 #endif
 
+#ifdef CONFIG_SPI_FLASH_4BAIS
+static u8 spi_flash_convert_opcode(u8 opcode, const u8 table[][2], size_t size)
+{
+	size_t i;
+
+	for (i = 0; i < size; i++)
+		if (table[i][0] == opcode)
+			return table[i][1];
+
+	/* No conversion found, keep input op code. */
+	return opcode;
+}
+
+static u8 spi_flash_convert_3to4_read(u8 opcode)
+{
+	static const u8 spi_flash_3to4_read[][2] = {
+		{CMD_READ_ARRAY_SLOW,		CMD_READ_ARRAY_SLOW_4B},
+		{CMD_READ_ARRAY_FAST,		CMD_READ_ARRAY_FAST_4B},
+		{CMD_READ_DUAL_OUTPUT_FAST,	CMD_READ_DUAL_OUTPUT_FAST_4B},
+		{CMD_READ_DUAL_IO_FAST,		CMD_READ_DUAL_IO_FAST_4B},
+		{CMD_READ_QUAD_OUTPUT_FAST,	CMD_READ_QUAD_OUTPUT_FAST_4B},
+		{CMD_READ_QUAD_IO_FAST,		CMD_READ_QUAD_IO_FAST_4B},
+	};
+
+	return spi_flash_convert_opcode(opcode, spi_flash_3to4_read,
+					ARRAY_SIZE(spi_flash_3to4_read));
+}
+
+static u8 spi_flash_convert_3to4_write(u8 opcode)
+{
+	static const u8 spi_flash_3to4_write[][2] = {
+		{CMD_PAGE_PROGRAM,		CMD_PAGE_PROGRAM_4B},
+		{CMD_PAGE_PROGRAM_1_1_4,	CMD_PAGE_PROGRAM_1_1_4_4B},
+		{CMD_PAGE_PROGRAM_1_4_4,	CMD_PAGE_PROGRAM_1_4_4_4B},
+	};
+
+	return spi_flash_convert_opcode(opcode, spi_flash_3to4_write,
+					ARRAY_SIZE(spi_flash_3to4_write));
+}
+
+static u8 spi_flash_convert_3to4_erase(u8 opcode)
+{
+	static const u8 spi_flash_3to4_erase[][2] = {
+		{CMD_ERASE_4K,	CMD_ERASE_4K_4B},
+		{CMD_ERASE_64K,	CMD_ERASE_64K_4B},
+	};
+
+	return spi_flash_convert_opcode(opcode, spi_flash_3to4_erase,
+					ARRAY_SIZE(spi_flash_3to4_erase));
+}
+
+static void spi_flash_set_4byte_addr_opcodes(struct spi_flash *flash,
+					     const struct spi_flash_info *info)
+{
+	flash->read_cmd = spi_flash_convert_3to4_read(flash->read_cmd);
+	flash->write_cmd = spi_flash_convert_3to4_write(flash->write_cmd);
+	flash->erase_cmd = spi_flash_convert_3to4_erase(flash->erase_cmd);
+	flash->addr_len = SPI_FLASH_4B_ADDR_LEN;
+}
+#endif
+
 #ifdef CONFIG_SF_DUAL_FLASH
 static void spi_flash_dual(struct spi_flash *flash, u32 *addr)
 {
@@ -965,6 +1026,7 @@ int spi_flash_scan(struct spi_flash *flash)
 {
 	struct spi_slave *spi = flash->spi;
 	const struct spi_flash_info *info = NULL;
+	bool above_16MB;
 	int ret;
 
 	info = spi_flash_read_id(flash);
@@ -1105,6 +1167,26 @@ int spi_flash_scan(struct spi_flash *flash)
 	/* Set the address length */
 	flash->addr_len = SPI_FLASH_3B_ADDR_LEN;
 
+	above_16MB = ((flash->dual_flash == SF_SINGLE_FLASH) &&
+		      (flash->size > SPI_FLASH_16MB_BOUN)) ||
+		     ((flash->dual_flash > SF_SINGLE_FLASH) &&
+		      (flash->size > SPI_FLASH_16MB_BOUN << 1));
+
+	/*
+	 * replace the selected 3-byte address op codes with the associated
+	 * 4-byte address op codes, if needed (flash->size > 16 MiB)
+	 */
+#ifdef CONFIG_SPI_FLASH_4BAIS
+	if (above_16MB) {
+		if (info->flags & NO_4BAIS) {
+			puts("SF: Warning - Only lower 16MiB accessible,");
+			puts(" 4-byte address instruction set not supported\n");
+		} else {
+			spi_flash_set_4byte_addr_opcodes(flash, info);
+		}
+	}
+#endif
+
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
 	ret = read_bar(flash, info);
@@ -1130,13 +1212,11 @@ int spi_flash_scan(struct spi_flash *flash)
 	puts("\n");
 #endif
 
-#ifndef CONFIG_SPI_FLASH_BAR
-	if (((flash->dual_flash == SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN)) ||
-	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
-	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
+#if !defined(CONFIG_SPI_FLASH_BAR) && !defined(CONFIG_SPI_FLASH_4BAIS)
+	if (above_16MB) {
 		puts("SF: Warning - Only lower 16MiB accessible,");
-		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
+		puts(" Full access #define CONFIG_SPI_FLASH_BAR");
+		puts(" or CONFIG_SPI_FLASH_4BAIS\n");
 	}
 #endif
 
-- 
2.13.0

[U-Boot] [PATCH v3 7/8] sf: add support to Microchip SST26 QSPI memories

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

This patch adds support to Microchip SST26 QSPI memories.

Erase blocks are protected at power up and must be unlocked first before
being erased then programmed.

Also, the erase block sizes are not uniform. The memory layout is uniform
only for the 4K sector blocks. The 64K Block Erase (D8h) op code cannot be
used as currently done by the SPI FLASH sub-system.
The 4K Sector Erase (20h) op code should be chosen instead even if
CONFIG_SPI_FLASH_USE_4K_SECTORS is not set.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2: None

 drivers/mtd/spi/sf_internal.h   |  3 +++
 drivers/mtd/spi/spi_flash.c     | 36 ++++++++++++++++++++++++++++++++----
 drivers/mtd/spi/spi_flash_ids.c |  5 +++++
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 30994f9f46..4354a2aa53 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -104,6 +104,7 @@ enum spi_nor_option_flags {
 #ifdef CONFIG_SPI_FLASH_SST
 # define CMD_SST_BP		0x02    /* Byte Program */
 # define CMD_SST_AAI_WP		0xAD	/* Auto Address Incr Word Program */
+# define CMD_SST_ULBPR		0x98	/* Global Block Protection Unlock */
 
 int sst_write_wp(struct spi_flash *flash, u32 offset, size_t len,
 		const void *buf);
@@ -151,6 +152,8 @@ struct spi_flash_info {
 					 * 4-byte address instruction set
 					 * NOT supported
 					 */
+#define SECT_4K_ONLY		BIT(9)  /* use only CMD_ERASE_4K */
+#define SST_ULBPR		BIT(10)	/* use SST unlock block protection */
 };
 
 extern const struct spi_flash_info spi_flash_ids[];
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 89ceae2221..503059cacd 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -73,7 +73,8 @@ static int write_sr(struct spi_flash *flash, u8 ws)
 	return 0;
 }
 
-#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
+#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) || \
+    defined(CONFIG_SPI_FLASH_SST)
 static int read_cr(struct spi_flash *flash, u8 *rc)
 {
 	struct spi_flash_command cmd;
@@ -891,6 +892,22 @@ int stm_unlock(struct spi_flash *flash, u32 ofs, size_t len)
 }
 #endif
 
+#ifdef CONFIG_SPI_FLASH_SST
+static int sst26_unlock(struct spi_flash *flash)
+{
+	struct spi_flash_command cmd;
+	int ret;
+
+	spi_flash_command_init(&cmd, CMD_SST_ULBPR, 0, SPI_FCMD_WRITE_REG);
+	ret = spi_flash_write_common(flash, &cmd);
+	if (ret) {
+		debug("SF: SST26 is still locked (read-only)\n");
+		return ret;
+	}
+
+	return 0;
+}
+#endif
 
 #ifdef CONFIG_SPI_FLASH_MACRONIX
 static int macronix_quad_enable(struct spi_flash *flash)
@@ -920,7 +937,8 @@ static int macronix_quad_enable(struct spi_flash *flash)
 }
 #endif
 
-#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
+#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) ||\
+    defined(CONFIG_SPI_FLASH_SST)
 static int spansion_quad_enable(struct spi_flash *flash)
 {
 	u8 qeb_status;
@@ -981,9 +999,11 @@ static int set_quad_mode(struct spi_flash *flash,
 	case SPI_FLASH_CFI_MFR_MACRONIX:
 		return macronix_quad_enable(flash);
 #endif
-#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND)
+#if defined(CONFIG_SPI_FLASH_SPANSION) || defined(CONFIG_SPI_FLASH_WINBOND) ||\
+    defined(CONFIG_SPI_FLASH_SST)
 	case SPI_FLASH_CFI_MFR_SPANSION:
 	case SPI_FLASH_CFI_MFR_WINBOND:
+	case SPI_FLASH_CFI_MFR_SST:
 		return spansion_quad_enable(flash);
 #endif
 #ifdef CONFIG_SPI_FLASH_STMICRO
@@ -1039,6 +1059,11 @@ int spi_flash_scan(struct spi_flash *flash)
 	    JEDEC_MFR(info) == SPI_FLASH_CFI_MFR_SST)
 		write_sr(flash, 0);
 
+#ifdef CONFIG_SPI_FLASH_SST
+	if (info->flags & SST_ULBPR)
+		sst26_unlock(flash);
+#endif
+
 	flash->name = info->name;
 	flash->memory_map = spi->memory_map;
 
@@ -1098,7 +1123,10 @@ int spi_flash_scan(struct spi_flash *flash)
 		flash->erase_size = 4096 << flash->shift;
 	} else
 #endif
-	{
+	if (info->flags & SECT_4K_ONLY) {
+		flash->erase_cmd = CMD_ERASE_4K;
+		flash->erase_size = 4096 << flash->shift;
+	} else {
 		flash->erase_cmd = CMD_ERASE_64K;
 		flash->erase_size = flash->sector_size;
 	}
diff --git a/drivers/mtd/spi/spi_flash_ids.c b/drivers/mtd/spi/spi_flash_ids.c
index edca94e30c..3d3132bc3b 100644
--- a/drivers/mtd/spi/spi_flash_ids.c
+++ b/drivers/mtd/spi/spi_flash_ids.c
@@ -146,6 +146,11 @@ const struct spi_flash_info spi_flash_ids[] = {
 	{"sst25wf040",	   INFO(0xbf2504, 0x0,	64 * 1024,     8, SECT_4K | SST_WR) },
 	{"sst25wf040b",	   INFO(0x621613, 0x0,	64 * 1024,     8, SECT_4K) },
 	{"sst25wf080",	   INFO(0xbf2505, 0x0,	64 * 1024,    16, SECT_4K | SST_WR) },
+	{"sst26vf016b",	   INFO(0xbf2641, 0x0,   4 * 1024,   512, SECT_4K_ONLY | SST_ULBPR | RD_FULL) },
+	{"sst26vf032b",	   INFO(0xbf2642, 0x0,   4 * 1024,  1024, SECT_4K_ONLY | SST_ULBPR | RD_FULL) },
+	{"sst26vf064b",	   INFO(0xbf2643, 0x0,   4 * 1024,  2048, SECT_4K_ONLY | SST_ULBPR | RD_FULL) },
+	{"sst26wf040b",	   INFO(0xbf2654, 0x0,   4 * 1024,   128, SECT_4K_ONLY | SST_ULBPR | RD_FULL) },
+	{"sst26wf080b",	   INFO(0xbf2658, 0x0,   4 * 1024,   256, SECT_4K_ONLY | SST_ULBPR | RD_FULL) },
 #endif
 #ifdef CONFIG_SPI_FLASH_WINBOND		/* WINBOND */
 	{"w25p80",	   INFO(0xef2014, 0x0,	64 * 1024,    16, 0) },
-- 
2.13.0

[U-Boot] [PATCH v3 8/8] sf: add driver for Atmel QSPI controller

[Wenyou Yang]

From: Cyrille Pitchen <cyrille.pitchen@atmel.com>

This patch adds support to the Atmel Quad SPI controller.

Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
---

Changes in v3: None
Changes in v2:
 - Rebase on the latest u-boot/master(2710d54f5).

 drivers/spi/Kconfig      |   7 +
 drivers/spi/Makefile     |   1 +
 drivers/spi/atmel_qspi.c | 404 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/spi/atmel_qspi.h | 169 ++++++++++++++++++++
 4 files changed, 581 insertions(+)
 create mode 100644 drivers/spi/atmel_qspi.c
 create mode 100644 drivers/spi/atmel_qspi.h

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8a8e8e480f..b3e8cb941e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -32,6 +32,13 @@ config ATH79_SPI
 	  uses driver model and requires a device tree binding to operate.
 	  please refer to doc/device-tree-bindings/spi/spi-ath79.txt.
 
+config ATMEL_QSPI
+	bool "Atmel QSPI driver"
+	depends on ARCH_AT91
+	help
+	  Enable the Ateml Quad-SPI (QSPI) driver. This driver can only be
+	  used to access SPI NOR flashes.
+
 config ATMEL_SPI
 	bool "Atmel SPI driver"
 	depends on ARCH_AT91
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9f8b86de76..358db2b065 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -18,6 +18,7 @@ endif
 obj-$(CONFIG_ALTERA_SPI) += altera_spi.o
 obj-$(CONFIG_ATH79_SPI) += ath79_spi.o
 obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o
+obj-$(CONFIG_ATMEL_QSPI) += atmel_qspi.o
 obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o
 obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o cadence_qspi_apb.o
 obj-$(CONFIG_CF_SPI) += cf_spi.o
diff --git a/drivers/spi/atmel_qspi.c b/drivers/spi/atmel_qspi.c
new file mode 100644
index 0000000000..0af7a4dc0a
--- /dev/null
+++ b/drivers/spi/atmel_qspi.c
@@ -0,0 +1,404 @@
+/*
+ * Copyright (C) 2017 Atmel Corporation
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <errno.h>
+#include <spi.h>
+#include <asm/io.h>
+#include <mach/clk.h>
+#include "atmel_qspi.h"
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static void atmel_qspi_memcpy_fromio(void *dst, unsigned long src, size_t len)
+{
+	u8 *d = (u8 *)dst;
+
+	while (len--) {
+		*d++ = readb(src);
+		src++;
+	}
+}
+
+static void atmel_qspi_memcpy_toio(unsigned long dst, const void *src,
+				   size_t len)
+{
+	const u8 *s = (const u8 *)src;
+
+	while (len--) {
+		writeb(*s, dst);
+		dst++;
+		s++;
+	}
+}
+
+static int atmel_qspi_set_ifr_tfrtype(u8 flags, u32 *ifr)
+{
+	u32 ifr_tfrtype;
+
+	switch (flags & SPI_FCMD_TYPE) {
+	case SPI_FCMD_READ:
+		ifr_tfrtype = QSPI_IFR_TFRTYPE_READ_MEMORY;
+		break;
+
+	case SPI_FCMD_WRITE:
+		ifr_tfrtype = QSPI_IFR_TFRTYPE_WRITE_MEMORY;
+		break;
+
+	case SPI_FCMD_ERASE:
+	case SPI_FCMD_WRITE_REG:
+		ifr_tfrtype = QSPI_IFR_TFRTYPE_WRITE;
+		break;
+
+	case SPI_FCMD_READ_REG:
+		ifr_tfrtype = QSPI_IFR_TFRTYPE_READ;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	*ifr = (*ifr & ~QSPI_IFR_TFRTYPE) | ifr_tfrtype;
+	return 0;
+}
+
+static int atmel_qpsi_set_ifr_width(enum spi_flash_protocol proto, u32 *ifr)
+{
+	u32 ifr_width;
+
+	switch (proto) {
+	case SPI_FPROTO_1_1_1:
+		ifr_width = QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
+		break;
+
+	case SPI_FPROTO_1_1_2:
+		ifr_width = QSPI_IFR_WIDTH_DUAL_OUTPUT;
+		break;
+
+	case SPI_FPROTO_1_2_2:
+		ifr_width = QSPI_IFR_WIDTH_DUAL_IO;
+		break;
+
+	case SPI_FPROTO_2_2_2:
+		ifr_width = QSPI_IFR_WIDTH_DUAL_CMD;
+		break;
+
+	case SPI_FPROTO_1_1_4:
+		ifr_width = QSPI_IFR_WIDTH_QUAD_OUTPUT;
+		break;
+
+	case SPI_FPROTO_1_4_4:
+		ifr_width = QSPI_IFR_WIDTH_QUAD_IO;
+		break;
+
+	case SPI_FPROTO_4_4_4:
+		ifr_width = QSPI_IFR_WIDTH_QUAD_CMD;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	*ifr = (*ifr & ~QSPI_IFR_WIDTH) | ifr_width;
+	return 0;
+}
+
+static int atmel_qspi_xfer(struct udevice *dev, unsigned int bitlen,
+			   const void *dout, void *din, unsigned long flags)
+{
+	/* This controller can only be used with SPI NOR flashes. */
+	return -EINVAL;
+}
+
+static int atmel_qspi_set_speed(struct udevice *bus, uint hz)
+{
+	struct atmel_qspi_priv *aq = dev_get_priv(bus);
+	u32 scr, scbr, mask, new_value;
+
+	/* Compute the QSPI baudrate */
+	scbr = DIV_ROUND_UP(aq->bus_clk_rate, hz);
+	if (scbr > 0)
+		scbr--;
+
+	new_value = QSPI_SCR_SCBR_(scbr);
+	mask = QSPI_SCR_SCBR;
+
+	scr = qspi_readl(aq, QSPI_SCR);
+	if ((scr & mask) == new_value)
+		return 0;
+
+	scr = (scr & ~mask) | new_value;
+	qspi_writel(aq, QSPI_SCR, scr);
+
+	return 0;
+}
+
+static int atmel_qspi_set_mode(struct udevice *bus, uint mode)
+{
+	struct atmel_qspi_priv *aq = dev_get_priv(bus);
+	u32 scr, mask, new_value;
+
+	new_value = (QSPI_SCR_CPOL_((mode & SPI_CPOL) != 0) |
+		     QSPI_SCR_CPHA_((mode & SPI_CPHA) != 0));
+	mask = (QSPI_SCR_CPOL | QSPI_SCR_CPHA);
+
+	scr = qspi_readl(aq, QSPI_SCR);
+	if ((scr & mask) == new_value)
+		return 0;
+
+	scr = (scr & ~mask) | new_value;
+	qspi_writel(aq, QSPI_SCR, scr);
+
+	return 0;
+}
+
+static bool
+atmel_qspi_is_flash_command_supported(struct udevice *dev,
+				      const struct spi_flash_command *cmd)
+{
+	return true;
+}
+
+static int atmel_qspi_exec_flash_command(struct udevice *dev,
+					 const struct spi_flash_command *cmd)
+{
+	struct udevice *bus = dev_get_parent(dev);
+	struct atmel_qspi_priv *aq = dev_get_priv(bus);
+	unsigned int iar, icr, ifr;
+	unsigned int offset;
+	unsigned int imr, sr;
+	unsigned long memaddr;
+	int err;
+
+	iar = 0;
+	icr = 0;
+	ifr = 0;
+
+	err = atmel_qspi_set_ifr_tfrtype(cmd->flags, &ifr);
+	if (err)
+		return err;
+
+	err = atmel_qpsi_set_ifr_width(cmd->proto, &ifr);
+	if (err)
+		return err;
+
+	/* Compute instruction parameters */
+	icr |= QSPI_ICR_INST_(cmd->inst);
+	ifr |= QSPI_IFR_INSTEN;
+
+	/* Compute address parameters. */
+	switch (cmd->addr_len) {
+	case 4:
+		ifr |= QSPI_IFR_ADDRL_32_BIT;
+		/*break;*/ /* fall through the 24bit (3 byte) address case */
+	case 3:
+		iar = cmd->data_len ? 0 : cmd->addr;
+		ifr |= QSPI_IFR_ADDREN;
+		offset = cmd->addr;
+		break;
+	case 0:
+		offset = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Compute option parameters. */
+	if (cmd->num_mode_cycles) {
+		unsigned int mode_cycle_bits, mode_bits;
+
+		icr |= QSPI_ICR_OPT_(cmd->mode);
+		ifr |= QSPI_IFR_OPTEN;
+
+		switch (ifr & QSPI_IFR_WIDTH) {
+		case QSPI_IFR_WIDTH_SINGLE_BIT_SPI:
+		case QSPI_IFR_WIDTH_DUAL_OUTPUT:
+		case QSPI_IFR_WIDTH_QUAD_OUTPUT:
+			mode_cycle_bits = 1;
+			break;
+		case QSPI_IFR_WIDTH_DUAL_IO:
+		case QSPI_IFR_WIDTH_DUAL_CMD:
+			mode_cycle_bits = 2;
+			break;
+		case QSPI_IFR_WIDTH_QUAD_IO:
+		case QSPI_IFR_WIDTH_QUAD_CMD:
+			mode_cycle_bits = 4;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		mode_bits = cmd->num_mode_cycles * mode_cycle_bits;
+		switch (mode_bits) {
+		case 1:
+			ifr |= QSPI_IFR_OPTL_1BIT;
+			break;
+
+		case 2:
+			ifr |= QSPI_IFR_OPTL_2BIT;
+			break;
+
+		case 4:
+			ifr |= QSPI_IFR_OPTL_4BIT;
+			break;
+
+		case 8:
+			ifr |= QSPI_IFR_OPTL_8BIT;
+			break;
+
+		default:
+			return -EINVAL;
+		}
+	}
+
+	/* Set the number of dummy cycles. */
+	if (cmd->num_wait_states)
+		ifr |= QSPI_IFR_NBDUM_(cmd->num_wait_states);
+
+	/* Set data enable. */
+	if (cmd->data_len)
+		ifr |= QSPI_IFR_DATAEN;
+
+	/* Clear pending interrupts. */
+	(void)qspi_readl(aq, QSPI_SR);
+
+	/* Set QSPI Instruction Frame registers. */
+	qspi_writel(aq, QSPI_IAR, iar);
+	qspi_writel(aq, QSPI_ICR, icr);
+	qspi_writel(aq, QSPI_IFR, ifr);
+
+	/* Skip to the final steps if there is no data. */
+	if (!cmd->data_len)
+		goto no_data;
+
+	/* Dummy read of QSPI_IFR to synchronize APB and AHB accesses. */
+	(void)qspi_readl(aq, QSPI_IFR);
+
+	/* Stop here for Continuous Read. */
+	memaddr = (unsigned long)(aq->membase + offset);
+	if (cmd->tx_data)
+		/* Write data. */
+		atmel_qspi_memcpy_toio(memaddr, cmd->tx_data, cmd->data_len);
+	else if (cmd->rx_data)
+		/* Read data. */
+		atmel_qspi_memcpy_fromio(cmd->rx_data, memaddr, cmd->data_len);
+
+	/* Release the chip-select. */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
+
+no_data:
+	/* Poll INSTruction End and Chip Select Rise flags. */
+	imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
+	sr = 0;
+	while (sr != (QSPI_SR_INSTRE | QSPI_SR_CSR))
+		sr |= qspi_readl(aq, QSPI_SR) & imr;
+
+	return 0;
+}
+
+
+static const struct dm_spi_ops atmel_qspi_ops = {
+	.xfer				= atmel_qspi_xfer,
+	.set_speed			= atmel_qspi_set_speed,
+	.set_mode			= atmel_qspi_set_mode,
+	.is_flash_command_supported	= atmel_qspi_is_flash_command_supported,
+	.exec_flash_command		= atmel_qspi_exec_flash_command,
+};
+
+static int atmel_qspi_enable_clk(struct udevice *bus)
+{
+	struct atmel_qspi_priv *aq = dev_get_priv(bus);
+	struct clk clk;
+	ulong clk_rate;
+	int ret;
+
+	ret = clk_get_by_index(bus, 0, &clk);
+	if (ret)
+		return -EINVAL;
+
+	ret = clk_enable(&clk);
+	if (ret)
+		goto free_clock;
+
+	clk_rate = clk_get_rate(&clk);
+	if (!clk_rate) {
+		ret = -EINVAL;
+		goto free_clock;
+	}
+
+	aq->bus_clk_rate = clk_rate;
+
+free_clock:
+	clk_free(&clk);
+
+	return ret;
+}
+
+static int atmel_qspi_probe(struct udevice *bus)
+{
+	const struct atmel_qspi_platdata *plat = dev_get_platdata(bus);
+	struct atmel_qspi_priv *aq = dev_get_priv(bus);
+	u32 mr;
+	int ret;
+
+	ret = atmel_qspi_enable_clk(bus);
+	if (ret)
+		return ret;
+
+	aq->regbase = plat->regbase;
+	aq->membase = plat->membase;
+
+	/* Reset the QSPI controler */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
+
+	/* Set the QSPI controller in Serial Memory Mode */
+	mr = (QSPI_MR_NBBITS_8_BIT |
+	      QSPI_MR_SMM_MEMORY |
+	      QSPI_MR_CSMODE_LASTXFER);
+	qspi_writel(aq, QSPI_MR, mr);
+
+	/* Enable the QSPI controller */
+	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
+
+	return 0;
+}
+
+static int atmel_qspi_ofdata_to_platdata(struct udevice *bus)
+{
+	struct atmel_qspi_platdata *plat = dev_get_platdata(bus);
+	const void *blob = gd->fdt_blob;
+	int node = dev_of_offset(bus);
+	u32 data[4];
+	int ret;
+
+	ret = fdtdec_get_int_array(blob, node, "reg", data, ARRAY_SIZE(data));
+	if (ret) {
+		printf("Error: Can't get base addresses (ret=%d)!\n", ret);
+		return -ENODEV;
+	}
+	plat->regbase = (void *)data[0];
+	plat->membase = (void *)data[2];
+
+	return 0;
+}
+
+static const struct udevice_id atmel_qspi_ids[] = {
+	{ .compatible = "atmel,sama5d2-qspi" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_qspi) = {
+	.name		= "atmel_qspi",
+	.id		= UCLASS_SPI,
+	.of_match	= atmel_qspi_ids,
+	.ops		= &atmel_qspi_ops,
+	.ofdata_to_platdata = atmel_qspi_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct atmel_qspi_platdata),
+	.priv_auto_alloc_size = sizeof(struct atmel_qspi_priv),
+	.probe		= atmel_qspi_probe,
+};
diff --git a/drivers/spi/atmel_qspi.h b/drivers/spi/atmel_qspi.h
new file mode 100644
index 0000000000..ee1a14bd72
--- /dev/null
+++ b/drivers/spi/atmel_qspi.h
@@ -0,0 +1,169 @@
+/*
+ * Copyright (C) 2016
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ATMEL_QSPI_H__
+#define __ATMEL_QSPI_H__
+
+/*
+ * Register Definitions
+ */
+#define	QSPI_CR		0x00	/* Control Register */
+#define	QSPI_MR		0x04	/* Mode Register */
+#define	QSPI_RDR	0x08	/* Receive Data Register */
+#define	QSPI_TDR	0x0c	/* Transmit Data Register */
+#define	QSPI_SR		0x10	/* Status Register */
+#define	QSPI_IER	0x14	/* Interrupt Enable Register */
+#define	QSPI_IDR	0x18	/* Interrupt Disable Register */
+#define	QSPI_IMR	0x1c	/* Interrupt Mask Register */
+#define	QSPI_SCR	0x20	/* Serial Clock Register */
+#define	QSPI_IAR	0x30	/* Instruction Address Register */
+#define	QSPI_ICR	0x34	/* Instruction Code Register */
+#define	QSPI_IFR	0x38	/* Instruction Frame Register */
+/* 0x3c Reserved */
+#define	QSPI_SMR	0x40	/* Scrambling Mode Register */
+#define	QSPI_SKR	0x44	/* Scrambling Key Register */
+/* 0x48 ~ 0xe0 */
+#define	QSPI_WPMR	0xe4	/* Write Protection Mode Register */
+#define	QSPI_WPSR	0xe8	/* Write Protection Status Register */
+/* 0xec ~ 0xf8 Reserved */
+/* 0xfc Reserved */
+
+/*
+ * Register Field Definitions
+ */
+/* QSPI_CR */
+#define	QSPI_CR_QSPIEN		BIT(0)	/* QSPI Enable */
+#define	QSPI_CR_QSPIDIS		BIT(1)	/* QSPI Disable */
+#define	QSPI_CR_SWRST		BIT(7)	/* QSPI Software Reset */
+#define	QSPI_CR_LASTXFER	BIT(24)	/* Last Transfer */
+
+/* QSPI_MR */
+#define	QSPI_MR_SMM		BIT(0)	/* Serial Memort Mode */
+#define		QSPI_MR_SMM_SPI		0
+#define		QSPI_MR_SMM_MEMORY	QSPI_MR_SMM
+#define	QSPI_MR_LLB		BIT(1)	/* Local Localback Enable */
+#define		QSPI_MR_LLB_DISABLED	0
+#define		QSPI_MR_LLB_ENABLED	QSPI_MR_LLB
+#define	QSPI_MR_WDRBT		BIT(2)	/* Wait Data Read Before Transfer */
+#define		QSPI_MR_WDRBT_DISABLED	0
+#define		QSPI_MR_WDRBT_ENABLED	QSPI_MR_WDRBT
+#define	QSPI_MR_SMRM		BIT(3)	/* Serial Memory Register Mode */
+#define		QSPI_MR_SMRM_AHB	0
+#define		QSPI_MR_SMRM_APB	QSPI_MR_SMRM
+#define	QSPI_MR_CSMODE		GENMASK(5, 4)	/* Chip Select Mode */
+#define		QSPI_MR_CSMODE_NOT_RELOADED	(0x0u << 4)
+#define		QSPI_MR_CSMODE_LASTXFER		(0x1u << 4)
+#define		QSPI_MR_CSMODE_SYSTEMATICALLY	(0x2u << 4)
+#define	QSPI_MR_NBBITS		GENMASK(11, 8)	/*
+						 * Number of Bits Per
+						 * Transfer
+						 */
+#define		QSPI_MR_NBBITS_8_BIT		(0x0u << 8)
+#define		QSPI_MR_NBBITS_16_BIT		(0x8u << 8)
+#define	QSPI_MR_DLYBCT		GENMASK(23, 16)	/*
+						 * Delay Between Consecutive
+						 * Transfers
+						 */
+#define	QSPI_MR_DLYCS		GENMASK(31, 24)	/* Minimum Inactive QCS Delay */
+
+/* QSPI_SR */
+#define	QSPI_SR_RDRF		BIT(0)	/* Receive Data Register Full */
+#define	QSPI_SR_TDRE		BIT(1)	/* Transmit Data Register Empty */
+#define	QSPI_SR_TXEMPTY		BIT(2)	/* Transmission Registers Empty */
+#define	QSPI_SR_OVRES		BIT(3)	/* Overrun Error Status */
+#define	QSPI_SR_CSR		BIT(8)	/* Chip Select Rise */
+#define	QSPI_SR_CSS		BIT(9)	/* Chip Select Status */
+#define	QSPI_SR_INSTRE		BIT(10)	/* Instruction End Status */
+#define	QSPI_SR_QSPIENS		BIT(24)	/* QSPI Enable Status */
+
+/* QSPI_SCR */
+#define	QSPI_SCR_CPOL		BIT(0)	/* Clock Polarity */
+#define	QSPI_SCR_CPOL_(x)	((x) << 0)
+#define	QSPI_SCR_CPHA		BIT(1)	/* Clock Phase */
+#define	QSPI_SCR_CPHA_(x)	((x) << 1)
+#define	QSPI_SCR_SCBR		GENMASK(15, 8)	/* Serial Clock Baud Rate */
+#define	QSPI_SCR_SCBR_(x)	(((x) << 8) & QSPI_SCR_SCBR)
+#define QSPI_SCR_DLYBS		GENMASK(23, 16)
+#define	QSPI_SCR_DLYBS_(x)	(((x) << 16) & QSPI_SCR_DLYBS)	/*
+								 * Delay Before
+								 * QSCK
+								 */
+
+/* QSPI_ICR */
+#define QSPI_ICR_INST		GENMASK(7, 0)
+#define	QSPI_ICR_INST_(x)	(((x) << 0) & QSPI_ICR_INST)	/*
+								 * Instruction
+								 * Code
+								 */
+#define QSPI_ICR_OPT		GENMASK(23, 16)
+#define	QSPI_ICR_OPT_(x)	(((x) << 16) & QSPI_ICR_OPT)	/*
+								 * Option
+								 * Code
+								 */
+
+/* QSPI_IFR */
+#define	QSPI_IFR_WIDTH		GENMASK(2, 0)	/*
+						 * Width of Instruction Code,
+						 * Address, Option Code and Data
+						 */
+#define		QSPI_IFR_WIDTH_SINGLE_BIT_SPI	(0x0u << 0)
+#define		QSPI_IFR_WIDTH_DUAL_OUTPUT	(0x1u << 0)
+#define		QSPI_IFR_WIDTH_QUAD_OUTPUT	(0x2u << 0)
+#define		QSPI_IFR_WIDTH_DUAL_IO		(0x3u << 0)
+#define		QSPI_IFR_WIDTH_QUAD_IO		(0x4u << 0)
+#define		QSPI_IFR_WIDTH_DUAL_CMD		(0x5u << 0)
+#define		QSPI_IFR_WIDTH_QUAD_CMD		(0x6u << 0)
+#define QSPI_IFR_WIDTH_(x)	(((x) << 0) & QSPI_IFR_WIDTH)
+#define	QSPI_IFR_INSTEN		BIT(4)	/* Instruction Enable*/
+#define	QSPI_IFR_ADDREN		BIT(5)	/* Address Enable*/
+#define	QSPI_IFR_OPTEN		BIT(6)	/* Option Enable*/
+#define	QSPI_IFR_DATAEN		BIT(7)	/* Data Enable*/
+#define	QSPI_IFR_OPTL		GENMASK(9, 8)	/* Option Code Length */
+#define		QSPI_IFR_OPTL_1BIT		(0x0u << 8)
+#define		QSPI_IFR_OPTL_2BIT		(0x1u << 8)
+#define		QSPI_IFR_OPTL_4BIT		(0x2u << 8)
+#define		QSPI_IFR_OPTL_8BIT		(0x3u << 8)
+#define	QSPI_IFR_ADDRL		BIT(10)	/* Address Length */
+#define		QSPI_IFR_ADDRL_24_BIT		0
+#define		QSPI_IFR_ADDRL_32_BIT		QSPI_IFR_ADDRL
+#define	QSPI_IFR_TFRTYPE	GENMASK(13, 12)	/* Data Transfer Type */
+#define		QSPI_IFR_TFRTYPE_READ		(0x0u << 12)
+#define		QSPI_IFR_TFRTYPE_READ_MEMORY	(0x1u << 12)
+#define		QSPI_IFR_TFRTYPE_WRITE		(0x2u << 12)
+#define		QSPI_IFR_TFRTYPE_WRITE_MEMORY	(0x3u << 12)
+#define QSPI_IFR_TFRTYPE_(x)	(((x) << 12) & QSPI_IFR_TFRTYPE)
+#define	QSPI_IFR_CRM		BIT(14)	/* Continuous Read Mode */
+#define QSPI_IFR_NBDUM		GENMASK(20, 16)
+#define	QSPI_IFR_NBDUM_(x)	(((x) << 16) & QSPI_IFR_NBDUM)	/*
+								 * Number Of
+								 * Dummy Cycles
+								 */
+
+
+struct atmel_qspi_platdata {
+	void		*regbase;
+	void		*membase;
+};
+
+struct atmel_qspi_priv {
+	ulong		bus_clk_rate;
+	void		*regbase;
+	void		*membase;
+};
+
+#include <asm/io.h>
+
+static inline u32 qspi_readl(struct atmel_qspi_priv *aq, u32 reg)
+{
+	return readl(aq->regbase + reg);
+}
+
+static inline void qspi_writel(struct atmel_qspi_priv *aq, u32 reg, u32 value)
+{
+	writel(value, aq->regbase + reg);
+}
+
+#endif /* __ATMEL_QSPI_H__ */
-- 
2.13.0

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

Hi,

Do you have comments?


Best Regards,
Wenyou Yang

On 2017/7/25 15:00, Wenyou Yang wrote:
> This series of patches are based and have been tested on the 'master'
> branch of the u-boot.git tree.
>
> Tests were passed with a sama5d2 xplained board which embeds both SPI and
> QSPI controllers.
>
> The following tests have been passed:
>
> - QSPI0 + Macronix MX25L25673G:
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-4 at offset 0x10000: OK
>      The Macronix datasheet tells that only Page Program 1-4-4 is
>      supported, not Page Program 1-1-4, however it worked, I don't know
>      why...
>
> - QSPI0 + Microchip SST26
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>      SST26 memories support Page Program 1-4-4 but with the op code of
>      Page Program 1-1-4, which is not standard so I don't use it.
>
> - QSPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>
> - SPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>    + Page Program 1-1-1 at offest 0x6000: OK
>
> - SPI1 + Atmel AT45
>    + probe: OK
>    + Read at offset 0 and other than 0: OK
>    + Write at offset 0 and other than 0: OK
>
> During my tests, I used:
>    - setenv/saveenv, reboot, printenv
>    or
>    - sf probe, sf read, sf write, sf erase and sf update.
>
> Changes in v3:
>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>
> Changes in v2:
>   - Rebase on the latest u-boot/master(2710d54f5).
>
> Cyrille Pitchen (8):
>    spi: add support of SPI flash commands
>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>    sf: select the relevant SPI flash protocol for read and write commands
>    sf: differentiate Page Program 1-1-4 and 1-4-4
>    sf: add 'addr_len' member to 'struct spi_flash'
>    sf: add new option to support SPI flash above 16MiB
>    sf: add support to Microchip SST26 QSPI memories
>    sf: add driver for Atmel QSPI controller
>
>   drivers/mtd/spi/Kconfig         |  16 +-
>   drivers/mtd/spi/sf.c            |  78 ++++++--
>   drivers/mtd/spi/sf_dataflash.c  | 119 ++++++------
>   drivers/mtd/spi/sf_internal.h   |  48 +++--
>   drivers/mtd/spi/spi_flash.c     | 341 +++++++++++++++++++++++----------
>   drivers/mtd/spi/spi_flash_ids.c |   5 +
>   drivers/spi/Kconfig             |   7 +
>   drivers/spi/Makefile            |   1 +
>   drivers/spi/atmel_qspi.c        | 404 ++++++++++++++++++++++++++++++++++++++++
>   drivers/spi/atmel_qspi.h        | 169 +++++++++++++++++
>   drivers/spi/spi-uclass.c        |  40 ++++
>   drivers/spi/spi.c               |  13 ++
>   include/spi.h                   | 168 +++++++++++++++++
>   include/spi_flash.h             |   7 +
>   14 files changed, 1226 insertions(+), 190 deletions(-)
>   create mode 100644 drivers/spi/atmel_qspi.c
>   create mode 100644 drivers/spi/atmel_qspi.h
>

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

Hi All,

Do you have some comment on it?


On 2017/7/25 15:00, Wenyou Yang wrote:
> This series of patches are based and have been tested on the 'master'
> branch of the u-boot.git tree.
>
> Tests were passed with a sama5d2 xplained board which embeds both SPI and
> QSPI controllers.
>
> The following tests have been passed:
>
> - QSPI0 + Macronix MX25L25673G:
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-4 at offset 0x10000: OK
>      The Macronix datasheet tells that only Page Program 1-4-4 is
>      supported, not Page Program 1-1-4, however it worked, I don't know
>      why...
>
> - QSPI0 + Microchip SST26
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>      SST26 memories support Page Program 1-4-4 but with the op code of
>      Page Program 1-1-4, which is not standard so I don't use it.
>
> - QSPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>
> - SPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>    + Page Program 1-1-1 at offest 0x6000: OK
>
> - SPI1 + Atmel AT45
>    + probe: OK
>    + Read at offset 0 and other than 0: OK
>    + Write at offset 0 and other than 0: OK
>
> During my tests, I used:
>    - setenv/saveenv, reboot, printenv
>    or
>    - sf probe, sf read, sf write, sf erase and sf update.
>
> Changes in v3:
>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>
> Changes in v2:
>   - Rebase on the latest u-boot/master(2710d54f5).
>
> Cyrille Pitchen (8):
>    spi: add support of SPI flash commands
>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>    sf: select the relevant SPI flash protocol for read and write commands
>    sf: differentiate Page Program 1-1-4 and 1-4-4
>    sf: add 'addr_len' member to 'struct spi_flash'
>    sf: add new option to support SPI flash above 16MiB
>    sf: add support to Microchip SST26 QSPI memories
>    sf: add driver for Atmel QSPI controller
>
>   drivers/mtd/spi/Kconfig         |  16 +-
>   drivers/mtd/spi/sf.c            |  78 ++++++--
>   drivers/mtd/spi/sf_dataflash.c  | 119 ++++++------
>   drivers/mtd/spi/sf_internal.h   |  48 +++--
>   drivers/mtd/spi/spi_flash.c     | 341 +++++++++++++++++++++++----------
>   drivers/mtd/spi/spi_flash_ids.c |   5 +
>   drivers/spi/Kconfig             |   7 +
>   drivers/spi/Makefile            |   1 +
>   drivers/spi/atmel_qspi.c        | 404 ++++++++++++++++++++++++++++++++++++++++
>   drivers/spi/atmel_qspi.h        | 169 +++++++++++++++++
>   drivers/spi/spi-uclass.c        |  40 ++++
>   drivers/spi/spi.c               |  13 ++
>   include/spi.h                   | 168 +++++++++++++++++
>   include/spi_flash.h             |   7 +
>   14 files changed, 1226 insertions(+), 190 deletions(-)
>   create mode 100644 drivers/spi/atmel_qspi.c
>   create mode 100644 drivers/spi/atmel_qspi.h

Best Regards,
Wenyou Yang

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

On Fri, Aug 11, 2017 at 6:32 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
> Hi All,
>
> Do you have some comment on it?

Yes, Will come back.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

Hi,

This patch set has been here for a long time, could you have a look and 
take it?


Best Regards,

Wenyou Yang


On 2017/7/25 15:00, Wenyou Yang wrote:
> This series of patches are based and have been tested on the 'master'
> branch of the u-boot.git tree.
>
> Tests were passed with a sama5d2 xplained board which embeds both SPI and
> QSPI controllers.
>
> The following tests have been passed:
>
> - QSPI0 + Macronix MX25L25673G:
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-4 at offset 0x10000: OK
>      The Macronix datasheet tells that only Page Program 1-4-4 is
>      supported, not Page Program 1-1-4, however it worked, I don't know
>      why...
>
> - QSPI0 + Microchip SST26
>    + probe: OK
>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>      SST26 memories support Page Program 1-4-4 but with the op code of
>      Page Program 1-1-4, which is not standard so I don't use it.
>
> - QSPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>    + Page Program 1-1-1 at offset 0x10000: OK
>
> - SPI0 + Adesto AT25DF321A
>    + probe: OK
>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>    + Page Program 1-1-1 at offest 0x6000: OK
>
> - SPI1 + Atmel AT45
>    + probe: OK
>    + Read at offset 0 and other than 0: OK
>    + Write at offset 0 and other than 0: OK
>
> During my tests, I used:
>    - setenv/saveenv, reboot, printenv
>    or
>    - sf probe, sf read, sf write, sf erase and sf update.
>
> Changes in v3:
>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>
> Changes in v2:
>   - Rebase on the latest u-boot/master(2710d54f5).
>
> Cyrille Pitchen (8):
>    spi: add support of SPI flash commands
>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>    sf: select the relevant SPI flash protocol for read and write commands
>    sf: differentiate Page Program 1-1-4 and 1-4-4
>    sf: add 'addr_len' member to 'struct spi_flash'
>    sf: add new option to support SPI flash above 16MiB
>    sf: add support to Microchip SST26 QSPI memories
>    sf: add driver for Atmel QSPI controller
>
>   drivers/mtd/spi/Kconfig         |  16 +-
>   drivers/mtd/spi/sf.c            |  78 ++++++--
>   drivers/mtd/spi/sf_dataflash.c  | 119 ++++++------
>   drivers/mtd/spi/sf_internal.h   |  48 +++--
>   drivers/mtd/spi/spi_flash.c     | 341 +++++++++++++++++++++++----------
>   drivers/mtd/spi/spi_flash_ids.c |   5 +
>   drivers/spi/Kconfig             |   7 +
>   drivers/spi/Makefile            |   1 +
>   drivers/spi/atmel_qspi.c        | 404 ++++++++++++++++++++++++++++++++++++++++
>   drivers/spi/atmel_qspi.h        | 169 +++++++++++++++++
>   drivers/spi/spi-uclass.c        |  40 ++++
>   drivers/spi/spi.c               |  13 ++
>   include/spi.h                   | 168 +++++++++++++++++
>   include/spi_flash.h             |   7 +
>   14 files changed, 1226 insertions(+), 190 deletions(-)
>   create mode 100644 drivers/spi/atmel_qspi.c
>   create mode 100644 drivers/spi/atmel_qspi.h
>

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
> Hi,
>
> This patch set has been here for a long time, could you have a look and take
> it?

Yeah, I'm holding this because of my current spi-nor work. But anyway
I will try to merge on coming MW If all OK.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Marek Vasut]

On 08/25/2017 06:07 PM, Jagan Teki wrote:
> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>> Hi,
>>
>> This patch set has been here for a long time, could you have a look and take
>> it?
> 
> Yeah, I'm holding this because of my current spi-nor work. But anyway
> I will try to merge on coming MW If all OK.

Is your work posted somewhere or available in some git repository ?

I don't see any reason why you should not perform your maintainer duties
by reviewing/replying to an incoming patch, no matter what work you do
to the subsystem though ...

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
> On 08/25/2017 06:07 PM, Jagan Teki wrote:
>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>> Hi,
>>>
>>> This patch set has been here for a long time, could you have a look and take
>>> it?
>>
>> Yeah, I'm holding this because of my current spi-nor work. But anyway
>> I will try to merge on coming MW If all OK.
>
> Is your work posted somewhere or available in some git repository ?
>
> I don't see any reason why you should not perform your maintainer duties
> by reviewing/replying to an incoming patch, no matter what work you do
> to the subsystem though ...

I didn't write "this series holding spi-nor work" since it has some
new features I'm taking time to review. There is nothing wrong with
maintainer duties here, you must need to understand.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Marek Vasut]

On 08/25/2017 06:28 PM, Jagan Teki wrote:
> On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
>> On 08/25/2017 06:07 PM, Jagan Teki wrote:
>>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>> Hi,
>>>>
>>>> This patch set has been here for a long time, could you have a look and take
>>>> it?
>>>
>>> Yeah, I'm holding this because of my current spi-nor work. But anyway
>>> I will try to merge on coming MW If all OK.
>>
>> Is your work posted somewhere or available in some git repository ?

You did not answer this question.

>> I don't see any reason why you should not perform your maintainer duties
>> by reviewing/replying to an incoming patch, no matter what work you do
>> to the subsystem though ...
> 
> I didn't write "this series holding spi-nor work" since it has some
> new features I'm taking time to review.

I never implied this. Rather the opposite, you claim you do some work on
the SPI NOR core, yet you let this patchset rot in the list for over a
month now and gave the author zero feedback.

Notifying the author about the core changes early could've prevented a
lot of wasted effort on his side. Reviewing early could've prevented a
lot of frustration from patches being ignored.

> There is nothing wrong with
> maintainer duties here, you must need to understand.

I disagree.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Bin Meng]

On Sat, Aug 26, 2017 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
> On 08/25/2017 06:28 PM, Jagan Teki wrote:
>> On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
>>> On 08/25/2017 06:07 PM, Jagan Teki wrote:
>>>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>>> Hi,
>>>>>
>>>>> This patch set has been here for a long time, could you have a look and take
>>>>> it?
>>>>
>>>> Yeah, I'm holding this because of my current spi-nor work. But anyway
>>>> I will try to merge on coming MW If all OK.
>>>
>>> Is your work posted somewhere or available in some git repository ?
>
> You did not answer this question.
>
>>> I don't see any reason why you should not perform your maintainer duties
>>> by reviewing/replying to an incoming patch, no matter what work you do
>>> to the subsystem though ...
>>
>> I didn't write "this series holding spi-nor work" since it has some
>> new features I'm taking time to review.
>
> I never implied this. Rather the opposite, you claim you do some work on
> the SPI NOR core, yet you let this patchset rot in the list for over a
> month now and gave the author zero feedback.
>
> Notifying the author about the core changes early could've prevented a
> lot of wasted effort on his side. Reviewing early could've prevented a
> lot of frustration from patches being ignored.
>
>> There is nothing wrong with
>> maintainer duties here, you must need to understand.
>
> I disagree.
>

I agree with Marek here. I found it's really hard to get feedback for
SF patches in time and what I have to do is to ping again and again.

Regards,
Bin

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

On Sat, Aug 26, 2017 at 4:35 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Sat, Aug 26, 2017 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>> On 08/25/2017 06:28 PM, Jagan Teki wrote:
>>> On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
>>>> On 08/25/2017 06:07 PM, Jagan Teki wrote:
>>>>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This patch set has been here for a long time, could you have a look and take
>>>>>> it?
>>>>>
>>>>> Yeah, I'm holding this because of my current spi-nor work. But anyway
>>>>> I will try to merge on coming MW If all OK.
>>>>
>>>> Is your work posted somewhere or available in some git repository ?
>>
>> You did not answer this question.
>>
>>>> I don't see any reason why you should not perform your maintainer duties
>>>> by reviewing/replying to an incoming patch, no matter what work you do
>>>> to the subsystem though ...
>>>
>>> I didn't write "this series holding spi-nor work" since it has some
>>> new features I'm taking time to review.
>>
>> I never implied this. Rather the opposite, you claim you do some work on
>> the SPI NOR core, yet you let this patchset rot in the list for over a
>> month now and gave the author zero feedback.
>>
>> Notifying the author about the core changes early could've prevented a
>> lot of wasted effort on his side. Reviewing early could've prevented a
>> lot of frustration from patches being ignored.
>>
>>> There is nothing wrong with
>>> maintainer duties here, you must need to understand.
>>
>> I disagree.
>>
>
> I agree with Marek here. I found it's really hard to get feedback for
> SF patches in time and what I have to do is to ping again and again.

I agree that the SF side patches have got some delay recently, but ie
something not intentional. SF stack is not stable as of now, so I'm
working on SPI-NOR (v10)[1] the new patches for SF need to wait -
please be patient.

[1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/mtd-spinor

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

Hi,

Thanks for the changes.

On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
> This series of patches are based and have been tested on the 'master'
> branch of the u-boot.git tree.
>
> Tests were passed with a sama5d2 xplained board which embeds both SPI and
> QSPI controllers.
>
> The following tests have been passed:
>
> - QSPI0 + Macronix MX25L25673G:
>   + probe: OK
>   + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>   + Page Program 1-1-4 at offset 0x10000: OK
>     The Macronix datasheet tells that only Page Program 1-4-4 is
>     supported, not Page Program 1-1-4, however it worked, I don't know
>     why...
>
> - QSPI0 + Microchip SST26
>   + probe: OK
>   + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>   + Page Program 1-1-1 at offset 0x10000: OK
>     SST26 memories support Page Program 1-4-4 but with the op code of
>     Page Program 1-1-4, which is not standard so I don't use it.
>
> - QSPI0 + Adesto AT25DF321A
>   + probe: OK
>   + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>   + Page Program 1-1-1 at offset 0x10000: OK
>
> - SPI0 + Adesto AT25DF321A
>   + probe: OK
>   + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>   + Page Program 1-1-1 at offest 0x6000: OK
>
> - SPI1 + Atmel AT45
>   + probe: OK
>   + Read at offset 0 and other than 0: OK
>   + Write at offset 0 and other than 0: OK
>
> During my tests, I used:
>   - setenv/saveenv, reboot, printenv
>   or
>   - sf probe, sf read, sf write, sf erase and sf update.
>
> Changes in v3:
>  - Add the include <spi.h> to fix build error for corvus_defconfig.
>
> Changes in v2:
>  - Rebase on the latest u-boot/master(2710d54f5).
>
> Cyrille Pitchen (8):
>   spi: add support of SPI flash commands
>   sf: describe all SPI flash commands with 'struct spi_flash_command'
>   sf: select the relevant SPI flash protocol for read and write commands
>   sf: differentiate Page Program 1-1-4 and 1-4-4
>   sf: add 'addr_len' member to 'struct spi_flash'
>   sf: add new option to support SPI flash above 16MiB
>   sf: add support to Microchip SST26 QSPI memories
>   sf: add driver for Atmel QSPI controller

Comments:
How about writing struct spi_flash_command in spi_flash area
(include/spi_flash.h)? and then write atmel_qspi with
UCLASS_SPI_FLASH?

Testing:
Basic testing works fine.

Issues:
- Build issue: with zynq_microzed_defconfig
drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
but not used [-Wunused-but-set-variable]
  bool above_16MB;
       ^~~~~~~~~~
  CC      spl/lib/membuff.o

- issue with spi_flash_cmd_read_ops 4BAIS
Need to calculate bank length only if BAR is in use. Otherwise,
consider the given len as read_len.

Will send separate patch for this.

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 89ceae2..b5d8ef3 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flash
*flash, u32 offset,
         if (ret < 0)
             return ret;
         bank_sel = flash->bank_curr;
-#endif
         remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
                 (bank_sel + 1)) - offset;
         if (len < remain_len)
             read_len = len;
         else
             read_len = remain_len;
+#else
+        read_len = len;
+#endif

         cmd.addr = read_addr;
         cmd.data_len = read_len;

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Marek Vasut]

On 08/26/2017 08:14 AM, Jagan Teki wrote:
> On Sat, Aug 26, 2017 at 4:35 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Sat, Aug 26, 2017 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
>>> On 08/25/2017 06:28 PM, Jagan Teki wrote:
>>>> On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
>>>>> On 08/25/2017 06:07 PM, Jagan Teki wrote:
>>>>>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This patch set has been here for a long time, could you have a look and take
>>>>>>> it?
>>>>>>
>>>>>> Yeah, I'm holding this because of my current spi-nor work. But anyway
>>>>>> I will try to merge on coming MW If all OK.
>>>>>
>>>>> Is your work posted somewhere or available in some git repository ?
>>>
>>> You did not answer this question.
>>>
>>>>> I don't see any reason why you should not perform your maintainer duties
>>>>> by reviewing/replying to an incoming patch, no matter what work you do
>>>>> to the subsystem though ...
>>>>
>>>> I didn't write "this series holding spi-nor work" since it has some
>>>> new features I'm taking time to review.
>>>
>>> I never implied this. Rather the opposite, you claim you do some work on
>>> the SPI NOR core, yet you let this patchset rot in the list for over a
>>> month now and gave the author zero feedback.
>>>
>>> Notifying the author about the core changes early could've prevented a
>>> lot of wasted effort on his side. Reviewing early could've prevented a
>>> lot of frustration from patches being ignored.
>>>
>>>> There is nothing wrong with
>>>> maintainer duties here, you must need to understand.
>>>
>>> I disagree.
>>>
>>
>> I agree with Marek here. I found it's really hard to get feedback for
>> SF patches in time and what I have to do is to ping again and again.
> 
> I agree that the SF side patches have got some delay recently, but ie
> something not intentional. SF stack is not stable as of now, so I'm
> working on SPI-NOR (v10)[1] the new patches for SF need to wait -
> please be patient.

What are the problems with the stack causing this instability ?

How do you plan to stabilize the stack for the current release ?

Given how complex and incomplete this patch(set) [1] is and that it's
already in v10, it seems it will take a while to get it into shape in
which it can be included in mainline. Blocking all other contributions
because of this patchset seems wrong and hurtful to the contributors to me.

> [1] http://git.denx.de/?p=u-boot-spi.git;a=shortlog;h=refs/heads/mtd-spinor
> 
> thanks!
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Tom Rini]

On Sat, Aug 26, 2017 at 10:36:57AM +0200, Marek Vasut wrote:
> On 08/26/2017 08:14 AM, Jagan Teki wrote:
> > On Sat, Aug 26, 2017 at 4:35 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> >> On Sat, Aug 26, 2017 at 12:45 AM, Marek Vasut <marex@denx.de> wrote:
> >>> On 08/25/2017 06:28 PM, Jagan Teki wrote:
> >>>> On Fri, Aug 25, 2017 at 9:43 PM, Marek Vasut <marex@denx.de> wrote:
> >>>>> On 08/25/2017 06:07 PM, Jagan Teki wrote:
> >>>>>> On Fri, Aug 25, 2017 at 6:47 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> This patch set has been here for a long time, could you have a look and take
> >>>>>>> it?
> >>>>>>
> >>>>>> Yeah, I'm holding this because of my current spi-nor work. But anyway
> >>>>>> I will try to merge on coming MW If all OK.
> >>>>>
> >>>>> Is your work posted somewhere or available in some git repository ?
> >>>
> >>> You did not answer this question.
> >>>
> >>>>> I don't see any reason why you should not perform your maintainer duties
> >>>>> by reviewing/replying to an incoming patch, no matter what work you do
> >>>>> to the subsystem though ...
> >>>>
> >>>> I didn't write "this series holding spi-nor work" since it has some
> >>>> new features I'm taking time to review.
> >>>
> >>> I never implied this. Rather the opposite, you claim you do some work on
> >>> the SPI NOR core, yet you let this patchset rot in the list for over a
> >>> month now and gave the author zero feedback.
> >>>
> >>> Notifying the author about the core changes early could've prevented a
> >>> lot of wasted effort on his side. Reviewing early could've prevented a
> >>> lot of frustration from patches being ignored.
> >>>
> >>>> There is nothing wrong with
> >>>> maintainer duties here, you must need to understand.
> >>>
> >>> I disagree.
> >>>
> >>
> >> I agree with Marek here. I found it's really hard to get feedback for
> >> SF patches in time and what I have to do is to ping again and again.
> > 
> > I agree that the SF side patches have got some delay recently, but ie
> > something not intentional. SF stack is not stable as of now, so I'm
> > working on SPI-NOR (v10)[1] the new patches for SF need to wait -
> > please be patient.
> 
> What are the problems with the stack causing this instability ?
> 
> How do you plan to stabilize the stack for the current release ?
> 
> Given how complex and incomplete this patch(set) [1] is and that it's
> already in v10, it seems it will take a while to get it into shape in
> which it can be included in mainline. Blocking all other contributions
> because of this patchset seems wrong and hurtful to the contributors to me.

Yes.  We cannot let a new framework block progress enabling things on
our current framework.  We are not at the point with spi-nor that we can
say nothing else is allowed to move forward.  Thanks!

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

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

Hi Jagan,

On 2017/8/26 14:34, Jagan Teki wrote:
> Hi,
>
> Thanks for the changes.
>
> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
>> This series of patches are based and have been tested on the 'master'
>> branch of the u-boot.git tree.
>>
>> Tests were passed with a sama5d2 xplained board which embeds both SPI and
>> QSPI controllers.
>>
>> The following tests have been passed:
>>
>> - QSPI0 + Macronix MX25L25673G:
>>    + probe: OK
>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-4 at offset 0x10000: OK
>>      The Macronix datasheet tells that only Page Program 1-4-4 is
>>      supported, not Page Program 1-1-4, however it worked, I don't know
>>      why...
>>
>> - QSPI0 + Microchip SST26
>>    + probe: OK
>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-1 at offset 0x10000: OK
>>      SST26 memories support Page Program 1-4-4 but with the op code of
>>      Page Program 1-1-4, which is not standard so I don't use it.
>>
>> - QSPI0 + Adesto AT25DF321A
>>    + probe: OK
>>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-1 at offset 0x10000: OK
>>
>> - SPI0 + Adesto AT25DF321A
>>    + probe: OK
>>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>    + Page Program 1-1-1 at offest 0x6000: OK
>>
>> - SPI1 + Atmel AT45
>>    + probe: OK
>>    + Read at offset 0 and other than 0: OK
>>    + Write at offset 0 and other than 0: OK
>>
>> During my tests, I used:
>>    - setenv/saveenv, reboot, printenv
>>    or
>>    - sf probe, sf read, sf write, sf erase and sf update.
>>
>> Changes in v3:
>>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>>
>> Changes in v2:
>>   - Rebase on the latest u-boot/master(2710d54f5).
>>
>> Cyrille Pitchen (8):
>>    spi: add support of SPI flash commands
>>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>>    sf: select the relevant SPI flash protocol for read and write commands
>>    sf: differentiate Page Program 1-1-4 and 1-4-4
>>    sf: add 'addr_len' member to 'struct spi_flash'
>>    sf: add new option to support SPI flash above 16MiB
>>    sf: add support to Microchip SST26 QSPI memories
>>    sf: add driver for Atmel QSPI controller
> Comments:
> How about writing struct spi_flash_command in spi_flash area
> (include/spi_flash.h)? and then write atmel_qspi with
> UCLASS_SPI_FLASH?
The spi_flash_command struct describes the relevant features of the spi 
controller, instead of the spi_flash device.
The purpose of patch set is used to supersede the spi_xfer() function to 
access the spi_flash device.
So putting it in include/spi.h is suitable, we should not move it in the 
spi_flash area.

Moreover, why do we write atmel_qspi with  UCLASS_SPI_FLASH?  It is not 
easy to understand.
>
> Testing:
> Basic testing works fine.
>
> Issues:
> - Build issue: with zynq_microzed_defconfig
> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
> but not used [-Wunused-but-set-variable]
>    bool above_16MB;

Will send a new version to fix it.  Thanks a lot.

>         ^~~~~~~~~~
>    CC      spl/lib/membuff.o
>
> - issue with spi_flash_cmd_read_ops 4BAIS
> Need to calculate bank length only if BAR is in use. Otherwise,
> consider the given len as read_len.
>
> Will send separate patch for this.
>
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 89ceae2..b5d8ef3 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flashmake
> *flash, u32 offset,
>           if (ret < 0)
>               return ret;
>           bank_sel = flash->bank_curr;
> -#endif
>           remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
>                   (bank_sel + 1)) - offset;
>           if (len < remain_len)
>               read_len = len;
>           else
>               read_len = remain_len;
> +#else
> +        read_len = len;
> +#endif
>
>           cmd.addr = read_addr;
>           cmd.data_len = read_len;
> thanks!

Best Regards,
Wenyou Yang

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

On 2017/8/26 14:34, Jagan Teki wrote:
> Hi,
>
> Thanks for the changes.
>
> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
>> This series of patches are based and have been tested on the 'master'
>> branch of the u-boot.git tree.
>>
>> Tests were passed with a sama5d2 xplained board which embeds both SPI and
>> QSPI controllers.
>>
>> The following tests have been passed:
>>
>> - QSPI0 + Macronix MX25L25673G:
>>    + probe: OK
>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-4 at offset 0x10000: OK
>>      The Macronix datasheet tells that only Page Program 1-4-4 is
>>      supported, not Page Program 1-1-4, however it worked, I don't know
>>      why...
>>
>> - QSPI0 + Microchip SST26
>>    + probe: OK
>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-1 at offset 0x10000: OK
>>      SST26 memories support Page Program 1-4-4 but with the op code of
>>      Page Program 1-1-4, which is not standard so I don't use it.
>>
>> - QSPI0 + Adesto AT25DF321A
>>    + probe: OK
>>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>    + Page Program 1-1-1 at offset 0x10000: OK
>>
>> - SPI0 + Adesto AT25DF321A
>>    + probe: OK
>>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>    + Page Program 1-1-1 at offest 0x6000: OK
>>
>> - SPI1 + Atmel AT45
>>    + probe: OK
>>    + Read at offset 0 and other than 0: OK
>>    + Write at offset 0 and other than 0: OK
>>
>> During my tests, I used:
>>    - setenv/saveenv, reboot, printenv
>>    or
>>    - sf probe, sf read, sf write, sf erase and sf update.
>>
>> Changes in v3:
>>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>>
>> Changes in v2:
>>   - Rebase on the latest u-boot/master(2710d54f5).
>>
>> Cyrille Pitchen (8):
>>    spi: add support of SPI flash commands
>>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>>    sf: select the relevant SPI flash protocol for read and write commands
>>    sf: differentiate Page Program 1-1-4 and 1-4-4
>>    sf: add 'addr_len' member to 'struct spi_flash'
>>    sf: add new option to support SPI flash above 16MiB
>>    sf: add support to Microchip SST26 QSPI memories
>>    sf: add driver for Atmel QSPI controller
> Comments:
> How about writing struct spi_flash_command in spi_flash area
> (include/spi_flash.h)? and then write atmel_qspi with
> UCLASS_SPI_FLASH?
>
> Testing:
> Basic testing works fine.
>
> Issues:
> - Build issue: with zynq_microzed_defconfig
> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
> but not used [-Wunused-but-set-variable]
>    bool above_16MB;
>         ^~~~~~~~~~
>    CC      spl/lib/membuff.o
>
> - issue with spi_flash_cmd_read_ops 4BAIS
> Need to calculate bank length only if BAR is in use. Otherwise,
> consider the given len as read_len.
>
> Will send separate patch for this.
Will You send a separate patch? or I include it in this patch set?
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 89ceae2..b5d8ef3 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flash
> *flash, u32 offset,
>           if (ret < 0)
>               return ret;
>           bank_sel = flash->bank_curr;
> -#endif
>           remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
>                   (bank_sel + 1)) - offset;
>           if (len < remain_len)
>               read_len = len;
>           else
>               read_len = remain_len;
> +#else
> +        read_len = len;
> +#endif
>
>           cmd.addr = read_addr;
>           cmd.data_len = read_len;
>
> thanks!

Best Regards,
Wenyou Yang

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Bin Meng]

On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
<Wenyou.Yang@microchip.com> wrote:
>
>
> On 2017/8/26 14:34, Jagan Teki wrote:
>>
>> Hi,
>>
>> Thanks for the changes.
>>
>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com>
>> wrote:
>>>
>>> This series of patches are based and have been tested on the 'master'
>>> branch of the u-boot.git tree.
>>>
>>> Tests were passed with a sama5d2 xplained board which embeds both SPI and
>>> QSPI controllers.
>>>
>>> The following tests have been passed:
>>>
>>> - QSPI0 + Macronix MX25L25673G:
>>>    + probe: OK
>>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-4 at offset 0x10000: OK
>>>      The Macronix datasheet tells that only Page Program 1-4-4 is
>>>      supported, not Page Program 1-1-4, however it worked, I don't know
>>>      why...
>>>
>>> - QSPI0 + Microchip SST26
>>>    + probe: OK
>>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offset 0x10000: OK
>>>      SST26 memories support Page Program 1-4-4 but with the op code of
>>>      Page Program 1-1-4, which is not standard so I don't use it.
>>>
>>> - QSPI0 + Adesto AT25DF321A
>>>    + probe: OK
>>>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offset 0x10000: OK
>>>
>>> - SPI0 + Adesto AT25DF321A
>>>    + probe: OK
>>>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offest 0x6000: OK
>>>
>>> - SPI1 + Atmel AT45
>>>    + probe: OK
>>>    + Read at offset 0 and other than 0: OK
>>>    + Write at offset 0 and other than 0: OK
>>>
>>> During my tests, I used:
>>>    - setenv/saveenv, reboot, printenv
>>>    or
>>>    - sf probe, sf read, sf write, sf erase and sf update.
>>>
>>> Changes in v3:
>>>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>
>>> Changes in v2:
>>>   - Rebase on the latest u-boot/master(2710d54f5).
>>>
>>> Cyrille Pitchen (8):
>>>    spi: add support of SPI flash commands
>>>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>    sf: select the relevant SPI flash protocol for read and write commands
>>>    sf: differentiate Page Program 1-1-4 and 1-4-4
>>>    sf: add 'addr_len' member to 'struct spi_flash'
>>>    sf: add new option to support SPI flash above 16MiB
>>>    sf: add support to Microchip SST26 QSPI memories
>>>    sf: add driver for Atmel QSPI controller
>>
>> Comments:
>> How about writing struct spi_flash_command in spi_flash area
>> (include/spi_flash.h)? and then write atmel_qspi with
>> UCLASS_SPI_FLASH?
>>
>> Testing:
>> Basic testing works fine.
>>
>> Issues:
>> - Build issue: with zynq_microzed_defconfig
>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>> but not used [-Wunused-but-set-variable]
>>    bool above_16MB;
>>         ^~~~~~~~~~
>>    CC      spl/lib/membuff.o
>>
>> - issue with spi_flash_cmd_read_ops 4BAIS
>> Need to calculate bank length only if BAR is in use. Otherwise,
>> consider the given len as read_len.
>>
>> Will send separate patch for this.
>
> Will You send a separate patch? or I include it in this patch set?
>>

This should not be a separate patch. Since every patch needs to pass
buildman testing.

>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>> index 89ceae2..b5d8ef3 100644
>> --- a/drivers/mtd/spi/spi_flash.c
>> +++ b/drivers/mtd/spi/spi_flash.c
>> @@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flash
>> *flash, u32 offset,
>>           if (ret < 0)
>>               return ret;
>>           bank_sel = flash->bank_curr;
>> -#endif
>>           remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
>>                   (bank_sel + 1)) - offset;
>>           if (len < remain_len)
>>               read_len = len;
>>           else
>>               read_len = remain_len;
>> +#else
>> +        read_len = len;
>> +#endif
>>
>>           cmd.addr = read_addr;
>>           cmd.data_len = read_len;
>>

Regards,
Bin

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

On 2017/8/30 11:43, Bin Meng wrote:
> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
> <Wenyou.Yang@microchip.com> wrote:
>>
>> On 2017/8/26 14:34, Jagan Teki wrote:
>>> Hi,
>>>
>>> Thanks for the changes.
>>>
>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com>
>>> wrote:
>>>> This series of patches are based and have been tested on the 'master'
>>>> branch of the u-boot.git tree.
>>>>
>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI and
>>>> QSPI controllers.
>>>>
>>>> The following tests have been passed:
>>>>
>>>> - QSPI0 + Macronix MX25L25673G:
>>>>     + probe: OK
>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>     + Page Program 1-1-4 at offset 0x10000: OK
>>>>       The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>       supported, not Page Program 1-1-4, however it worked, I don't know
>>>>       why...
>>>>
>>>> - QSPI0 + Microchip SST26
>>>>     + probe: OK
>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>       SST26 memories support Page Program 1-4-4 but with the op code of
>>>>       Page Program 1-1-4, which is not standard so I don't use it.
>>>>
>>>> - QSPI0 + Adesto AT25DF321A
>>>>     + probe: OK
>>>>     + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>
>>>> - SPI0 + Adesto AT25DF321A
>>>>     + probe: OK
>>>>     + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>     + Page Program 1-1-1 at offest 0x6000: OK
>>>>
>>>> - SPI1 + Atmel AT45
>>>>     + probe: OK
>>>>     + Read at offset 0 and other than 0: OK
>>>>     + Write at offset 0 and other than 0: OK
>>>>
>>>> During my tests, I used:
>>>>     - setenv/saveenv, reboot, printenv
>>>>     or
>>>>     - sf probe, sf read, sf write, sf erase and sf update.
>>>>
>>>> Changes in v3:
>>>>    - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>
>>>> Changes in v2:
>>>>    - Rebase on the latest u-boot/master(2710d54f5).
>>>>
>>>> Cyrille Pitchen (8):
>>>>     spi: add support of SPI flash commands
>>>>     sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>     sf: select the relevant SPI flash protocol for read and write commands
>>>>     sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>     sf: add 'addr_len' member to 'struct spi_flash'
>>>>     sf: add new option to support SPI flash above 16MiB
>>>>     sf: add support to Microchip SST26 QSPI memories
>>>>     sf: add driver for Atmel QSPI controller
>>> Comments:
>>> How about writing struct spi_flash_command in spi_flash area
>>> (include/spi_flash.h)? and then write atmel_qspi with
>>> UCLASS_SPI_FLASH?
>>>
>>> Testing:
>>> Basic testing works fine.
>>>
>>> Issues:
>>> - Build issue: with zynq_microzed_defconfig
>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>> but not used [-Wunused-but-set-variable]
>>>     bool above_16MB;
>>>          ^~~~~~~~~~
>>>     CC      spl/lib/membuff.o
>>>
>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>> consider the given len as read_len.
>>>
>>> Will send separate patch for this.
>> Will You send a separate patch? or I include it in this patch set?
> This should not be a separate patch. Since every patch needs to pass
> buildman testing.
But it is not introduced by this patch set. So should be a separate 
patch to fix.
>
>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
>>> index 89ceae2..b5d8ef3 100644
>>> --- a/drivers/mtd/spi/spi_flash.c
>>> +++ b/drivers/mtd/spi/spi_flash.c
>>> @@ -558,13 +558,15 @@ int spi_flash_cmd_read_ops(struct spi_flash
>>> *flash, u32 offset,
>>>            if (ret < 0)
>>>                return ret;
>>>            bank_sel = flash->bank_curr;
>>> -#endif
>>>            remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
>>>                    (bank_sel + 1)) - offset;
>>>            if (len < remain_len)
>>>                read_len = len;
>>>            else
>>>                read_len = remain_len;
>>> +#else
>>> +        read_len = len;
>>> +#endif
>>>
>>>            cmd.addr = read_addr;
>>>            cmd.data_len = read_len;
>>>
> Regards,
> Bin

Best Regards,
Wenyou Yang

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Bin Meng]

On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>
>
> On 2017/8/30 11:43, Bin Meng wrote:
>>
>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
>> <Wenyou.Yang@microchip.com> wrote:
>>>
>>>
>>> On 2017/8/26 14:34, Jagan Teki wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks for the changes.
>>>>
>>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
>>>> <wenyou.yang@microchip.com>
>>>> wrote:
>>>>>
>>>>> This series of patches are based and have been tested on the 'master'
>>>>> branch of the u-boot.git tree.
>>>>>
>>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI
>>>>> and
>>>>> QSPI controllers.
>>>>>
>>>>> The following tests have been passed:
>>>>>
>>>>> - QSPI0 + Macronix MX25L25673G:
>>>>>     + probe: OK
>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>     + Page Program 1-1-4 at offset 0x10000: OK
>>>>>       The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>>       supported, not Page Program 1-1-4, however it worked, I don't
>>>>> know
>>>>>       why...
>>>>>
>>>>> - QSPI0 + Microchip SST26
>>>>>     + probe: OK
>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>       SST26 memories support Page Program 1-4-4 but with the op code of
>>>>>       Page Program 1-1-4, which is not standard so I don't use it.
>>>>>
>>>>> - QSPI0 + Adesto AT25DF321A
>>>>>     + probe: OK
>>>>>     + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>
>>>>> - SPI0 + Adesto AT25DF321A
>>>>>     + probe: OK
>>>>>     + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>>     + Page Program 1-1-1 at offest 0x6000: OK
>>>>>
>>>>> - SPI1 + Atmel AT45
>>>>>     + probe: OK
>>>>>     + Read at offset 0 and other than 0: OK
>>>>>     + Write at offset 0 and other than 0: OK
>>>>>
>>>>> During my tests, I used:
>>>>>     - setenv/saveenv, reboot, printenv
>>>>>     or
>>>>>     - sf probe, sf read, sf write, sf erase and sf update.
>>>>>
>>>>> Changes in v3:
>>>>>    - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>>
>>>>> Changes in v2:
>>>>>    - Rebase on the latest u-boot/master(2710d54f5).
>>>>>
>>>>> Cyrille Pitchen (8):
>>>>>     spi: add support of SPI flash commands
>>>>>     sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>>     sf: select the relevant SPI flash protocol for read and write
>>>>> commands
>>>>>     sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>>     sf: add 'addr_len' member to 'struct spi_flash'
>>>>>     sf: add new option to support SPI flash above 16MiB
>>>>>     sf: add support to Microchip SST26 QSPI memories
>>>>>     sf: add driver for Atmel QSPI controller
>>>>
>>>> Comments:
>>>> How about writing struct spi_flash_command in spi_flash area
>>>> (include/spi_flash.h)? and then write atmel_qspi with
>>>> UCLASS_SPI_FLASH?
>>>>
>>>> Testing:
>>>> Basic testing works fine.
>>>>
>>>> Issues:
>>>> - Build issue: with zynq_microzed_defconfig
>>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>>> but not used [-Wunused-but-set-variable]
>>>>     bool above_16MB;
>>>>          ^~~~~~~~~~
>>>>     CC      spl/lib/membuff.o
>>>>
>>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>>> consider the given len as read_len.
>>>>
>>>> Will send separate patch for this.
>>>
>>> Will You send a separate patch? or I include it in this patch set?
>>
>> This should not be a separate patch. Since every patch needs to pass
>> buildman testing.
>
> But it is not introduced by this patch set. So should be a separate patch to
> fix.

Do you mean the build warnings exist in current u-boot/master?

If so, Jagan can you please explain why you mention this? This is
nothing related to this patch review.

Regards,
Bin

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Yang, Wenyou]

On 2017/8/30 13:41, Bin Meng wrote:
> On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>
>> On 2017/8/30 11:43, Bin Meng wrote:
>>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
>>> <Wenyou.Yang@microchip.com> wrote:
>>>>
>>>> On 2017/8/26 14:34, Jagan Teki wrote:
>>>>> Hi,
>>>>>
>>>>> Thanks for the changes.
>>>>>
>>>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
>>>>> <wenyou.yang@microchip.com>
>>>>> wrote:
>>>>>> This series of patches are based and have been tested on the 'master'
>>>>>> branch of the u-boot.git tree.
>>>>>>
>>>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI
>>>>>> and
>>>>>> QSPI controllers.
>>>>>>
>>>>>> The following tests have been passed:
>>>>>>
>>>>>> - QSPI0 + Macronix MX25L25673G:
>>>>>>      + probe: OK
>>>>>>      + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>      + Page Program 1-1-4 at offset 0x10000: OK
>>>>>>        The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>>>        supported, not Page Program 1-1-4, however it worked, I don't
>>>>>> know
>>>>>>        why...
>>>>>>
>>>>>> - QSPI0 + Microchip SST26
>>>>>>      + probe: OK
>>>>>>      + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>      + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>        SST26 memories support Page Program 1-4-4 but with the op code of
>>>>>>        Page Program 1-1-4, which is not standard so I don't use it.
>>>>>>
>>>>>> - QSPI0 + Adesto AT25DF321A
>>>>>>      + probe: OK
>>>>>>      + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>>>      + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>
>>>>>> - SPI0 + Adesto AT25DF321A
>>>>>>      + probe: OK
>>>>>>      + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>>>      + Page Program 1-1-1 at offest 0x6000: OK
>>>>>>
>>>>>> - SPI1 + Atmel AT45
>>>>>>      + probe: OK
>>>>>>      + Read at offset 0 and other than 0: OK
>>>>>>      + Write at offset 0 and other than 0: OK
>>>>>>
>>>>>> During my tests, I used:
>>>>>>      - setenv/saveenv, reboot, printenv
>>>>>>      or
>>>>>>      - sf probe, sf read, sf write, sf erase and sf update.
>>>>>>
>>>>>> Changes in v3:
>>>>>>     - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>>>
>>>>>> Changes in v2:
>>>>>>     - Rebase on the latest u-boot/master(2710d54f5).
>>>>>>
>>>>>> Cyrille Pitchen (8):
>>>>>>      spi: add support of SPI flash commands
>>>>>>      sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>>>      sf: select the relevant SPI flash protocol for read and write
>>>>>> commands
>>>>>>      sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>>>      sf: add 'addr_len' member to 'struct spi_flash'
>>>>>>      sf: add new option to support SPI flash above 16MiB
>>>>>>      sf: add support to Microchip SST26 QSPI memories
>>>>>>      sf: add driver for Atmel QSPI controller
>>>>> Comments:
>>>>> How about writing struct spi_flash_command in spi_flash area
>>>>> (include/spi_flash.h)? and then write atmel_qspi with
>>>>> UCLASS_SPI_FLASH?
>>>>>
>>>>> Testing:
>>>>> Basic testing works fine.
>>>>>
>>>>> Issues:
>>>>> - Build issue: with zynq_microzed_defconfig
>>>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>>>> but not used [-Wunused-but-set-variable]
>>>>>      bool above_16MB;
>>>>>           ^~~~~~~~~~
>>>>>      CC      spl/lib/membuff.o
>>>>>
>>>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>>>> consider the given len as read_len.
>>>>>
>>>>> Will send separate patch for this.
>>>> Will You send a separate patch? or I include it in this patch set?
>>> This should not be a separate patch. Since every patch needs to pass
>>> buildman testing.
>> But it is not introduced by this patch set. So should be a separate patch to
>> fix.
> Do you mean the build warnings exist in current u-boot/master?
Here are two issue, one is the build warning, other is the issue with 
spi_flash_cmd_read_ops.
The build warning is related with this patch, I will fix it in the next 
version.
But  the issue with spi_flash_cmd_read_ops on bank length is not related 
with this patch.

>
> If so, Jagan can you please explain why you mention this? This is
> nothing related to this patch review.
>
> Regards,
> Bin

Best Regards,
Wenyou Yang

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

Hi Bin,

On Wed, Aug 30, 2017 at 11:11 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>
>>
>> On 2017/8/30 11:43, Bin Meng wrote:
>>>
>>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
>>> <Wenyou.Yang@microchip.com> wrote:
>>>>
>>>>
>>>> On 2017/8/26 14:34, Jagan Teki wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Thanks for the changes.
>>>>>
>>>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
>>>>> <wenyou.yang@microchip.com>
>>>>> wrote:
>>>>>>
>>>>>> This series of patches are based and have been tested on the 'master'
>>>>>> branch of the u-boot.git tree.
>>>>>>
>>>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI
>>>>>> and
>>>>>> QSPI controllers.
>>>>>>
>>>>>> The following tests have been passed:
>>>>>>
>>>>>> - QSPI0 + Macronix MX25L25673G:
>>>>>>     + probe: OK
>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>     + Page Program 1-1-4 at offset 0x10000: OK
>>>>>>       The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>>>       supported, not Page Program 1-1-4, however it worked, I don't
>>>>>> know
>>>>>>       why...
>>>>>>
>>>>>> - QSPI0 + Microchip SST26
>>>>>>     + probe: OK
>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>       SST26 memories support Page Program 1-4-4 but with the op code of
>>>>>>       Page Program 1-1-4, which is not standard so I don't use it.
>>>>>>
>>>>>> - QSPI0 + Adesto AT25DF321A
>>>>>>     + probe: OK
>>>>>>     + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>
>>>>>> - SPI0 + Adesto AT25DF321A
>>>>>>     + probe: OK
>>>>>>     + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>>>     + Page Program 1-1-1 at offest 0x6000: OK
>>>>>>
>>>>>> - SPI1 + Atmel AT45
>>>>>>     + probe: OK
>>>>>>     + Read at offset 0 and other than 0: OK
>>>>>>     + Write at offset 0 and other than 0: OK
>>>>>>
>>>>>> During my tests, I used:
>>>>>>     - setenv/saveenv, reboot, printenv
>>>>>>     or
>>>>>>     - sf probe, sf read, sf write, sf erase and sf update.
>>>>>>
>>>>>> Changes in v3:
>>>>>>    - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>>>
>>>>>> Changes in v2:
>>>>>>    - Rebase on the latest u-boot/master(2710d54f5).
>>>>>>
>>>>>> Cyrille Pitchen (8):
>>>>>>     spi: add support of SPI flash commands
>>>>>>     sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>>>     sf: select the relevant SPI flash protocol for read and write
>>>>>> commands
>>>>>>     sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>>>     sf: add 'addr_len' member to 'struct spi_flash'
>>>>>>     sf: add new option to support SPI flash above 16MiB
>>>>>>     sf: add support to Microchip SST26 QSPI memories
>>>>>>     sf: add driver for Atmel QSPI controller
>>>>>
>>>>> Comments:
>>>>> How about writing struct spi_flash_command in spi_flash area
>>>>> (include/spi_flash.h)? and then write atmel_qspi with
>>>>> UCLASS_SPI_FLASH?
>>>>>
>>>>> Testing:
>>>>> Basic testing works fine.
>>>>>
>>>>> Issues:
>>>>> - Build issue: with zynq_microzed_defconfig
>>>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>>>> but not used [-Wunused-but-set-variable]
>>>>>     bool above_16MB;
>>>>>          ^~~~~~~~~~
>>>>>     CC      spl/lib/membuff.o
>>>>>
>>>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>>>> consider the given len as read_len.
>>>>>
>>>>> Will send separate patch for this.
>>>>
>>>> Will You send a separate patch? or I include it in this patch set?
>>>
>>> This should not be a separate patch. Since every patch needs to pass
>>> buildman testing.
>>
>> But it is not introduced by this patch set. So should be a separate patch to
>> fix.
>
> Do you mean the build warnings exist in current u-boot/master?
>
> If so, Jagan can you please explain why you mention this? This is
> nothing related to this patch review.

Please read my previous comments, I was clearly explain the issue and
diff. Issue came up this series with 4BAIS on spi_flash_cmd_read_ops

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

Hi Wenyou Yang,

On Wed, Aug 30, 2017 at 7:28 AM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
> Hi Jagan,
>
>
> On 2017/8/26 14:34, Jagan Teki wrote:
>>
>> Hi,
>>
>> Thanks for the changes.
>>
>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com>
>> wrote:
>>>
>>> This series of patches are based and have been tested on the 'master'
>>> branch of the u-boot.git tree.
>>>
>>> Tests were passed with a sama5d2 xplained board which embeds both SPI and
>>> QSPI controllers.
>>>
>>> The following tests have been passed:
>>>
>>> - QSPI0 + Macronix MX25L25673G:
>>>    + probe: OK
>>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-4 at offset 0x10000: OK
>>>      The Macronix datasheet tells that only Page Program 1-4-4 is
>>>      supported, not Page Program 1-1-4, however it worked, I don't know
>>>      why...
>>>
>>> - QSPI0 + Microchip SST26
>>>    + probe: OK
>>>    + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offset 0x10000: OK
>>>      SST26 memories support Page Program 1-4-4 but with the op code of
>>>      Page Program 1-1-4, which is not standard so I don't use it.
>>>
>>> - QSPI0 + Adesto AT25DF321A
>>>    + probe: OK
>>>    + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offset 0x10000: OK
>>>
>>> - SPI0 + Adesto AT25DF321A
>>>    + probe: OK
>>>    + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>    + Page Program 1-1-1 at offest 0x6000: OK
>>>
>>> - SPI1 + Atmel AT45
>>>    + probe: OK
>>>    + Read at offset 0 and other than 0: OK
>>>    + Write at offset 0 and other than 0: OK
>>>
>>> During my tests, I used:
>>>    - setenv/saveenv, reboot, printenv
>>>    or
>>>    - sf probe, sf read, sf write, sf erase and sf update.
>>>
>>> Changes in v3:
>>>   - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>
>>> Changes in v2:
>>>   - Rebase on the latest u-boot/master(2710d54f5).
>>>
>>> Cyrille Pitchen (8):
>>>    spi: add support of SPI flash commands
>>>    sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>    sf: select the relevant SPI flash protocol for read and write commands
>>>    sf: differentiate Page Program 1-1-4 and 1-4-4
>>>    sf: add 'addr_len' member to 'struct spi_flash'
>>>    sf: add new option to support SPI flash above 16MiB
>>>    sf: add support to Microchip SST26 QSPI memories
>>>    sf: add driver for Atmel QSPI controller
>>
>> Comments:
>> How about writing struct spi_flash_command in spi_flash area
>> (include/spi_flash.h)? and then write atmel_qspi with
>> UCLASS_SPI_FLASH?
>
> The spi_flash_command struct describes the relevant features of the spi
> controller, instead of the spi_flash device.
> The purpose of patch set is used to supersede the spi_xfer() function to
> access the spi_flash device.
> So putting it in include/spi.h is suitable, we should not move it in the
> spi_flash area.
>
> Moreover, why do we write atmel_qspi with  UCLASS_SPI_FLASH?  It is not easy
> to understand.

I will send explanation for these on respective patches, that would
rather clean to understand from code point-of-view.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Bin Meng]

Hi Jagan,

On Wed, Aug 30, 2017 at 2:30 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi Bin,
>
> On Wed, Aug 30, 2017 at 11:11 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>
>>>
>>> On 2017/8/30 11:43, Bin Meng wrote:
>>>>
>>>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
>>>> <Wenyou.Yang@microchip.com> wrote:
>>>>>
>>>>>
>>>>> On 2017/8/26 14:34, Jagan Teki wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Thanks for the changes.
>>>>>>
>>>>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
>>>>>> <wenyou.yang@microchip.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> This series of patches are based and have been tested on the 'master'
>>>>>>> branch of the u-boot.git tree.
>>>>>>>
>>>>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI
>>>>>>> and
>>>>>>> QSPI controllers.
>>>>>>>
>>>>>>> The following tests have been passed:
>>>>>>>
>>>>>>> - QSPI0 + Macronix MX25L25673G:
>>>>>>>     + probe: OK
>>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>>     + Page Program 1-1-4 at offset 0x10000: OK
>>>>>>>       The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>>>>       supported, not Page Program 1-1-4, however it worked, I don't
>>>>>>> know
>>>>>>>       why...
>>>>>>>
>>>>>>> - QSPI0 + Microchip SST26
>>>>>>>     + probe: OK
>>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>>       SST26 memories support Page Program 1-4-4 but with the op code of
>>>>>>>       Page Program 1-1-4, which is not standard so I don't use it.
>>>>>>>
>>>>>>> - QSPI0 + Adesto AT25DF321A
>>>>>>>     + probe: OK
>>>>>>>     + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>>
>>>>>>> - SPI0 + Adesto AT25DF321A
>>>>>>>     + probe: OK
>>>>>>>     + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>>>>     + Page Program 1-1-1 at offest 0x6000: OK
>>>>>>>
>>>>>>> - SPI1 + Atmel AT45
>>>>>>>     + probe: OK
>>>>>>>     + Read at offset 0 and other than 0: OK
>>>>>>>     + Write at offset 0 and other than 0: OK
>>>>>>>
>>>>>>> During my tests, I used:
>>>>>>>     - setenv/saveenv, reboot, printenv
>>>>>>>     or
>>>>>>>     - sf probe, sf read, sf write, sf erase and sf update.
>>>>>>>
>>>>>>> Changes in v3:
>>>>>>>    - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>>    - Rebase on the latest u-boot/master(2710d54f5).
>>>>>>>
>>>>>>> Cyrille Pitchen (8):
>>>>>>>     spi: add support of SPI flash commands
>>>>>>>     sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>>>>     sf: select the relevant SPI flash protocol for read and write
>>>>>>> commands
>>>>>>>     sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>>>>     sf: add 'addr_len' member to 'struct spi_flash'
>>>>>>>     sf: add new option to support SPI flash above 16MiB
>>>>>>>     sf: add support to Microchip SST26 QSPI memories
>>>>>>>     sf: add driver for Atmel QSPI controller
>>>>>>
>>>>>> Comments:
>>>>>> How about writing struct spi_flash_command in spi_flash area
>>>>>> (include/spi_flash.h)? and then write atmel_qspi with
>>>>>> UCLASS_SPI_FLASH?
>>>>>>
>>>>>> Testing:
>>>>>> Basic testing works fine.
>>>>>>
>>>>>> Issues:
>>>>>> - Build issue: with zynq_microzed_defconfig
>>>>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>>>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>>>>> but not used [-Wunused-but-set-variable]
>>>>>>     bool above_16MB;
>>>>>>          ^~~~~~~~~~
>>>>>>     CC      spl/lib/membuff.o
>>>>>>
>>>>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>>>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>>>>> consider the given len as read_len.
>>>>>>
>>>>>> Will send separate patch for this.
>>>>>
>>>>> Will You send a separate patch? or I include it in this patch set?
>>>>
>>>> This should not be a separate patch. Since every patch needs to pass
>>>> buildman testing.
>>>
>>> But it is not introduced by this patch set. So should be a separate patch to
>>> fix.
>>
>> Do you mean the build warnings exist in current u-boot/master?
>>
>> If so, Jagan can you please explain why you mention this? This is
>> nothing related to this patch review.
>
> Please read my previous comments, I was clearly explain the issue and
> diff. Issue came up this series with 4BAIS on spi_flash_cmd_read_ops
>

So your words and Wenyou are contradictory. Wenyou claimed the
warnings do not come from his patchset while you said yes. Anyway will
leave you guys to resolve.

My point is: if the warnings exist in current mainline, this should be
fixed as a unrelated patch. If the warnings come from this series,
this should be fixed in the particular patch that introduces the
warnings!

Regards,
Bin

[U-Boot] [PATCH v3 0/8] sf: improve support of (Q)SPI flash memories

[Jagan Teki]

Hi Bin,

On Wed, Aug 30, 2017 at 1:17 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Jagan,
>
> On Wed, Aug 30, 2017 at 2:30 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Hi Bin,
>>
>> On Wed, Aug 30, 2017 at 11:11 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> On Wed, Aug 30, 2017 at 1:27 PM, Yang, Wenyou <Wenyou.Yang@microchip.com> wrote:
>>>>
>>>>
>>>> On 2017/8/30 11:43, Bin Meng wrote:
>>>>>
>>>>> On Wed, Aug 30, 2017 at 11:25 AM, Yang, Wenyou
>>>>> <Wenyou.Yang@microchip.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 2017/8/26 14:34, Jagan Teki wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thanks for the changes.
>>>>>>>
>>>>>>> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
>>>>>>> <wenyou.yang@microchip.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> This series of patches are based and have been tested on the 'master'
>>>>>>>> branch of the u-boot.git tree.
>>>>>>>>
>>>>>>>> Tests were passed with a sama5d2 xplained board which embeds both SPI
>>>>>>>> and
>>>>>>>> QSPI controllers.
>>>>>>>>
>>>>>>>> The following tests have been passed:
>>>>>>>>
>>>>>>>> - QSPI0 + Macronix MX25L25673G:
>>>>>>>>     + probe: OK
>>>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>>>     + Page Program 1-1-4 at offset 0x10000: OK
>>>>>>>>       The Macronix datasheet tells that only Page Program 1-4-4 is
>>>>>>>>       supported, not Page Program 1-1-4, however it worked, I don't
>>>>>>>> know
>>>>>>>>       why...
>>>>>>>>
>>>>>>>> - QSPI0 + Microchip SST26
>>>>>>>>     + probe: OK
>>>>>>>>     + Fast Read 1-1-4 at offset 0x10000 (u-boot env): OK
>>>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>>>       SST26 memories support Page Program 1-4-4 but with the op code of
>>>>>>>>       Page Program 1-1-4, which is not standard so I don't use it.
>>>>>>>>
>>>>>>>> - QSPI0 + Adesto AT25DF321A
>>>>>>>>     + probe: OK
>>>>>>>>     + Fast Read 1-1-1 at offset 0x10000 (u-boot env): OK
>>>>>>>>     + Page Program 1-1-1 at offset 0x10000: OK
>>>>>>>>
>>>>>>>> - SPI0 + Adesto AT25DF321A
>>>>>>>>     + probe: OK
>>>>>>>>     + Fast Read 1-1-1 at offset 0x6000 (u-boot env): OK
>>>>>>>>     + Page Program 1-1-1 at offest 0x6000: OK
>>>>>>>>
>>>>>>>> - SPI1 + Atmel AT45
>>>>>>>>     + probe: OK
>>>>>>>>     + Read at offset 0 and other than 0: OK
>>>>>>>>     + Write at offset 0 and other than 0: OK
>>>>>>>>
>>>>>>>> During my tests, I used:
>>>>>>>>     - setenv/saveenv, reboot, printenv
>>>>>>>>     or
>>>>>>>>     - sf probe, sf read, sf write, sf erase and sf update.
>>>>>>>>
>>>>>>>> Changes in v3:
>>>>>>>>    - Add the include <spi.h> to fix build error for corvus_defconfig.
>>>>>>>>
>>>>>>>> Changes in v2:
>>>>>>>>    - Rebase on the latest u-boot/master(2710d54f5).
>>>>>>>>
>>>>>>>> Cyrille Pitchen (8):
>>>>>>>>     spi: add support of SPI flash commands
>>>>>>>>     sf: describe all SPI flash commands with 'struct spi_flash_command'
>>>>>>>>     sf: select the relevant SPI flash protocol for read and write
>>>>>>>> commands
>>>>>>>>     sf: differentiate Page Program 1-1-4 and 1-4-4
>>>>>>>>     sf: add 'addr_len' member to 'struct spi_flash'
>>>>>>>>     sf: add new option to support SPI flash above 16MiB
>>>>>>>>     sf: add support to Microchip SST26 QSPI memories
>>>>>>>>     sf: add driver for Atmel QSPI controller
>>>>>>>
>>>>>>> Comments:
>>>>>>> How about writing struct spi_flash_command in spi_flash area
>>>>>>> (include/spi_flash.h)? and then write atmel_qspi with
>>>>>>> UCLASS_SPI_FLASH?
>>>>>>>
>>>>>>> Testing:
>>>>>>> Basic testing works fine.
>>>>>>>
>>>>>>> Issues:
>>>>>>> - Build issue: with zynq_microzed_defconfig
>>>>>>> drivers/mtd/spi/spi_flash.c: In function ‘spi_flash_scan’:
>>>>>>> drivers/mtd/spi/spi_flash.c:1049:7: warning: variable ‘above_16MB’ set
>>>>>>> but not used [-Wunused-but-set-variable]
>>>>>>>     bool above_16MB;
>>>>>>>          ^~~~~~~~~~
>>>>>>>     CC      spl/lib/membuff.o
>>>>>>>
>>>>>>> - issue with spi_flash_cmd_read_ops 4BAIS
>>>>>>> Need to calculate bank length only if BAR is in use. Otherwise,
>>>>>>> consider the given len as read_len.
>>>>>>>
>>>>>>> Will send separate patch for this.
>>>>>>
>>>>>> Will You send a separate patch? or I include it in this patch set?
>>>>>
>>>>> This should not be a separate patch. Since every patch needs to pass
>>>>> buildman testing.
>>>>
>>>> But it is not introduced by this patch set. So should be a separate patch to
>>>> fix.
>>>
>>> Do you mean the build warnings exist in current u-boot/master?
>>>
>>> If so, Jagan can you please explain why you mention this? This is
>>> nothing related to this patch review.
>>
>> Please read my previous comments, I was clearly explain the issue and
>> diff. Issue came up this series with 4BAIS on spi_flash_cmd_read_ops
>>
>
> So your words and Wenyou are contradictory. Wenyou claimed the
> warnings do not come from his patchset while you said yes. Anyway will
> leave you guys to resolve.

No, I didn't say that I wrote "Will send separate patch for this."
for 4BIAS fix not for the warning issue.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands

[Jagan Teki]

On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
> From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>
> This patch introduces 'struct spi_flash_command' and functions
> spi_is_flash_command_supported() / spi_exec_flash_command().
>

Answer for why this shouldn't be part of SPI area.

[snip]

drivers/spi and include/spi.h is Generic SPI area code - this can deal
all connected slave devices like EEPROM, SPI-NOR etc
drivers/mtd/spi and include/spi_flash.h is SPI-Flash or SPI-NOR code -
this can handle only SPI flashes.

>
> +bool dm_spi_is_flash_command_supported(struct udevice *dev,
> +                                      const struct spi_flash_command *cmd)
> +{
> +       struct udevice *bus = dev->parent;
> +       struct dm_spi_ops *ops = spi_get_ops(bus);
> +
> +       if (ops->is_flash_command_supported)
> +               return ops->is_flash_command_supported(dev, cmd);
> +
> +       return false;
> +}
> +
> +int dm_spi_exec_flash_command(struct udevice *dev,
> +                             const struct spi_flash_command *cmd)
> +{
> +       struct udevice *bus = dev->parent;
> +       struct dm_spi_ops *ops = spi_get_ops(bus);
> +
> +       if (ops->exec_flash_command)
> +               return ops->exec_flash_command(dev, cmd);
> +
> +       return -EINVAL;
> +}
> +
>  int spi_claim_bus(struct spi_slave *slave)
>  {
>         return dm_spi_claim_bus(slave->dev);
> @@ -107,6 +131,18 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
>         return dm_spi_xfer(slave->dev, bitlen, dout, din, flags);
>  }
>
> +bool spi_is_flash_command_supported(struct spi_slave *slave,
> +                                   const struct spi_flash_command *cmd)
> +{
> +       return dm_spi_is_flash_command_supported(slave->dev, cmd);
> +}
> +
> +int spi_exec_flash_command(struct spi_slave *slave,
> +                          const struct spi_flash_command *cmd)
> +{
> +       return dm_spi_exec_flash_command(slave->dev, cmd);
> +}

Handling spi-flash stuff in spi core is not proper way of dealing, and
I saw these functions are not-at-all needed for generic controller
drivers in drivers/spi except the Atmel QSPI driver (in v3,8/8).

> +
>  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
>  static int spi_child_post_bind(struct udevice *dev)
>  {
> @@ -144,6 +180,10 @@ static int spi_post_probe(struct udevice *bus)
>                 ops->set_mode += gd->reloc_off;
>         if (ops->cs_info)
>                 ops->cs_info += gd->reloc_off;
> +       if (ops->is_flash_command_supported)
> +               ops->is_flash_command_supported += gd->reloc_off;
> +       if (ops->exec_flash_command)
> +               ops->exec_flash_command += gd->reloc_off;
>  #endif
>
>         return 0;
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 7d81fbd7f8..e47acdc9e4 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <common.h>
> +#include <errno.h>
>  #include <fdtdec.h>
>  #include <malloc.h>
>  #include <spi.h>
> @@ -58,3 +59,15 @@ struct spi_slave *spi_base_setup_slave_fdt(const void *blob, int busnum,
>         return spi_setup_slave(busnum, cs, max_hz, mode);
>  }
>  #endif
> +
> +__weak bool spi_is_flash_command_supported(struct spi_slave *slave,
> +                                          const struct spi_flash_command *cmd)
> +{
> +       return false;
> +}
> +
> +__weak int spi_exec_flash_command(struct spi_slave *slave,
> +                                 const struct spi_flash_command *cmd)
> +{
> +       return -EINVAL;
> +}
> diff --git a/include/spi.h b/include/spi.h
> index 8c4b882c54..a090266b52 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -10,6 +10,8 @@
>  #ifndef _SPI_H_
>  #define _SPI_H_
>
> +#include <linux/string.h> /* memset() */
> +
>  /* SPI mode flags */
>  #define SPI_CPHA       BIT(0)                  /* clock phase */
>  #define SPI_CPOL       BIT(1)                  /* clock polarity */
> @@ -64,6 +66,116 @@ struct dm_spi_slave_platdata {
>  #endif /* CONFIG_DM_SPI */
>
>  /**
> + * enum spi_flash_protocol - SPI flash command protocol
> + */
> +#define SPI_FPROTO_INST_SHIFT  16
> +#define SPI_FPROTO_INST_MASK   GENMASK(23, 16)
> +#define SPI_FPROTO_INST(nbits)                                 \
> +       ((((unsigned long)(nbits)) << SPI_FPROTO_INST_SHIFT) &  \
> +        SPI_FPROTO_INST_MASK)
> +
> +#define SPI_FPROTO_ADDR_SHIFT  8
> +#define SPI_FPROTO_ADDR_MASK   GENMASK(15, 8)
> +#define SPI_FPROTO_ADDR(nbits)                                 \
> +       ((((unsigned long)(nbits)) << SPI_FPROTO_ADDR_SHIFT) &  \
> +        SPI_FPROTO_ADDR_MASK)
> +
> +#define SPI_FPROTO_DATA_SHIFT  0
> +#define SPI_FPROTO_DATA_MASK   GENMASK(7, 0)
> +#define SPI_FPROTO_DATA(nbits)                                 \
> +       ((((unsigned long)(nbits)) << SPI_FPROTO_DATA_SHIFT) &  \
> +        SPI_FPROTO_DATA_MASK)
> +
> +#define SPI_FPROTO(inst_nbits, addr_nbits, data_nbits) \
> +       (SPI_FPROTO_INST(inst_nbits) |                  \
> +        SPI_FPROTO_ADDR(addr_nbits) |                  \
> +        SPI_FPROTO_DATA(data_nbits))
> +
> +enum spi_flash_protocol {
> +       SPI_FPROTO_1_1_1 = SPI_FPROTO(1, 1, 1),
> +       SPI_FPROTO_1_1_2 = SPI_FPROTO(1, 1, 2),
> +       SPI_FPROTO_1_1_4 = SPI_FPROTO(1, 1, 4),
> +       SPI_FPROTO_1_2_2 = SPI_FPROTO(1, 2, 2),
> +       SPI_FPROTO_1_4_4 = SPI_FPROTO(1, 4, 4),
> +       SPI_FPROTO_2_2_2 = SPI_FPROTO(2, 2, 2),
> +       SPI_FPROTO_4_4_4 = SPI_FPROTO(4, 4, 4),
> +};

these protocol also includes IO's like dual and quad and IO's which
are specific for SPI-NOR flash and not Generic SPI protocols.

> +
> +static inline
> +u8 spi_flash_protocol_get_inst_nbits(enum spi_flash_protocol proto)
> +{
> +       return ((unsigned long)(proto & SPI_FPROTO_INST_MASK)) >>
> +               SPI_FPROTO_INST_SHIFT;
> +}
> +
> +static inline
> +u8 spi_flash_protocol_get_addr_nbits(enum spi_flash_protocol proto)
> +{
> +       return ((unsigned long)(proto & SPI_FPROTO_ADDR_MASK)) >>
> +               SPI_FPROTO_ADDR_SHIFT;
> +}
> +
> +static inline
> +u8 spi_flash_protocol_get_data_nbits(enum spi_flash_protocol proto)
> +{
> +       return ((unsigned long)(proto & SPI_FPROTO_DATA_MASK)) >>
> +               SPI_FPROTO_DATA_SHIFT;
> +}
> +
> +/**
> + * struct spi_flash_command - SPI flash command structure
> + *
> + * @instr:             Opcode sent to the SPI slave during instr clock cycles.
> + * @mode:              Value sent to the SPI slave during mode clock cycles.
> + * @num_mode_cycles:   Number of mode clock cycles.
> + * @num_wait_states:   Number of wait-state clock cycles.
> + * @addr_len:          Number of bytes sent during address clock cycles:
> + *                     should be 0, 3, or 4.
> + * @addr:              Value sent to the SPI slave during address clock cycles.
> + * @data_len:          Number of bytes to be sent during data clock cycles.
> + * @tx_data:           Data sent to the SPI slave during data clock cycles.
> + * @rx_data:           Data read from the SPI slave during data clock cycles.
> + */
> +struct spi_flash_command {
> +       enum spi_flash_protocol proto;
> +       u8 flags;
> +#define SPI_FCMD_TYPE          GENMASK(2, 0)
> +#define SPI_FCMD_READ          (0x0U << 0)
> +#define SPI_FCMD_WRITE         (0x1U << 0)
> +#define SPI_FCMD_ERASE         (0x2U << 0)
> +#define SPI_FCMD_READ_REG      (0x3U << 0)
> +#define SPI_FCMD_WRITE_REG     (0x4U << 0)
> +
> +       u8 inst;
> +       u8 mode;
> +       u8 num_mode_cycles;
> +       u8 num_wait_states;
> +       u8 addr_len;
> +       u32 addr;
> +       size_t data_len;
> +       const void *tx_data;
> +       void *rx_data;
> +};

I saw from the other patches, this structure is initializing for each
I/O interface between SPI to driver/mtd like spi_transfer, if so it
shouldn't have flash attributes. Better to move flash contents to
spi_flash structure and do the respective operations.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 8/8] sf: add driver for Atmel QSPI controller

[Jagan Teki]

On Tue, Jul 25, 2017 at 12:31 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
> From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>
> This patch adds support to the Atmel Quad SPI controller.

So, this is SPI flash driver not Generic SPI - the problem with these
were resides at drivers/spi is entire SPI layer becomes SPI-flash
oriented which is absolutely a wrong indication where SPI layer
getting effected more with flash operation.

>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
> ---
>
> Changes in v3: None
> Changes in v2:
>  - Rebase on the latest u-boot/master(2710d54f5).
>
>  drivers/spi/Kconfig      |   7 +
>  drivers/spi/Makefile     |   1 +
>  drivers/spi/atmel_qspi.c | 404 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/spi/atmel_qspi.h | 169 ++++++++++++++++++++
>  4 files changed, 581 insertions(+)
>  create mode 100644 drivers/spi/atmel_qspi.c
>  create mode 100644 drivers/spi/atmel_qspi.h
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 8a8e8e480f..b3e8cb941e 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -32,6 +32,13 @@ config ATH79_SPI
>           uses driver model and requires a device tree binding to operate.
>           please refer to doc/device-tree-bindings/spi/spi-ath79.txt.
>
> +config ATMEL_QSPI
> +       bool "Atmel QSPI driver"
> +       depends on ARCH_AT91
> +       help
> +         Enable the Ateml Quad-SPI (QSPI) driver. This driver can only be
> +         used to access SPI NOR flashes.
> +
>  config ATMEL_SPI
>         bool "Atmel SPI driver"
>         depends on ARCH_AT91
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 9f8b86de76..358db2b065 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -18,6 +18,7 @@ endif
>  obj-$(CONFIG_ALTERA_SPI) += altera_spi.o
>  obj-$(CONFIG_ATH79_SPI) += ath79_spi.o
>  obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o
> +obj-$(CONFIG_ATMEL_QSPI) += atmel_qspi.o
>  obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o
>  obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o cadence_qspi_apb.o
>  obj-$(CONFIG_CF_SPI) += cf_spi.o
> diff --git a/drivers/spi/atmel_qspi.c b/drivers/spi/atmel_qspi.c
> new file mode 100644
> index 0000000000..0af7a4dc0a
> --- /dev/null
> +++ b/drivers/spi/atmel_qspi.c
> @@ -0,0 +1,404 @@
> +/*
> + * Copyright (C) 2017 Atmel Corporation
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <spi.h>
> +#include <asm/io.h>
> +#include <mach/clk.h>
> +#include "atmel_qspi.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void atmel_qspi_memcpy_fromio(void *dst, unsigned long src, size_t len)
> +{
> +       u8 *d = (u8 *)dst;
> +
> +       while (len--) {
> +               *d++ = readb(src);
> +               src++;
> +       }
> +}
> +
> +static void atmel_qspi_memcpy_toio(unsigned long dst, const void *src,
> +                                  size_t len)
> +{
> +       const u8 *s = (const u8 *)src;
> +
> +       while (len--) {
> +               writeb(*s, dst);
> +               dst++;
> +               s++;
> +       }
> +}
> +
> +static int atmel_qspi_set_ifr_tfrtype(u8 flags, u32 *ifr)
> +{
> +       u32 ifr_tfrtype;
> +
> +       switch (flags & SPI_FCMD_TYPE) {
> +       case SPI_FCMD_READ:
> +               ifr_tfrtype = QSPI_IFR_TFRTYPE_READ_MEMORY;
> +               break;
> +
> +       case SPI_FCMD_WRITE:
> +               ifr_tfrtype = QSPI_IFR_TFRTYPE_WRITE_MEMORY;
> +               break;
> +
> +       case SPI_FCMD_ERASE:
> +       case SPI_FCMD_WRITE_REG:
> +               ifr_tfrtype = QSPI_IFR_TFRTYPE_WRITE;
> +               break;
> +
> +       case SPI_FCMD_READ_REG:
> +               ifr_tfrtype = QSPI_IFR_TFRTYPE_READ;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       *ifr = (*ifr & ~QSPI_IFR_TFRTYPE) | ifr_tfrtype;
> +       return 0;
> +}
> +
> +static int atmel_qpsi_set_ifr_width(enum spi_flash_protocol proto, u32 *ifr)
> +{
> +       u32 ifr_width;
> +
> +       switch (proto) {
> +       case SPI_FPROTO_1_1_1:
> +               ifr_width = QSPI_IFR_WIDTH_SINGLE_BIT_SPI;
> +               break;
> +
> +       case SPI_FPROTO_1_1_2:
> +               ifr_width = QSPI_IFR_WIDTH_DUAL_OUTPUT;
> +               break;
> +
> +       case SPI_FPROTO_1_2_2:
> +               ifr_width = QSPI_IFR_WIDTH_DUAL_IO;
> +               break;
> +
> +       case SPI_FPROTO_2_2_2:
> +               ifr_width = QSPI_IFR_WIDTH_DUAL_CMD;
> +               break;
> +
> +       case SPI_FPROTO_1_1_4:
> +               ifr_width = QSPI_IFR_WIDTH_QUAD_OUTPUT;
> +               break;
> +
> +       case SPI_FPROTO_1_4_4:
> +               ifr_width = QSPI_IFR_WIDTH_QUAD_IO;
> +               break;
> +
> +       case SPI_FPROTO_4_4_4:
> +               ifr_width = QSPI_IFR_WIDTH_QUAD_CMD;
> +               break;
> +
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       *ifr = (*ifr & ~QSPI_IFR_WIDTH) | ifr_width;
> +       return 0;
> +}
> +
> +static int atmel_qspi_xfer(struct udevice *dev, unsigned int bitlen,
> +                          const void *dout, void *din, unsigned long flags)
> +{
> +       /* This controller can only be used with SPI NOR flashes. */
> +       return -EINVAL;
> +}
> +
> +static int atmel_qspi_set_speed(struct udevice *bus, uint hz)
> +{
> +       struct atmel_qspi_priv *aq = dev_get_priv(bus);
> +       u32 scr, scbr, mask, new_value;
> +
> +       /* Compute the QSPI baudrate */
> +       scbr = DIV_ROUND_UP(aq->bus_clk_rate, hz);
> +       if (scbr > 0)
> +               scbr--;
> +
> +       new_value = QSPI_SCR_SCBR_(scbr);
> +       mask = QSPI_SCR_SCBR;
> +
> +       scr = qspi_readl(aq, QSPI_SCR);
> +       if ((scr & mask) == new_value)
> +               return 0;
> +
> +       scr = (scr & ~mask) | new_value;
> +       qspi_writel(aq, QSPI_SCR, scr);
> +
> +       return 0;
> +}
> +
> +static int atmel_qspi_set_mode(struct udevice *bus, uint mode)
> +{
> +       struct atmel_qspi_priv *aq = dev_get_priv(bus);
> +       u32 scr, mask, new_value;
> +
> +       new_value = (QSPI_SCR_CPOL_((mode & SPI_CPOL) != 0) |
> +                    QSPI_SCR_CPHA_((mode & SPI_CPHA) != 0));
> +       mask = (QSPI_SCR_CPOL | QSPI_SCR_CPHA);
> +
> +       scr = qspi_readl(aq, QSPI_SCR);
> +       if ((scr & mask) == new_value)
> +               return 0;
> +
> +       scr = (scr & ~mask) | new_value;
> +       qspi_writel(aq, QSPI_SCR, scr);
> +
> +       return 0;
> +}
> +
> +static bool
> +atmel_qspi_is_flash_command_supported(struct udevice *dev,
> +                                     const struct spi_flash_command *cmd)
> +{
> +       return true;
> +}
> +
> +static int atmel_qspi_exec_flash_command(struct udevice *dev,
> +                                        const struct spi_flash_command *cmd)
> +{
> +       struct udevice *bus = dev_get_parent(dev);
> +       struct atmel_qspi_priv *aq = dev_get_priv(bus);
> +       unsigned int iar, icr, ifr;
> +       unsigned int offset;
> +       unsigned int imr, sr;
> +       unsigned long memaddr;
> +       int err;
> +
> +       iar = 0;
> +       icr = 0;
> +       ifr = 0;
> +
> +       err = atmel_qspi_set_ifr_tfrtype(cmd->flags, &ifr);
> +       if (err)
> +               return err;
> +
> +       err = atmel_qpsi_set_ifr_width(cmd->proto, &ifr);
> +       if (err)
> +               return err;
> +
> +       /* Compute instruction parameters */
> +       icr |= QSPI_ICR_INST_(cmd->inst);
> +       ifr |= QSPI_IFR_INSTEN;
> +
> +       /* Compute address parameters. */
> +       switch (cmd->addr_len) {
> +       case 4:
> +               ifr |= QSPI_IFR_ADDRL_32_BIT;
> +               /*break;*/ /* fall through the 24bit (3 byte) address case */
> +       case 3:
> +               iar = cmd->data_len ? 0 : cmd->addr;
> +               ifr |= QSPI_IFR_ADDREN;
> +               offset = cmd->addr;
> +               break;
> +       case 0:
> +               offset = 0;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       /* Compute option parameters. */
> +       if (cmd->num_mode_cycles) {
> +               unsigned int mode_cycle_bits, mode_bits;
> +
> +               icr |= QSPI_ICR_OPT_(cmd->mode);
> +               ifr |= QSPI_IFR_OPTEN;
> +
> +               switch (ifr & QSPI_IFR_WIDTH) {
> +               case QSPI_IFR_WIDTH_SINGLE_BIT_SPI:
> +               case QSPI_IFR_WIDTH_DUAL_OUTPUT:
> +               case QSPI_IFR_WIDTH_QUAD_OUTPUT:
> +                       mode_cycle_bits = 1;
> +                       break;
> +               case QSPI_IFR_WIDTH_DUAL_IO:
> +               case QSPI_IFR_WIDTH_DUAL_CMD:
> +                       mode_cycle_bits = 2;
> +                       break;
> +               case QSPI_IFR_WIDTH_QUAD_IO:
> +               case QSPI_IFR_WIDTH_QUAD_CMD:
> +                       mode_cycle_bits = 4;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
> +
> +               mode_bits = cmd->num_mode_cycles * mode_cycle_bits;
> +               switch (mode_bits) {
> +               case 1:
> +                       ifr |= QSPI_IFR_OPTL_1BIT;
> +                       break;
> +
> +               case 2:
> +                       ifr |= QSPI_IFR_OPTL_2BIT;
> +                       break;
> +
> +               case 4:
> +                       ifr |= QSPI_IFR_OPTL_4BIT;
> +                       break;
> +
> +               case 8:
> +                       ifr |= QSPI_IFR_OPTL_8BIT;
> +                       break;
> +
> +               default:
> +                       return -EINVAL;
> +               }
> +       }
> +
> +       /* Set the number of dummy cycles. */
> +       if (cmd->num_wait_states)
> +               ifr |= QSPI_IFR_NBDUM_(cmd->num_wait_states);
> +
> +       /* Set data enable. */
> +       if (cmd->data_len)
> +               ifr |= QSPI_IFR_DATAEN;
> +
> +       /* Clear pending interrupts. */
> +       (void)qspi_readl(aq, QSPI_SR);
> +
> +       /* Set QSPI Instruction Frame registers. */
> +       qspi_writel(aq, QSPI_IAR, iar);
> +       qspi_writel(aq, QSPI_ICR, icr);
> +       qspi_writel(aq, QSPI_IFR, ifr);
> +
> +       /* Skip to the final steps if there is no data. */
> +       if (!cmd->data_len)
> +               goto no_data;
> +
> +       /* Dummy read of QSPI_IFR to synchronize APB and AHB accesses. */
> +       (void)qspi_readl(aq, QSPI_IFR);
> +
> +       /* Stop here for Continuous Read. */
> +       memaddr = (unsigned long)(aq->membase + offset);
> +       if (cmd->tx_data)
> +               /* Write data. */
> +               atmel_qspi_memcpy_toio(memaddr, cmd->tx_data, cmd->data_len);
> +       else if (cmd->rx_data)
> +               /* Read data. */
> +               atmel_qspi_memcpy_fromio(cmd->rx_data, memaddr, cmd->data_len);
> +
> +       /* Release the chip-select. */
> +       qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
> +
> +no_data:
> +       /* Poll INSTruction End and Chip Select Rise flags. */
> +       imr = QSPI_SR_INSTRE | QSPI_SR_CSR;
> +       sr = 0;
> +       while (sr != (QSPI_SR_INSTRE | QSPI_SR_CSR))
> +               sr |= qspi_readl(aq, QSPI_SR) & imr;
> +
> +       return 0;
> +}

From v3 1/8 comments, this can be local function where we each IO
opeartions can initiate this run command.

> +
> +
> +static const struct dm_spi_ops atmel_qspi_ops = {
> +       .xfer                           = atmel_qspi_xfer,
> +       .set_speed                      = atmel_qspi_set_speed,
> +       .set_mode                       = atmel_qspi_set_mode,
> +       .is_flash_command_supported     = atmel_qspi_is_flash_command_supported,
> +       .exec_flash_command             = atmel_qspi_exec_flash_command,
> +};
> +
> +static int atmel_qspi_enable_clk(struct udevice *bus)
> +{
> +       struct atmel_qspi_priv *aq = dev_get_priv(bus);
> +       struct clk clk;
> +       ulong clk_rate;
> +       int ret;
> +
> +       ret = clk_get_by_index(bus, 0, &clk);
> +       if (ret)
> +               return -EINVAL;
> +
> +       ret = clk_enable(&clk);
> +       if (ret)
> +               goto free_clock;
> +
> +       clk_rate = clk_get_rate(&clk);
> +       if (!clk_rate) {
> +               ret = -EINVAL;
> +               goto free_clock;
> +       }
> +
> +       aq->bus_clk_rate = clk_rate;
> +
> +free_clock:
> +       clk_free(&clk);
> +
> +       return ret;
> +}
> +
> +static int atmel_qspi_probe(struct udevice *bus)
> +{
> +       const struct atmel_qspi_platdata *plat = dev_get_platdata(bus);
> +       struct atmel_qspi_priv *aq = dev_get_priv(bus);
> +       u32 mr;
> +       int ret;
> +
> +       ret = atmel_qspi_enable_clk(bus);
> +       if (ret)
> +               return ret;
> +
> +       aq->regbase = plat->regbase;
> +       aq->membase = plat->membase;
> +
> +       /* Reset the QSPI controler */
> +       qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
> +
> +       /* Set the QSPI controller in Serial Memory Mode */
> +       mr = (QSPI_MR_NBBITS_8_BIT |
> +             QSPI_MR_SMM_MEMORY |
> +             QSPI_MR_CSMODE_LASTXFER);
> +       qspi_writel(aq, QSPI_MR, mr);
> +
> +       /* Enable the QSPI controller */
> +       qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
> +
> +       return 0;
> +}
> +
> +static int atmel_qspi_ofdata_to_platdata(struct udevice *bus)
> +{
> +       struct atmel_qspi_platdata *plat = dev_get_platdata(bus);
> +       const void *blob = gd->fdt_blob;
> +       int node = dev_of_offset(bus);
> +       u32 data[4];
> +       int ret;
> +
> +       ret = fdtdec_get_int_array(blob, node, "reg", data, ARRAY_SIZE(data));
> +       if (ret) {
> +               printf("Error: Can't get base addresses (ret=%d)!\n", ret);
> +               return -ENODEV;
> +       }
> +       plat->regbase = (void *)data[0];
> +       plat->membase = (void *)data[2];
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id atmel_qspi_ids[] = {
> +       { .compatible = "atmel,sama5d2-qspi" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(atmel_qspi) = {
> +       .name           = "atmel_qspi",
> +       .id             = UCLASS_SPI,

Summary, better to write UCLASS_SPI_FLASH driver.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 2/8] sf: describe all SPI flash commands with 'struct spi_flash_command'

[Jagan Teki]

On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com> wrote:
> From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>
> Now that the SPI sub-system API has been extended with
> 'struct spi_flash_command' and spi_is_flash_command_supported() /
> spi_exec_flash_command() functions, we update the SPI FLASH sub-system to
> use this new API.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Signed-off-by: Wenyou Yang <wenyou.yang@microchip.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  drivers/mtd/spi/sf.c           |  78 +++++++++++++----
>  drivers/mtd/spi/sf_dataflash.c | 119 +++++++++++++-------------
>  drivers/mtd/spi/sf_internal.h  |  24 +++---
>  drivers/mtd/spi/spi_flash.c    | 184 +++++++++++++++++++++++------------------
>  4 files changed, 236 insertions(+), 169 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf.c b/drivers/mtd/spi/sf.c
> index d5e175ca00..6178b0aa98 100644
> --- a/drivers/mtd/spi/sf.c
> +++ b/drivers/mtd/spi/sf.c
> @@ -9,46 +9,88 @@
>
>  #include <common.h>
>  #include <spi.h>
> +#include <spi_flash.h>
>
> -static int spi_flash_read_write(struct spi_slave *spi,
> -                               const u8 *cmd, size_t cmd_len,
> -                               const u8 *data_out, u8 *data_in,
> -                               size_t data_len)
> +#include "sf_internal.h"
> +
> +static void spi_flash_addr(u32 addr, u8 addr_len, u8 *cmd_buf)
>  {
> +       u8 i;
> +
> +       for (i = 0; i < addr_len; i++)
> +               cmd_buf[i] = addr >> ((addr_len - 1 - i) * 8);
> +}
> +
> +static u8 spi_compute_num_dummy_bytes(enum spi_flash_protocol proto,
> +                                     u8 num_dummy_clock_cycles)
> +{
> +       int shift = fls(spi_flash_protocol_get_addr_nbits(proto)) - 1;
> +
> +       if (shift < 0)
> +               shift = 0;
> +       return (num_dummy_clock_cycles << shift) >> 3;
> +}
> +
> +static int spi_flash_exec(struct spi_flash *flash,
> +                         const struct spi_flash_command *cmd)
> +{
> +       struct spi_slave *spi = flash->spi;
> +       u8 cmd_buf[SPI_FLASH_CMD_LEN];
> +       size_t cmd_len, num_dummy_bytes;
>         unsigned long flags = SPI_XFER_BEGIN;
>         int ret;
>
> -       if (data_len == 0)
> +       if (spi_is_flash_command_supported(spi, cmd))
> +               return spi_exec_flash_command(spi, cmd);
> +
> +       if (cmd->data_len == 0)
>                 flags |= SPI_XFER_END;
>
> -       ret = spi_xfer(spi, cmd_len * 8, cmd, NULL, flags);
> +       cmd_buf[0] = cmd->inst;
> +       spi_flash_addr(cmd->addr, cmd->addr_len, cmd_buf + 1);
> +       cmd_len = 1 + cmd->addr_len;
> +
> +       num_dummy_bytes = spi_compute_num_dummy_bytes(cmd->proto,
> +                                                     cmd->num_mode_cycles +
> +                                                     cmd->num_wait_states);
> +       memset(cmd_buf + cmd_len, 0xff, num_dummy_bytes);
> +       cmd_len += num_dummy_bytes;
> +
> +       ret = spi_xfer(spi, cmd_len * 8, cmd_buf, NULL, flags);
>         if (ret) {
>                 debug("SF: Failed to send command (%zu bytes): %d\n",
>                       cmd_len, ret);
> -       } else if (data_len != 0) {
> -               ret = spi_xfer(spi, data_len * 8, data_out, data_in,
> -                                       SPI_XFER_END);
> +       } else if (cmd->data_len != 0) {
> +               ret = spi_xfer(spi, cmd->data_len * 8,
> +                              cmd->tx_data, cmd->rx_data,
> +                              SPI_XFER_END);
>                 if (ret)
>                         debug("SF: Failed to transfer %zu bytes of data: %d\n",
> -                             data_len, ret);
> +                             cmd->data_len, ret);
>         }
>
>         return ret;
>  }
>
> -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(struct spi_flash *flash,
> +                      const struct spi_flash_command *cmd)
>  {
> -       return spi_flash_read_write(spi, cmd, cmd_len, NULL, data, data_len);
> +       return spi_flash_exec(flash, cmd);
>  }
>
> -int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len)
> +int spi_flash_cmd(struct spi_flash *flash, u8 instr, void *response, size_t len)
>  {
> -       return spi_flash_cmd_read(spi, &cmd, 1, response, len);
> +       struct spi_flash_command cmd;
> +       u8 flags = (response && len) ? SPI_FCMD_READ_REG : SPI_FCMD_WRITE_REG;
> +
> +       spi_flash_command_init(&cmd, instr, 0, flags);
> +       cmd.data_len = len;
> +       cmd.rx_data = response;
> +       return spi_flash_exec(flash, &cmd);
>  }
>
> -int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
> -               const void *data, size_t data_len)
> +int spi_flash_cmd_write(struct spi_flash *flash,
> +                       const struct spi_flash_command *cmd)
>  {
> -       return spi_flash_read_write(spi, cmd, cmd_len, data, NULL, data_len);
> +       return spi_flash_exec(flash, cmd);
>  }
> diff --git a/drivers/mtd/spi/sf_dataflash.c b/drivers/mtd/spi/sf_dataflash.c
> index bcddfa0755..b2166ad4e5 100644
> --- a/drivers/mtd/spi/sf_dataflash.c
> +++ b/drivers/mtd/spi/sf_dataflash.c
> @@ -73,7 +73,7 @@ struct dataflash {
>  };
>
>  /* Return the status of the DataFlash device */
> -static inline int dataflash_status(struct spi_slave *spi)
> +static inline int dataflash_status(struct spi_flash *spi_flash)
>  {
>         int ret;
>         u8 status;
> @@ -81,7 +81,7 @@ static inline int dataflash_status(struct spi_slave *spi)
>          * NOTE:  at45db321c over 25 MHz wants to write
>          * a dummy byte after the opcode...
>          */
> -       ret = spi_flash_cmd(spi, OP_READ_STATUS, &status, 1);
> +       ret = spi_flash_cmd(spi_flash, OP_READ_STATUS, &status, 1);
>         return ret ? -EIO : status;
>  }
>
> @@ -90,7 +90,7 @@ static inline int dataflash_status(struct spi_slave *spi)
>   * This usually takes 5-20 msec or so; more for sector erase.
>   * ready: return > 0
>   */
> -static int dataflash_waitready(struct spi_slave *spi)
> +static int dataflash_waitready(struct spi_flash *spi_flash)
>  {
>         int status;
>         int timeout = 2 * CONFIG_SYS_HZ;
> @@ -98,7 +98,7 @@ static int dataflash_waitready(struct spi_slave *spi)
>
>         timebase = get_timer(0);
>         do {
> -               status = dataflash_status(spi);
> +               status = dataflash_status(spi_flash);
>                 if (status < 0)
>                         status = 0;
>
> @@ -114,11 +114,11 @@ static int dataflash_waitready(struct spi_slave *spi)
>  /* Erase pages of flash */
>  static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
>  {
> +       struct spi_flash_command        cmd;
>         struct dataflash        *dataflash;
>         struct spi_flash        *spi_flash;
>         struct spi_slave        *spi;
>         unsigned                blocksize;
> -       uint8_t                 *command;
>         uint32_t                rem;
>         int                     status;
>
> @@ -128,9 +128,6 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
>
>         blocksize = spi_flash->page_size << 3;
>
> -       memset(dataflash->command, 0 , sizeof(dataflash->command));
> -       command = dataflash->command;
> -
>         debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
>
>         div_u64_rem(len, spi_flash->page_size, &rem);
> @@ -146,6 +143,8 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
>                 return status;
>         }
>
> +       spi_flash_command_init(&cmd, OP_ERASE_BLOCK, SPI_FLASH_3B_ADDR_LEN,
> +                              SPI_FCMD_ERASE);
>         while (len > 0) {
>                 unsigned int    pageaddr;
>                 int             do_block;
> @@ -157,23 +156,24 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
>                 do_block = (pageaddr & 0x7) == 0 && len >= blocksize;
>                 pageaddr = pageaddr << dataflash->page_offset;
>
> -               command[0] = do_block ? OP_ERASE_BLOCK : OP_ERASE_PAGE;
> -               command[1] = (uint8_t)(pageaddr >> 16);
> -               command[2] = (uint8_t)(pageaddr >> 8);
> -               command[3] = 0;
> +               cmd.inst = do_block ? OP_ERASE_BLOCK : OP_ERASE_PAGE;
> +               cmd.addr = pageaddr & 0x00FFFF00;
>
>                 debug("%s ERASE %s: (%x) %x %x %x [%d]\n",
>                       dev->name, do_block ? "block" : "page",
> -                     command[0], command[1], command[2], command[3],
> +                     cmd.inst,
> +                     (cmd.addr >> 16) & 0xff,
> +                     (cmd.addr >>  8) & 0xff,
> +                     (cmd.addr >>  0) & 0xff,
>                       pageaddr);
>
> -               status = spi_flash_cmd_write(spi, command, 4, NULL, 0);
> +               status = spi_flash_cmd_write(spi_flash, &cmd);
>                 if (status < 0) {
>                         debug("%s: erase send command error!\n", dev->name);
>                         return -EIO;
>                 }
>
> -               status = dataflash_waitready(spi);
> +               status = dataflash_waitready(spi_flash);
>                 if (status < 0) {
>                         debug("%s: erase waitready error!\n", dev->name);
>                         return status;
> @@ -202,23 +202,18 @@ static int spi_dataflash_erase(struct udevice *dev, u32 offset, size_t len)
>  static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
>                               void *buf)
>  {
> +       struct spi_flash_command        cmd;
>         struct dataflash        *dataflash;
>         struct spi_flash        *spi_flash;
>         struct spi_slave        *spi;
>         unsigned int            addr;
> -       uint8_t                 *command;
>         int                     status;
>
>         dataflash = dev_get_priv(dev);
>         spi_flash = dev_get_uclass_priv(dev);
>         spi = spi_flash->spi;
>
> -       memset(dataflash->command, 0 , sizeof(dataflash->command));
> -       command = dataflash->command;
> -
> -       debug("%s: erase addr=0x%x len 0x%x\n", dev->name, offset, len);
> -       debug("READ: (%x) %x %x %x\n",
> -             command[0], command[1], command[2], command[3]);
> +       debug("%s: read addr=0x%x len 0x%x\n", dev->name, offset, len);
>
>         /* Calculate flash page/byte address */
>         addr = (((unsigned)offset / spi_flash->page_size)
> @@ -236,13 +231,15 @@ static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
>          * the peak rate available.  Some chips support commands with
>          * fewer "don't care" bytes.  Both buffers stay unchanged.
>          */
> -       command[0] = OP_READ_CONTINUOUS;
> -       command[1] = (uint8_t)(addr >> 16);
> -       command[2] = (uint8_t)(addr >> 8);
> -       command[3] = (uint8_t)(addr >> 0);
> +       spi_flash_command_init(&cmd, OP_READ_CONTINUOUS, SPI_FLASH_3B_ADDR_LEN,
> +                              SPI_FCMD_READ);
> +       cmd.addr = addr;
> +       cmd.num_wait_states = 4 * 8; /* 4 "don't care" bytes */
> +       cmd.data_len = len;
> +       cmd.rx_data = buf;
>
>         /* plus 4 "don't care" bytes, command len: 4 + 4 "don't care" bytes */
> -       status = spi_flash_cmd_read(spi, command, 8, buf, len);
> +       status = spi_flash_cmd_read(spi_flash, &cmd);
>
>         spi_release_bus(spi);
>
> @@ -258,10 +255,10 @@ static int spi_dataflash_read(struct udevice *dev, u32 offset, size_t len,
>  int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>                         const void *buf)
>  {
> +       struct spi_flash_command        cmd;
>         struct dataflash        *dataflash;
>         struct spi_flash        *spi_flash;
>         struct spi_slave        *spi;
> -       uint8_t                 *command;
>         unsigned int            pageaddr, addr, to, writelen;
>         size_t                  remaining = len;
>         u_char                  *writebuf = (u_char *)buf;
> @@ -271,9 +268,6 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>         spi_flash = dev_get_uclass_priv(dev);
>         spi = spi_flash->spi;
>
> -       memset(dataflash->command, 0 , sizeof(dataflash->command));
> -       command = dataflash->command;
> -
>         debug("%s: write 0x%x..0x%x\n", dev->name, offset, (offset + len));
>
>         pageaddr = ((unsigned)offset / spi_flash->page_size);
> @@ -289,6 +283,8 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>                 return status;
>         }
>
> +       spi_flash_command_init(&cmd, OP_TRANSFER_BUF1, SPI_FLASH_3B_ADDR_LEN,
> +                              SPI_FCMD_WRITE);
>         while (remaining > 0) {
>                 debug("write @ %d:%d len=%d\n", pageaddr, to, writelen);
>
> @@ -313,22 +309,25 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>
>                 /* (1) Maybe transfer partial page to Buffer1 */
>                 if (writelen != spi_flash->page_size) {
> -                       command[0] = OP_TRANSFER_BUF1;
> -                       command[1] = (addr & 0x00FF0000) >> 16;
> -                       command[2] = (addr & 0x0000FF00) >> 8;
> -                       command[3] = 0;
> +                       cmd.inst = OP_TRANSFER_BUF1;
> +                       cmd.addr = (addr & 0x00FFFF00);
> +                       cmd.data_len = 0;
> +                       cmd.tx_data = NULL;
>
>                         debug("TRANSFER: (%x) %x %x %x\n",
> -                             command[0], command[1], command[2], command[3]);
> +                             cmd.inst,
> +                             (cmd.addr >> 16) & 0xff,
> +                             (cmd.addr >>  8) & 0xff,
> +                             (cmd.addr >>  0) & 0xff);
>
> -                       status = spi_flash_cmd_write(spi, command, 4, NULL, 0);
> +                       status = spi_flash_cmd_write(spi_flash, &cmd);
>                         if (status < 0) {
>                                 debug("%s: write(<pagesize) command error!\n",
>                                       dev->name);
>                                 return -EIO;
>                         }
>
> -                       status = dataflash_waitready(spi);
> +                       status = dataflash_waitready(spi_flash);
>                         if (status < 0) {
>                                 debug("%s: write(<pagesize) waitready error!\n",
>                                       dev->name);
> @@ -338,22 +337,24 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>
>                 /* (2) Program full page via Buffer1 */
>                 addr += to;
> -               command[0] = OP_PROGRAM_VIA_BUF1;
> -               command[1] = (addr & 0x00FF0000) >> 16;
> -               command[2] = (addr & 0x0000FF00) >> 8;
> -               command[3] = (addr & 0x000000FF);
> +               cmd.inst = OP_PROGRAM_VIA_BUF1;
> +               cmd.addr = addr;
> +               cmd.data_len = writelen;
> +               cmd.tx_data = writebuf;
>
>                 debug("PROGRAM: (%x) %x %x %x\n",
> -                     command[0], command[1], command[2], command[3]);
> +                     cmd.inst,
> +                     (cmd.addr >> 16) & 0xff,
> +                     (cmd.addr >>  8) & 0xff,
> +                     (cmd.addr >>  0) & 0xff);
>
> -               status = spi_flash_cmd_write(spi, command,
> -                                            4, writebuf, writelen);
> +               status = spi_flash_cmd_write(spi_flash, &cmd);
>                 if (status < 0) {
>                         debug("%s: write send command error!\n", dev->name);
>                         return -EIO;
>                 }
>
> -               status = dataflash_waitready(spi);
> +               status = dataflash_waitready(spi_flash);
>                 if (status < 0) {
>                         debug("%s: write waitready error!\n", dev->name);
>                         return status;
> @@ -362,16 +363,18 @@ int spi_dataflash_write(struct udevice *dev, u32 offset, size_t len,
>  #ifdef CONFIG_SPI_DATAFLASH_WRITE_VERIFY
>                 /* (3) Compare to Buffer1 */
>                 addr = pageaddr << dataflash->page_offset;
> -               command[0] = OP_COMPARE_BUF1;
> -               command[1] = (addr & 0x00FF0000) >> 16;
> -               command[2] = (addr & 0x0000FF00) >> 8;
> -               command[3] = 0;
> +               cmd.inst = OP_COMPARE_BUF1;
> +               cmd.addr = addr & 0x00FFFF00;
> +               cmd.data_len = writelen;
> +               cmd.tx_data = writebuf;
>
>                 debug("COMPARE: (%x) %x %x %x\n",
> -                     command[0], command[1], command[2], command[3]);
> +                     cmd.inst,
> +                     (cmd.addr >> 16) & 0xff,
> +                     (cmd.addr >>  8) & 0xff,
> +                     (cmd.addr >>  0) & 0xff);
>
> -               status = spi_flash_cmd_write(spi, command,
> -                                            4, writebuf, writelen);
> +               status = spi_flash_cmd_write(spi, &cmd);
>                 if (status < 0) {
>                         debug("%s: write(compare) send command error!\n",
>                               dev->name);
> @@ -496,7 +499,7 @@ static struct flash_info dataflash_data[] = {
>         { "at45db642d",  0x1f2800, 8192, 1024, 10, SUP_POW2PS | IS_POW2PS},
>  };
>
> -static struct flash_info *jedec_probe(struct spi_slave *spi)
> +static struct flash_info *jedec_probe(struct spi_flash *spi_flash)
>  {
>         int                     tmp;
>         uint8_t                 id[5];
> @@ -513,7 +516,7 @@ static struct flash_info *jedec_probe(struct spi_slave *spi)
>          * That's not an error; only rev C and newer chips handle it, and
>          * only Atmel sells these chips.
>          */
> -       tmp = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
> +       tmp = spi_flash_cmd(spi_flash, CMD_READ_ID, id, sizeof(id));
>         if (tmp < 0) {
>                 printf("dataflash: error %d reading JEDEC ID\n", tmp);
>                 return ERR_PTR(tmp);
> @@ -532,7 +535,7 @@ static struct flash_info *jedec_probe(struct spi_slave *spi)
>                         tmp++, info++) {
>                 if (info->jedec_id == jedec) {
>                         if (info->flags & SUP_POW2PS) {
> -                               status = dataflash_status(spi);
> +                               status = dataflash_status(spi_flash);
>                                 if (status < 0) {
>                                         debug("dataflash: status error %d\n",
>                                               status);
> @@ -596,7 +599,7 @@ static int spi_dataflash_probe(struct udevice *dev)
>          * Both support the security register, though with different
>          * write procedures.
>          */
> -       info = jedec_probe(spi);
> +       info = jedec_probe(spi_flash);
>         if (IS_ERR(info))
>                 goto err_jedec_probe;
>         if (info != NULL) {
> @@ -611,7 +614,7 @@ static int spi_dataflash_probe(struct udevice *dev)
>         * Older chips support only legacy commands, identifing
>         * capacity using bits in the status byte.
>         */
> -       status = dataflash_status(spi);
> +       status = dataflash_status(spi_flash);
>         if (status <= 0 || status == 0xff) {
>                 printf("dataflash: read status error %d\n", status);
>                 if (status == 0 || status == 0xff)
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 839cdbe1b0..5c551089d6 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -27,7 +27,7 @@ enum spi_nor_option_flags {
>  };
>
>  #define SPI_FLASH_3B_ADDR_LEN          3
> -#define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_3B_ADDR_LEN)
> +#define SPI_FLASH_CMD_LEN              (1 + SPI_FLASH_3B_ADDR_LEN + 16)
>  #define SPI_FLASH_16MB_BOUN            0x1000000
>
>  /* CFI Manufacture ID's */
> @@ -137,21 +137,21 @@ struct spi_flash_info {
>  extern const struct spi_flash_info spi_flash_ids[];
>
>  /* Send a single-byte command to the device and read the response */
> -int spi_flash_cmd(struct spi_slave *spi, u8 cmd, void *response, size_t len);
> +int spi_flash_cmd(struct spi_flash *flash, u8 instr, void *response, size_t len);
>
>  /*
>   * Send a multi-byte command to the device and read the response. Used
>   * for flash array reads, etc.
>   */
> -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(struct spi_flash *flash,
> +                      const struct spi_flash_command *cmd);
>
>  /*
>   * Send a multi-byte command to the device followed by (optional)
>   * data. Used for programming the flash array, etc.
>   */
> -int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
> -               const void *data, size_t data_len);
> +int spi_flash_cmd_write(struct spi_flash *flash,
> +                       const struct spi_flash_command *cmd);
>
>
>  /* Flash erase(sectors) operation, support all possible erase commands */
> @@ -169,13 +169,13 @@ int stm_is_locked(struct spi_flash *flash, u32 ofs, size_t len);
>  /* Enable writing on the SPI flash */
>  static inline int spi_flash_cmd_write_enable(struct spi_flash *flash)
>  {
> -       return spi_flash_cmd(flash->spi, CMD_WRITE_ENABLE, NULL, 0);
> +       return spi_flash_cmd(flash, CMD_WRITE_ENABLE, NULL, 0);
>  }
>
>  /* Disable writing on the SPI flash */
>  static inline int spi_flash_cmd_write_disable(struct spi_flash *flash)
>  {
> -       return spi_flash_cmd(flash->spi, CMD_WRITE_DISABLE, NULL, 0);
> +       return spi_flash_cmd(flash, CMD_WRITE_DISABLE, NULL, 0);
>  }
>
>  /*
> @@ -186,8 +186,8 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash)
>   * - spi_flash_wait_till_ready
>   * - SPI release
>   */
> -int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
> -               size_t cmd_len, const void *buf, size_t buf_len);
> +int spi_flash_write_common(struct spi_flash *flash,
> +                          const struct spi_flash_command *cmd);
>
>  /*
>   * Flash write operation, support all possible write commands.
> @@ -201,8 +201,8 @@ int spi_flash_cmd_write_ops(struct spi_flash *flash, u32 offset,
>   * Same as spi_flash_cmd_read() except it also claims/releases the SPI
>   * bus. Used as common part of the ->read() operation.
>   */
> -int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
> -               size_t cmd_len, void *data, size_t data_len);
> +int spi_flash_read_common(struct spi_flash *flash,
> +                         const struct spi_flash_command *cmd);
>
>  /* Flash read operation, support all possible read commands */
>  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> index 0034a28d5f..80541d0ab4 100644
> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -22,21 +22,15 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -static void spi_flash_addr(u32 addr, u8 *cmd)
> -{
> -       /* cmd[0] is actual command */
> -       cmd[1] = addr >> 16;
> -       cmd[2] = addr >> 8;
> -       cmd[3] = addr >> 0;
> -}
> -
>  static int read_sr(struct spi_flash *flash, u8 *rs)
>  {
> +       struct spi_flash_command cmd;
>         int ret;
> -       u8 cmd;
>
> -       cmd = CMD_READ_STATUS;
> -       ret = spi_flash_read_common(flash, &cmd, 1, rs, 1);
> +       spi_flash_command_init(&cmd, CMD_READ_STATUS, 0, SPI_FCMD_READ_REG);
> +       cmd.data_len = 1;
> +       cmd.rx_data = rs;

Better to do these command initialization in sf.c just before the
transfer initiate instead of doing it for each I/O operation.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands

[Wenyou.Yang at microchip.com]

Hi Jagan,

Thank you for your comments.

I will rework this patch set according to your advice. Thank you!


Best Regards,
Wenyou Yang

> -----Original Message-----
> From: Jagan Teki [mailto:jagannadh.teki at gmail.com]
> Sent: 2017年8月30日 21:50
> To: Wenyou Yang - A41535 <Wenyou.Yang@microchip.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Marek Vasut <marex@denx.de>;
> Cyrille Pitchen <cyrille.pitchen@atmel.com>; Jagan Teki <jagan@openedev.com>
> Subject: Re: [U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands
> 
> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.yang@microchip.com>
> wrote:
> > From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> >
> > This patch introduces 'struct spi_flash_command' and functions
> > spi_is_flash_command_supported() / spi_exec_flash_command().
> >
> 
> Answer for why this shouldn't be part of SPI area.
> 
> [snip]
> 
> drivers/spi and include/spi.h is Generic SPI area code - this can deal all
> connected slave devices like EEPROM, SPI-NOR etc drivers/mtd/spi and
> include/spi_flash.h is SPI-Flash or SPI-NOR code - this can handle only SPI
> flashes.
> 
> >
> > +bool dm_spi_is_flash_command_supported(struct udevice *dev,
> > +                                      const struct spi_flash_command
> > +*cmd) {
> > +       struct udevice *bus = dev->parent;
> > +       struct dm_spi_ops *ops = spi_get_ops(bus);
> > +
> > +       if (ops->is_flash_command_supported)
> > +               return ops->is_flash_command_supported(dev, cmd);
> > +
> > +       return false;
> > +}
> > +
> > +int dm_spi_exec_flash_command(struct udevice *dev,
> > +                             const struct spi_flash_command *cmd) {
> > +       struct udevice *bus = dev->parent;
> > +       struct dm_spi_ops *ops = spi_get_ops(bus);
> > +
> > +       if (ops->exec_flash_command)
> > +               return ops->exec_flash_command(dev, cmd);
> > +
> > +       return -EINVAL;
> > +}
> > +
> >  int spi_claim_bus(struct spi_slave *slave)  {
> >         return dm_spi_claim_bus(slave->dev); @@ -107,6 +131,18 @@ int
> > spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> >         return dm_spi_xfer(slave->dev, bitlen, dout, din, flags);  }
> >
> > +bool spi_is_flash_command_supported(struct spi_slave *slave,
> > +                                   const struct spi_flash_command
> > +*cmd) {
> > +       return dm_spi_is_flash_command_supported(slave->dev, cmd); }
> > +
> > +int spi_exec_flash_command(struct spi_slave *slave,
> > +                          const struct spi_flash_command *cmd) {
> > +       return dm_spi_exec_flash_command(slave->dev, cmd); }
> 
> Handling spi-flash stuff in spi core is not proper way of dealing, and I saw these
> functions are not-at-all needed for generic controller drivers in drivers/spi except
> the Atmel QSPI driver (in v3,8/8).
> 
> > +
> >  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >  static int spi_child_post_bind(struct udevice *dev)  { @@ -144,6
> > +180,10 @@ static int spi_post_probe(struct udevice *bus)
> >                 ops->set_mode += gd->reloc_off;
> >         if (ops->cs_info)
> >                 ops->cs_info += gd->reloc_off;
> > +       if (ops->is_flash_command_supported)
> > +               ops->is_flash_command_supported += gd->reloc_off;
> > +       if (ops->exec_flash_command)
> > +               ops->exec_flash_command += gd->reloc_off;
> >  #endif
> >
> >         return 0;
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index
> > 7d81fbd7f8..e47acdc9e4 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <common.h>
> > +#include <errno.h>
> >  #include <fdtdec.h>
> >  #include <malloc.h>
> >  #include <spi.h>
> > @@ -58,3 +59,15 @@ struct spi_slave *spi_base_setup_slave_fdt(const void
> *blob, int busnum,
> >         return spi_setup_slave(busnum, cs, max_hz, mode);  }  #endif
> > +
> > +__weak bool spi_is_flash_command_supported(struct spi_slave *slave,
> > +                                          const struct
> > +spi_flash_command *cmd) {
> > +       return false;
> > +}
> > +
> > +__weak int spi_exec_flash_command(struct spi_slave *slave,
> > +                                 const struct spi_flash_command *cmd)
> > +{
> > +       return -EINVAL;
> > +}
> > diff --git a/include/spi.h b/include/spi.h index
> > 8c4b882c54..a090266b52 100644
> > --- a/include/spi.h
> > +++ b/include/spi.h
> > @@ -10,6 +10,8 @@
> >  #ifndef _SPI_H_
> >  #define _SPI_H_
> >
> > +#include <linux/string.h> /* memset() */
> > +
> >  /* SPI mode flags */
> >  #define SPI_CPHA       BIT(0)                  /* clock phase */
> >  #define SPI_CPOL       BIT(1)                  /* clock polarity */
> > @@ -64,6 +66,116 @@ struct dm_spi_slave_platdata {  #endif /*
> > CONFIG_DM_SPI */
> >
> >  /**
> > + * enum spi_flash_protocol - SPI flash command protocol  */ #define
> > +SPI_FPROTO_INST_SHIFT  16
> > +#define SPI_FPROTO_INST_MASK   GENMASK(23, 16)
> > +#define SPI_FPROTO_INST(nbits)                                 \
> > +       ((((unsigned long)(nbits)) << SPI_FPROTO_INST_SHIFT) &  \
> > +        SPI_FPROTO_INST_MASK)
> > +
> > +#define SPI_FPROTO_ADDR_SHIFT  8
> > +#define SPI_FPROTO_ADDR_MASK   GENMASK(15, 8)
> > +#define SPI_FPROTO_ADDR(nbits)                                 \
> > +       ((((unsigned long)(nbits)) << SPI_FPROTO_ADDR_SHIFT) &  \
> > +        SPI_FPROTO_ADDR_MASK)
> > +
> > +#define SPI_FPROTO_DATA_SHIFT  0
> > +#define SPI_FPROTO_DATA_MASK   GENMASK(7, 0)
> > +#define SPI_FPROTO_DATA(nbits)                                 \
> > +       ((((unsigned long)(nbits)) << SPI_FPROTO_DATA_SHIFT) &  \
> > +        SPI_FPROTO_DATA_MASK)
> > +
> > +#define SPI_FPROTO(inst_nbits, addr_nbits, data_nbits) \
> > +       (SPI_FPROTO_INST(inst_nbits) |                  \
> > +        SPI_FPROTO_ADDR(addr_nbits) |                  \
> > +        SPI_FPROTO_DATA(data_nbits))
> > +
> > +enum spi_flash_protocol {
> > +       SPI_FPROTO_1_1_1 = SPI_FPROTO(1, 1, 1),
> > +       SPI_FPROTO_1_1_2 = SPI_FPROTO(1, 1, 2),
> > +       SPI_FPROTO_1_1_4 = SPI_FPROTO(1, 1, 4),
> > +       SPI_FPROTO_1_2_2 = SPI_FPROTO(1, 2, 2),
> > +       SPI_FPROTO_1_4_4 = SPI_FPROTO(1, 4, 4),
> > +       SPI_FPROTO_2_2_2 = SPI_FPROTO(2, 2, 2),
> > +       SPI_FPROTO_4_4_4 = SPI_FPROTO(4, 4, 4), };
> 
> these protocol also includes IO's like dual and quad and IO's which are specific for
> SPI-NOR flash and not Generic SPI protocols.
> 
> > +
> > +static inline
> > +u8 spi_flash_protocol_get_inst_nbits(enum spi_flash_protocol proto) {
> > +       return ((unsigned long)(proto & SPI_FPROTO_INST_MASK)) >>
> > +               SPI_FPROTO_INST_SHIFT; }
> > +
> > +static inline
> > +u8 spi_flash_protocol_get_addr_nbits(enum spi_flash_protocol proto) {
> > +       return ((unsigned long)(proto & SPI_FPROTO_ADDR_MASK)) >>
> > +               SPI_FPROTO_ADDR_SHIFT; }
> > +
> > +static inline
> > +u8 spi_flash_protocol_get_data_nbits(enum spi_flash_protocol proto) {
> > +       return ((unsigned long)(proto & SPI_FPROTO_DATA_MASK)) >>
> > +               SPI_FPROTO_DATA_SHIFT; }
> > +
> > +/**
> > + * struct spi_flash_command - SPI flash command structure
> > + *
> > + * @instr:             Opcode sent to the SPI slave during instr clock cycles.
> > + * @mode:              Value sent to the SPI slave during mode clock cycles.
> > + * @num_mode_cycles:   Number of mode clock cycles.
> > + * @num_wait_states:   Number of wait-state clock cycles.
> > + * @addr_len:          Number of bytes sent during address clock cycles:
> > + *                     should be 0, 3, or 4.
> > + * @addr:              Value sent to the SPI slave during address clock cycles.
> > + * @data_len:          Number of bytes to be sent during data clock cycles.
> > + * @tx_data:           Data sent to the SPI slave during data clock cycles.
> > + * @rx_data:           Data read from the SPI slave during data clock cycles.
> > + */
> > +struct spi_flash_command {
> > +       enum spi_flash_protocol proto;
> > +       u8 flags;
> > +#define SPI_FCMD_TYPE          GENMASK(2, 0)
> > +#define SPI_FCMD_READ          (0x0U << 0)
> > +#define SPI_FCMD_WRITE         (0x1U << 0)
> > +#define SPI_FCMD_ERASE         (0x2U << 0)
> > +#define SPI_FCMD_READ_REG      (0x3U << 0)
> > +#define SPI_FCMD_WRITE_REG     (0x4U << 0)
> > +
> > +       u8 inst;
> > +       u8 mode;
> > +       u8 num_mode_cycles;
> > +       u8 num_wait_states;
> > +       u8 addr_len;
> > +       u32 addr;
> > +       size_t data_len;
> > +       const void *tx_data;
> > +       void *rx_data;
> > +};
> 
> I saw from the other patches, this structure is initializing for each I/O interface
> between SPI to driver/mtd like spi_transfer, if so it shouldn't have flash attributes.
> Better to move flash contents to spi_flash structure and do the respective
> operations.
> 
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
> Maintainer Hyderabad, India.