[U-Boot] [PATCH 0/2 v3] arm: socfpga: Add Cadence QSPI support

[U-Boot] [PATCH 0/2 v3] arm: socfpga: Add Cadence QSPI support

[Stefan Roese]

Hi!

So this is my 3rd posting regarding the Candence SPI driver on SoCFPGA.

The current status is, that SPI NOR flash works now without problems.
And dcache is still enabled. The previous disabling was not needed.
And only caused problems while booting into Linux. No cache flush or
invalidate is needed. As no DMA engine is involved in this transfer.

The "solution" I used to get SPI NOR flash support working now
is implemented in patch 0004 (still marked as RFC). Here a software reset
is issued on the Micron N25Q256A SPI NOR flash. This seems to solve all
problems and reading / writing to the SPI NOR flash seems to work now
just fine. Even rebooting via pushbutton-reset or reset command works.

I think this version is now in much better state. Thats why I removed
the WIP from the subject lines.

Again, this is tested on the EBV SoCrates eval board.

Thanks,
Stefan

Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Vince Bridgers <vbridger@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

[U-Boot] [PATCH 1/4 v3] spi: Add Cadence QSPI driver used by SoCFPGA

[Stefan Roese]

This driver is copied directly from the Altera Rockerboard.org U-Boot
repository. I used this git tag: ACDS14.0.1_REL_GSRD_RC2. With minimal
changes to enable compilation in mainline U-Boot.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Vince Bridgers <vbridger@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
v3:
- Added SPDX License Indentifier
- Removed CQSPI_WRITEL / CQSPI_READL

 drivers/spi/Makefile           |   1 +
 drivers/spi/cadence_qspi.c     | 339 ++++++++++++++++
 drivers/spi/cadence_qspi.h     |  54 +++
 drivers/spi/cadence_qspi_apb.c | 900 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1294 insertions(+)
 create mode 100644 drivers/spi/cadence_qspi.c
 create mode 100644 drivers/spi/cadence_qspi.h
 create mode 100644 drivers/spi/cadence_qspi_apb.c

diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index f02c35a..4b7b566 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_ATMEL_DATAFLASH_SPI) += atmel_dataflash_spi.o
 obj-$(CONFIG_ATMEL_SPI) += atmel_spi.o
 obj-$(CONFIG_BFIN_SPI) += bfin_spi.o
 obj-$(CONFIG_BFIN_SPI6XX) += bfin_spi6xx.o
+obj-$(CONFIG_CADENCE_QSPI) += cadence_qspi.o cadence_qspi_apb.o
 obj-$(CONFIG_CF_SPI) += cf_spi.o
 obj-$(CONFIG_CF_QSPI) += cf_qspi.o
 obj-$(CONFIG_DAVINCI_SPI) += davinci_spi.o
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
new file mode 100644
index 0000000..a471beb
--- /dev/null
+++ b/drivers/spi/cadence_qspi.c
@@ -0,0 +1,339 @@
+/*
+ * Copyright (C) 2012
+ * Altera Corporation <www.altera.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <spi.h>
+#include "cadence_qspi.h"
+
+#define CQSPI_STIG_READ			0
+#define CQSPI_STIG_WRITE		1
+#define CQSPI_INDIRECT_READ		2
+#define CQSPI_INDIRECT_WRITE		3
+
+static int qspi_is_init;
+static unsigned int qspi_calibrated_hz;
+static unsigned int qspi_calibrated_cs;
+
+struct cadence_qspi_slave {
+	struct spi_slave slave;
+	unsigned int	mode;
+	unsigned int	max_hz;
+	void		*regbase;
+	void		*ahbbase;
+	size_t		cmd_len;
+	u8		cmd_buf[32];
+	size_t		data_len;
+};
+
+#define to_cadence_qspi_slave(s)		\
+		container_of(s, struct cadence_qspi_slave, slave)
+
+void spi_set_speed(struct spi_slave *slave, uint hz)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	void *base = cadence_qspi->regbase;
+
+	cadence_qspi_apb_config_baudrate_div(base, CONFIG_CQSPI_REF_CLK, hz);
+
+	/* Reconfigure delay timing if speed is changed. */
+	cadence_qspi_apb_delay(base, CONFIG_CQSPI_REF_CLK, hz,
+		CONFIG_CQSPI_TSHSL_NS, CONFIG_CQSPI_TSD2D_NS,
+		CONFIG_CQSPI_TCHSH_NS, CONFIG_CQSPI_TSLCH_NS);
+	return;
+}
+
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
+		unsigned int max_hz, unsigned int mode)
+{
+	struct cadence_qspi_slave *cadence_qspi;
+
+	debug("%s: bus %d cs %d max_hz %dMHz mode %d\n", __func__,
+		bus, cs, max_hz/1000000, mode);
+
+	if (!spi_cs_is_valid(bus, cs))
+		return NULL;
+
+	cadence_qspi = calloc(sizeof(struct cadence_qspi_slave), 1);
+	if (!cadence_qspi) {
+		printf("QSPI: Can't allocate struct cadence_qspi_slave. "
+			"Bus %d cs %d\n", bus, cs);
+		return NULL;
+	}
+
+	cadence_qspi->slave.bus = bus;
+	cadence_qspi->slave.cs = cs;
+	cadence_qspi->mode = mode;
+	cadence_qspi->max_hz = max_hz;
+	cadence_qspi->regbase = (void *)QSPI_BASE;
+	cadence_qspi->ahbbase = (void *)QSPI_AHB_BASE;
+
+	if (!qspi_is_init)
+		spi_init();
+
+	return &cadence_qspi->slave;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	free(cadence_qspi);
+	return;
+}
+
+void spi_init(void)
+{
+	cadence_qspi_apb_controller_init((void *)QSPI_BASE);
+	qspi_is_init = 1;
+	return;
+}
+
+/* calibration sequence to determine the read data capture delay register */
+int spi_calibration(struct spi_slave *slave)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	void *base = cadence_qspi->regbase;
+	u8 opcode_rdid = 0x9F;
+	unsigned int idcode = 0, temp = 0;
+	int err = 0, i, range_lo = -1, range_hi = -1;
+
+	/* start with slowest clock (1 MHz) */
+	spi_set_speed(slave, 1000000);
+
+	/* configure the read data capture delay register to 0 */
+	cadence_qspi_apb_readdata_capture(base, 1, 0);
+
+	/* Enable QSPI */
+	cadence_qspi_apb_controller_enable(base);
+
+	/* read the ID which will be our golden value */
+	err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid,
+		3, (u8 *)&idcode);
+	if (err) {
+		puts("SF: Calibration failed (read)\n");
+		return err;
+	}
+
+	/* use back the intended clock and find low range */
+	spi_set_speed(slave, cadence_qspi->max_hz);
+	for (i = 0; i < CQSPI_READ_CAPTURE_MAX_DELAY; i++) {
+		/* Disable QSPI */
+		cadence_qspi_apb_controller_disable(base);
+
+		/* reconfigure the read data capture delay register */
+		cadence_qspi_apb_readdata_capture(base, 1, i);
+
+		/* Enable back QSPI */
+		cadence_qspi_apb_controller_enable(base);
+
+		/* issue a RDID to get the ID value */
+		err = cadence_qspi_apb_command_read(base, 1, &opcode_rdid,
+			3, (u8 *)&temp);
+		if (err) {
+			puts("SF: Calibration failed (read)\n");
+			return err;
+		}
+
+		/* search for range lo */
+		if (range_lo == -1 && temp == idcode) {
+			range_lo = i;
+			continue;
+		}
+
+		/* search for range hi */
+		if (range_lo != -1 && temp != idcode) {
+			range_hi = i - 1;
+			break;
+		}
+		range_hi = i;
+	}
+
+	if (range_lo == -1) {
+		puts("SF: Calibration failed (low range)\n");
+		return err;
+	}
+
+	/* Disable QSPI for subsequent initialization */
+	cadence_qspi_apb_controller_disable(base);
+
+	/* configure the final value for read data capture delay register */
+	cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
+	printf("SF: Read data capture delay calibrated to %i (%i - %i)\n",
+		(range_hi + range_lo) / 2, range_lo, range_hi);
+
+	/* just to ensure we do once only when speed or chip select change */
+	qspi_calibrated_hz = cadence_qspi->max_hz;
+	qspi_calibrated_cs = slave->cs;
+	return 0;
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	unsigned int clk_pol = (cadence_qspi->mode & SPI_CPOL) ? 1 : 0;
+	unsigned int clk_pha = (cadence_qspi->mode & SPI_CPHA) ? 1 : 0;
+	void *base = cadence_qspi->regbase;
+	int err = 0;
+
+	debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
+
+	/* Disable QSPI */
+	cadence_qspi_apb_controller_disable(base);
+
+	/* Set Chip select */
+	cadence_qspi_apb_chipselect(base, slave->cs, CONFIG_CQSPI_DECODER);
+
+	/* Set SPI mode */
+	cadence_qspi_apb_set_clk_mode(base, clk_pol, clk_pha);
+
+	/* Set clock speed */
+	spi_set_speed(slave, cadence_qspi->max_hz);
+
+	/* calibration required for different SCLK speed or chip select */
+	if (qspi_calibrated_hz != cadence_qspi->max_hz ||
+		qspi_calibrated_cs != slave->cs) {
+		err = spi_calibration(slave);
+		if (err)
+			return err;
+	}
+
+	/* Enable QSPI */
+	cadence_qspi_apb_controller_enable(base);
+
+	return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+	return;
+}
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
+		void *data_in, unsigned long flags)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	void *base = cadence_qspi->regbase;
+	void *ahbbase = cadence_qspi->ahbbase;
+	u8 *cmd_buf = cadence_qspi->cmd_buf;
+	size_t data_bytes;
+	int err = 0;
+	u32 mode = CQSPI_STIG_WRITE;
+
+	if (flags & SPI_XFER_BEGIN) {
+		/* copy command to local buffer */
+		cadence_qspi->cmd_len = bitlen / 8;
+		memcpy(cmd_buf, data_out, cadence_qspi->cmd_len);
+	}
+
+	if (flags == (SPI_XFER_BEGIN | SPI_XFER_END)) {
+		/* if start and end bit are set, the data bytes is 0. */
+		data_bytes = 0;
+	} else {
+		data_bytes = bitlen / 8;
+	}
+
+	if ((flags & SPI_XFER_END) || (flags == 0)) {
+
+		if (cadence_qspi->cmd_len == 0) {
+			printf("QSPI: Error, command is empty.\n");
+			return -1;
+		}
+
+		if (data_in && data_bytes) {
+			/* read */
+			/* Use STIG if no address. */
+			if (!CQSPI_IS_ADDR(cadence_qspi->cmd_len))
+				mode = CQSPI_STIG_READ;
+			else
+				mode = CQSPI_INDIRECT_READ;
+		} else if (data_out && !(flags & SPI_XFER_BEGIN)) {
+			/* write */
+			if (!CQSPI_IS_ADDR(cadence_qspi->cmd_len))
+				mode = CQSPI_STIG_WRITE;
+			else
+				mode = CQSPI_INDIRECT_WRITE;
+		}
+
+		switch (mode) {
+		case CQSPI_STIG_READ:
+			err = cadence_qspi_apb_command_read(
+				base, cadence_qspi->cmd_len, cmd_buf,
+				data_bytes, data_in);
+
+		break;
+		case CQSPI_STIG_WRITE:
+			err = cadence_qspi_apb_command_write(base,
+				cadence_qspi->cmd_len, cmd_buf,
+				data_bytes, data_out);
+		break;
+		case CQSPI_INDIRECT_READ:
+			err = cadence_qspi_apb_indirect_read_setup(
+				base, QSPI_AHB_BASE,
+				cadence_qspi->cmd_len, cmd_buf);
+			if (!err) {
+				err = cadence_qspi_apb_indirect_read_execute
+				(base, ahbbase, data_bytes, data_in);
+			}
+		break;
+		case CQSPI_INDIRECT_WRITE:
+			err = cadence_qspi_apb_indirect_write_setup
+				(base, QSPI_AHB_BASE,
+				cadence_qspi->cmd_len, cmd_buf);
+			if (!err) {
+				err = cadence_qspi_apb_indirect_write_execute
+				(base, ahbbase, data_bytes, data_out);
+			}
+		break;
+		default:
+			err = -1;
+			break;
+		}
+
+		if (flags & SPI_XFER_END) {
+			/* clear command buffer */
+			memset(cmd_buf, 0, sizeof(cadence_qspi->cmd_buf));
+			cadence_qspi->cmd_len = 0;
+		}
+	}
+	return err;
+}
+
+int spi_cs_is_valid(unsigned int bus, unsigned int cs)
+{
+#if (CONFIG_CQSPI_DECODER == 1)
+	if (((cs >= 0) && (cs < CQSPI_DECODER_MAX_CS)) && ((bus >= 0) &&
+		(bus < CQSPI_DECODER_MAX_CS))) {
+		return 1;
+	}
+#else
+	if (((cs >= 0) && (cs < CQSPI_NO_DECODER_MAX_CS)) &&
+		((bus >= 0) && (bus < CQSPI_NO_DECODER_MAX_CS))) {
+		return 1;
+	}
+#endif
+	printf("QSPI: Invalid bus or cs. Bus %d cs %d\n", bus, cs);
+	return 0;
+}
+
+void spi_cs_activate(struct spi_slave *slave)
+{
+	return;
+}
+
+void spi_cs_deactivate(struct spi_slave *slave)
+{
+	return;
+}
+
+void spi_enter_xip(struct spi_slave *slave, char xip_dummy)
+{
+	struct cadence_qspi_slave *cadence_qspi = to_cadence_qspi_slave(slave);
+	void *base = cadence_qspi->regbase;
+	/* Enter XiP */
+	cadence_qspi_apb_enter_xip(base, xip_dummy);
+	return;
+}
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
new file mode 100644
index 0000000..467d9c9
--- /dev/null
+++ b/drivers/spi/cadence_qspi.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2012
+ * Altera Corporation <www.altera.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __CADENCE_QSPI_H__
+#define __CADENCE_QSPI_H__
+
+#define QSPI_BASE			(CONFIG_CQSPI_BASE)
+#define QSPI_AHB_BASE			(CONFIG_CQSPI_AHB_BASE)
+#define CQSPI_IS_ADDR(cmd_len)		(cmd_len > 1 ? 1 : 0)
+
+#define CQSPI_NO_DECODER_MAX_CS		(4)
+#define CQSPI_DECODER_MAX_CS		(16)
+#define CQSPI_READ_CAPTURE_MAX_DELAY	(16)
+
+/* Functions call declaration */
+void cadence_qspi_apb_controller_init(void *reg_base_addr);
+void cadence_qspi_apb_controller_enable(void *reg_base_addr);
+void cadence_qspi_apb_controller_disable(void *reg_base_addr);
+
+int cadence_qspi_apb_command_read(void *reg_base_addr,
+	unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen, u8* rxbuf);
+int cadence_qspi_apb_command_write(void *reg_base_addr,
+	unsigned int cmdlen, const u8 *cmdbuf,
+	unsigned int txlen,  const u8 *txbuf);
+
+int cadence_qspi_apb_indirect_read_setup(void *reg_base,
+	unsigned int ahb_phy_addr, unsigned int cmdlen, const u8 *cmdbuf);
+int cadence_qspi_apb_indirect_read_execute(void *reg_base_addr,
+	void *ahb_base_addr, unsigned int rxlen, u8 *rxbuf);
+
+int cadence_qspi_apb_indirect_write_setup(void *reg_base,
+	unsigned int ahb_phy_addr, unsigned int cmdlen, const u8 *cmdbuf);
+int cadence_qspi_apb_indirect_write_execute(void *reg_base_addr,
+	void *ahb_base_addr, unsigned int txlen, const u8 *txbuf);
+
+void cadence_qspi_apb_chipselect(void *reg_base,
+	unsigned int chip_select, unsigned int decoder_enable);
+void cadence_qspi_apb_set_clk_mode(void *reg_base_addr,
+	unsigned int clk_pol, unsigned int clk_pha);
+void cadence_qspi_apb_config_baudrate_div(void *reg_base,
+	unsigned int ref_clk_hz, unsigned int sclk_hz);
+void cadence_qspi_apb_delay(void *reg_base,
+	unsigned int ref_clk, unsigned int sclk_hz,
+	unsigned int tshsl_ns, unsigned int tsd2d_ns,
+	unsigned int tchsh_ns, unsigned int tslch_ns);
+void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy);
+void cadence_qspi_apb_readdata_capture(void *reg_base,
+	unsigned int bypass, unsigned int delay);
+
+#endif /* __CADENCE_QSPI_H__ */
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
new file mode 100644
index 0000000..ca1c526
--- /dev/null
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -0,0 +1,900 @@
+/*
+ * Copyright (C) 2012 Altera Corporation <www.altera.com>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *  - Neither the name of the Altera Corporation nor the
+ *    names of its contributors may be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL ALTERA CORPORATION BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+#include "cadence_qspi.h"
+
+#define CQSPI_REG_POLL_US			(1) /* 1us */
+#define CQSPI_REG_RETRY				(10000)
+#define CQSPI_POLL_IDLE_RETRY			(3)
+
+#define CQSPI_FIFO_WIDTH			(4)
+
+/* Controller sram size in word */
+#define CQSPI_REG_SRAM_SIZE_WORD		(128)
+#define CQSPI_REG_SRAM_RESV_WORDS		(2)
+#define CQSPI_REG_SRAM_PARTITION_WR		(1)
+#define CQSPI_REG_SRAM_PARTITION_RD		\
+	(CQSPI_REG_SRAM_SIZE_WORD - CQSPI_REG_SRAM_RESV_WORDS)
+#define CQSPI_REG_SRAM_THRESHOLD_WORDS		(50)
+
+/* Transfer mode */
+#define CQSPI_INST_TYPE_SINGLE			(0)
+#define CQSPI_INST_TYPE_DUAL			(1)
+#define CQSPI_INST_TYPE_QUAD			(2)
+
+#define CQSPI_STIG_DATA_LEN_MAX			(8)
+#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
+
+#define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
+#define CQSPI_DUMMY_BYTES_MAX			(4)
+
+
+#define CQSPI_REG_SRAM_FILL_THRESHOLD	\
+	((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
+/****************************************************************************
+ * Controller's configuration and status register (offset from QSPI_BASE)
+ ****************************************************************************/
+#define	CQSPI_REG_CONFIG			0x00
+#define	CQSPI_REG_CONFIG_CLK_POL_LSB		1
+#define	CQSPI_REG_CONFIG_CLK_PHA_LSB		2
+#define	CQSPI_REG_CONFIG_ENABLE_MASK		(1 << 0)
+#define	CQSPI_REG_CONFIG_DIRECT_MASK		(1 << 7)
+#define	CQSPI_REG_CONFIG_DECODE_MASK		(1 << 9)
+#define	CQSPI_REG_CONFIG_XIP_IMM_MASK		(1 << 18)
+#define	CQSPI_REG_CONFIG_CHIPSELECT_LSB		10
+#define	CQSPI_REG_CONFIG_BAUD_LSB		19
+#define	CQSPI_REG_CONFIG_IDLE_LSB		31
+#define	CQSPI_REG_CONFIG_CHIPSELECT_MASK	0xF
+#define	CQSPI_REG_CONFIG_BAUD_MASK		0xF
+
+#define	CQSPI_REG_RD_INSTR			0x04
+#define	CQSPI_REG_RD_INSTR_OPCODE_LSB		0
+#define	CQSPI_REG_RD_INSTR_TYPE_INSTR_LSB	8
+#define	CQSPI_REG_RD_INSTR_TYPE_ADDR_LSB	12
+#define	CQSPI_REG_RD_INSTR_TYPE_DATA_LSB	16
+#define	CQSPI_REG_RD_INSTR_MODE_EN_LSB		20
+#define	CQSPI_REG_RD_INSTR_DUMMY_LSB		24
+#define	CQSPI_REG_RD_INSTR_TYPE_INSTR_MASK	0x3
+#define	CQSPI_REG_RD_INSTR_TYPE_ADDR_MASK	0x3
+#define	CQSPI_REG_RD_INSTR_TYPE_DATA_MASK	0x3
+#define	CQSPI_REG_RD_INSTR_DUMMY_MASK		0x1F
+
+#define	CQSPI_REG_WR_INSTR			0x08
+#define	CQSPI_REG_WR_INSTR_OPCODE_LSB		0
+
+#define	CQSPI_REG_DELAY				0x0C
+#define	CQSPI_REG_DELAY_TSLCH_LSB		0
+#define	CQSPI_REG_DELAY_TCHSH_LSB		8
+#define	CQSPI_REG_DELAY_TSD2D_LSB		16
+#define	CQSPI_REG_DELAY_TSHSL_LSB		24
+#define	CQSPI_REG_DELAY_TSLCH_MASK		0xFF
+#define	CQSPI_REG_DELAY_TCHSH_MASK		0xFF
+#define	CQSPI_REG_DELAY_TSD2D_MASK		0xFF
+#define	CQSPI_REG_DELAY_TSHSL_MASK		0xFF
+
+#define	CQSPI_READLCAPTURE			0x10
+#define	CQSPI_READLCAPTURE_BYPASS_LSB		0
+#define	CQSPI_READLCAPTURE_DELAY_LSB		1
+#define	CQSPI_READLCAPTURE_DELAY_MASK		0xF
+
+#define	CQSPI_REG_SIZE				0x14
+#define	CQSPI_REG_SIZE_ADDRESS_LSB		0
+#define	CQSPI_REG_SIZE_PAGE_LSB			4
+#define	CQSPI_REG_SIZE_BLOCK_LSB		16
+#define	CQSPI_REG_SIZE_ADDRESS_MASK		0xF
+#define	CQSPI_REG_SIZE_PAGE_MASK		0xFFF
+#define	CQSPI_REG_SIZE_BLOCK_MASK		0x3F
+
+#define	CQSPI_REG_SRAMPARTITION			0x18
+#define	CQSPI_REG_INDIRECTTRIGGER		0x1C
+
+#define	CQSPI_REG_REMAP				0x24
+#define	CQSPI_REG_MODE_BIT			0x28
+
+#define	CQSPI_REG_SDRAMLEVEL			0x2C
+#define	CQSPI_REG_SDRAMLEVEL_RD_LSB		0
+#define	CQSPI_REG_SDRAMLEVEL_WR_LSB		16
+#define	CQSPI_REG_SDRAMLEVEL_RD_MASK		0xFFFF
+#define	CQSPI_REG_SDRAMLEVEL_WR_MASK		0xFFFF
+
+#define	CQSPI_REG_IRQSTATUS			0x40
+#define	CQSPI_REG_IRQMASK			0x44
+
+#define	CQSPI_REG_INDIRECTRD			0x60
+#define	CQSPI_REG_INDIRECTRD_START_MASK		(1 << 0)
+#define	CQSPI_REG_INDIRECTRD_CANCEL_MASK	(1 << 1)
+#define	CQSPI_REG_INDIRECTRD_INPROGRESS_MASK	(1 << 2)
+#define	CQSPI_REG_INDIRECTRD_DONE_MASK		(1 << 5)
+
+#define	CQSPI_REG_INDIRECTRDWATERMARK		0x64
+#define	CQSPI_REG_INDIRECTRDSTARTADDR		0x68
+#define	CQSPI_REG_INDIRECTRDBYTES		0x6C
+
+#define	CQSPI_REG_CMDCTRL			0x90
+#define	CQSPI_REG_CMDCTRL_EXECUTE_MASK		(1 << 0)
+#define	CQSPI_REG_CMDCTRL_INPROGRESS_MASK	(1 << 1)
+#define	CQSPI_REG_CMDCTRL_DUMMY_LSB		7
+#define	CQSPI_REG_CMDCTRL_WR_BYTES_LSB		12
+#define	CQSPI_REG_CMDCTRL_WR_EN_LSB		15
+#define	CQSPI_REG_CMDCTRL_ADD_BYTES_LSB		16
+#define	CQSPI_REG_CMDCTRL_ADDR_EN_LSB		19
+#define	CQSPI_REG_CMDCTRL_RD_BYTES_LSB		20
+#define	CQSPI_REG_CMDCTRL_RD_EN_LSB		23
+#define	CQSPI_REG_CMDCTRL_OPCODE_LSB		24
+#define	CQSPI_REG_CMDCTRL_DUMMY_MASK		0x1F
+#define	CQSPI_REG_CMDCTRL_WR_BYTES_MASK		0x7
+#define	CQSPI_REG_CMDCTRL_ADD_BYTES_MASK	0x3
+#define	CQSPI_REG_CMDCTRL_RD_BYTES_MASK		0x7
+#define	CQSPI_REG_CMDCTRL_OPCODE_MASK		0xFF
+
+#define	CQSPI_REG_INDIRECTWR			0x70
+#define	CQSPI_REG_INDIRECTWR_START_MASK		(1 << 0)
+#define	CQSPI_REG_INDIRECTWR_CANCEL_MASK	(1 << 1)
+#define	CQSPI_REG_INDIRECTWR_INPROGRESS_MASK	(1 << 2)
+#define	CQSPI_REG_INDIRECTWR_DONE_MASK		(1 << 5)
+
+#define	CQSPI_REG_INDIRECTWRWATERMARK		0x74
+#define	CQSPI_REG_INDIRECTWRSTARTADDR		0x78
+#define	CQSPI_REG_INDIRECTWRBYTES		0x7C
+
+#define	CQSPI_REG_CMDADDRESS			0x94
+#define	CQSPI_REG_CMDREADDATALOWER		0xA0
+#define	CQSPI_REG_CMDREADDATAUPPER		0xA4
+#define	CQSPI_REG_CMDWRITEDATALOWER		0xA8
+#define	CQSPI_REG_CMDWRITEDATAUPPER		0xAC
+
+#define CQSPI_REG_IS_IDLE(base)					\
+	((readl(base + CQSPI_REG_CONFIG) >>		\
+		CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
+
+#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)		\
+	((((tdelay_ns) - (tsclk_ns)) / (tref_ns)))
+
+#define CQSPI_GET_RD_SRAM_LEVEL(reg_basse)			\
+	(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>	\
+	CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
+
+#define CQSPI_GET_WR_SRAM_LEVEL(reg_basse)			\
+	(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>	\
+	CQSPI_REG_SDRAMLEVEL_WR_LSB) & CQSPI_REG_SDRAMLEVEL_WR_MASK)
+
+static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf,
+	unsigned int addr_width)
+{
+	unsigned int addr;
+
+	addr = (addr_buf[0] << 16) | (addr_buf[1] << 8) | addr_buf[2];
+
+	if (addr_width == 4)
+		addr = (addr << 8) | addr_buf[3];
+
+	return addr;
+}
+
+static void cadence_qspi_apb_read_fifo_data(void *dest,
+	const void *src_ahb_addr, unsigned int bytes)
+{
+	unsigned int temp;
+	int remaining = bytes;
+	unsigned int *dest_ptr = (unsigned int *)dest;
+	unsigned int *src_ptr = (unsigned int *)src_ahb_addr;
+
+	while (remaining > 0) {
+		if (remaining >= CQSPI_FIFO_WIDTH) {
+			*dest_ptr = readl(src_ptr);
+			remaining -= CQSPI_FIFO_WIDTH;
+		} else {
+			/* dangling bytes */
+			temp = readl(src_ptr);
+			memcpy(dest_ptr, &temp, remaining);
+			break;
+		}
+		dest_ptr++;
+	}
+
+	return;
+}
+
+static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
+	const void *src, unsigned int bytes)
+{
+	unsigned int temp;
+	int remaining = bytes;
+	unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr;
+	unsigned int *src_ptr = (unsigned int *)src;
+
+	while (remaining > 0) {
+		if (remaining >= CQSPI_FIFO_WIDTH) {
+			writel(*src_ptr, dest_ptr);
+			remaining -= sizeof(unsigned int);
+		} else {
+			/* dangling bytes */
+			memcpy(&temp, src_ptr, remaining);
+			writel(temp, dest_ptr);
+			break;
+		}
+		src_ptr++;
+	}
+
+	return;
+}
+
+/* Read from SRAM FIFO with polling SRAM fill level. */
+static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr,
+			const void *src_addr,  unsigned int num_bytes)
+{
+	unsigned int remaining = num_bytes;
+	unsigned int retry;
+	unsigned int sram_level = 0;
+	unsigned char *dest = (unsigned char *)dest_addr;
+
+	while (remaining > 0) {
+		retry = CQSPI_REG_RETRY;
+		while (retry--) {
+			sram_level = CQSPI_GET_RD_SRAM_LEVEL(reg_base);
+			if (sram_level)
+				break;
+			udelay(1);
+		}
+
+		if (!retry) {
+			printf("QSPI: No receive data after polling for %d "
+				"times\n", CQSPI_REG_RETRY);
+			return -1;
+		}
+
+		sram_level *= CQSPI_FIFO_WIDTH;
+		sram_level = sram_level > remaining ? remaining : sram_level;
+
+		/* Read data from FIFO. */
+		cadence_qspi_apb_read_fifo_data(dest, src_addr, sram_level);
+		dest += sram_level;
+		remaining -= sram_level;
+		udelay(1);
+	}
+	return 0;
+}
+
+
+/* Write to SRAM FIFO with polling SRAM fill level. */
+static int qpsi_write_sram_fifo_push(const void *reg_base, void *dest_addr,
+				const void *src_addr, unsigned int num_bytes)
+{
+	unsigned int retry = CQSPI_REG_RETRY;
+	unsigned int sram_level;
+	unsigned int wr_bytes;
+	unsigned char *src = (unsigned char *)src_addr;
+	int remaining = num_bytes;
+	unsigned int page_size = CONFIG_CQSPI_PAGE_SIZE;
+	unsigned int sram_threshold_words = CQSPI_REG_SRAM_THRESHOLD_WORDS;
+
+	while (remaining > 0) {
+		retry = CQSPI_REG_RETRY;
+		while (retry--) {
+			sram_level = CQSPI_GET_WR_SRAM_LEVEL(reg_base);
+			if (sram_level <= sram_threshold_words)
+				break;
+		}
+		if (!retry) {
+			printf("QSPI: SRAM fill level (0x%08x) "
+				"not hit lower expected level (0x%08x)",
+				sram_level, sram_threshold_words);
+			return -1;
+		}
+		/* Write a page or remaining bytes. */
+		wr_bytes = (remaining > page_size) ?
+					page_size : remaining;
+
+		cadence_qspi_apb_write_fifo_data(dest_addr, src, wr_bytes);
+		src += wr_bytes;
+		remaining -= wr_bytes;
+	}
+
+	return 0;
+}
+
+void cadence_qspi_apb_controller_enable(void *reg_base)
+{
+	unsigned int reg;
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+	return;
+}
+
+void cadence_qspi_apb_controller_disable(void *reg_base)
+{
+	unsigned int reg;
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+	return;
+}
+
+/* Return 1 if idle, otherwise return 0 (busy). */
+static unsigned int cadence_qspi_wait_idle(void *reg_base)
+{
+	unsigned int start, count = 0;
+	/* timeout in unit of ms */
+	unsigned int timeout = 5000;
+
+	start = get_timer(0);
+	for ( ; get_timer(start) < timeout ; ) {
+		if (CQSPI_REG_IS_IDLE(reg_base))
+			count++;
+		else
+			count = 0;
+		/*
+		 * Ensure the QSPI controller is in true idle state after
+		 * reading back the same idle status consecutively
+		 */
+		if (count >= CQSPI_POLL_IDLE_RETRY)
+			return 1;
+	}
+
+	/* Timeout, still in busy mode. */
+	printf("QSPI: QSPI is still busy after poll for %d times.\n",
+		CQSPI_REG_RETRY);
+	return 0;
+}
+
+void cadence_qspi_apb_readdata_capture(void *reg_base,
+				unsigned int bypass, unsigned int delay)
+{
+	unsigned int reg;
+	cadence_qspi_apb_controller_disable(reg_base);
+
+	reg = readl(reg_base + CQSPI_READLCAPTURE);
+
+	if (bypass)
+		reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+	else
+		reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+
+	reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
+		<< CQSPI_READLCAPTURE_DELAY_LSB);
+
+	reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
+		<< CQSPI_READLCAPTURE_DELAY_LSB);
+
+	writel(reg, reg_base + CQSPI_READLCAPTURE);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+void cadence_qspi_apb_config_baudrate_div(void *reg_base,
+	unsigned int ref_clk_hz, unsigned int sclk_hz)
+{
+	unsigned int reg;
+	unsigned int div;
+
+	cadence_qspi_apb_controller_disable(reg_base);
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
+
+	div = ref_clk_hz / sclk_hz;
+
+	if (div > 32)
+		div = 32;
+
+	/* Check if even number. */
+	if ((div & 1))
+		div = (div / 2);
+	else {
+		if (ref_clk_hz % sclk_hz)
+			/* ensure generated SCLK doesn't exceed user
+			specified sclk_hz */
+			div = (div / 2);
+		else
+			div = (div / 2) - 1;
+	}
+
+	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
+		ref_clk_hz, sclk_hz, div);
+
+	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB;
+	reg |= div;
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+void cadence_qspi_apb_set_clk_mode(void *reg_base,
+	unsigned int clk_pol, unsigned int clk_pha)
+{
+	unsigned int reg;
+
+	cadence_qspi_apb_controller_disable(reg_base);
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	reg &= ~(1 <<
+		(CQSPI_REG_CONFIG_CLK_POL_LSB | CQSPI_REG_CONFIG_CLK_PHA_LSB));
+
+	reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
+	reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+void cadence_qspi_apb_chipselect(void *reg_base,
+	unsigned int chip_select, unsigned int decoder_enable)
+{
+	unsigned int reg;
+
+	cadence_qspi_apb_controller_disable(reg_base);
+
+	debug("%s : chipselect %d decode %d\n", __func__, chip_select,
+		decoder_enable);
+
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	/* docoder */
+	if (decoder_enable)
+		reg |= CQSPI_REG_CONFIG_DECODE_MASK;
+	else {
+		reg &= ~CQSPI_REG_CONFIG_DECODE_MASK;
+		/* Convert CS if without decoder.
+		 * CS0 to 4b'1110
+		 * CS1 to 4b'1101
+		 * CS2 to 4b'1011
+		 * CS3 to 4b'0111
+		 */
+		chip_select = 0xF & ~(1 << chip_select);
+	}
+
+	reg &= ~(CQSPI_REG_CONFIG_CHIPSELECT_MASK
+			<< CQSPI_REG_CONFIG_CHIPSELECT_LSB);
+	reg |= (chip_select & CQSPI_REG_CONFIG_CHIPSELECT_MASK)
+			<< CQSPI_REG_CONFIG_CHIPSELECT_LSB;
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+void cadence_qspi_apb_delay(void *reg_base,
+	unsigned int ref_clk, unsigned int sclk_hz,
+	unsigned int tshsl_ns, unsigned int tsd2d_ns,
+	unsigned int tchsh_ns, unsigned int tslch_ns)
+{
+	unsigned int ref_clk_ns;
+	unsigned int sclk_ns;
+	unsigned int tshsl, tchsh, tslch, tsd2d;
+	unsigned int reg;
+
+	cadence_qspi_apb_controller_disable(reg_base);
+
+	/* Convert to ns. */
+	ref_clk_ns = (1000000000) / ref_clk;
+
+	/* Convert to ns. */
+	sclk_ns = (1000000000) / sclk_hz;
+
+	/* Plus 1 to round up 1 clock cycle. */
+	tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
+	tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
+	tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
+	tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
+
+	reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
+			<< CQSPI_REG_DELAY_TSHSL_LSB);
+	reg |= ((tchsh & CQSPI_REG_DELAY_TCHSH_MASK)
+			<< CQSPI_REG_DELAY_TCHSH_LSB);
+	reg |= ((tslch & CQSPI_REG_DELAY_TSLCH_MASK)
+			<< CQSPI_REG_DELAY_TSLCH_LSB);
+	reg |= ((tsd2d & CQSPI_REG_DELAY_TSD2D_MASK)
+			<< CQSPI_REG_DELAY_TSD2D_LSB);
+	writel(reg, reg_base + CQSPI_REG_DELAY);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+void cadence_qspi_apb_controller_init(void *reg_base)
+{
+	unsigned reg;
+
+	cadence_qspi_apb_controller_disable(reg_base);
+
+	/* Configure the device size and address bytes */
+	reg = readl(reg_base + CQSPI_REG_SIZE);
+	/* Clear the previous value */
+	reg &= ~(CQSPI_REG_SIZE_PAGE_MASK << CQSPI_REG_SIZE_PAGE_LSB);
+	reg &= ~(CQSPI_REG_SIZE_BLOCK_MASK << CQSPI_REG_SIZE_BLOCK_LSB);
+	reg |= (CONFIG_CQSPI_PAGE_SIZE << CQSPI_REG_SIZE_PAGE_LSB);
+	reg |= (CONFIG_CQSPI_BLOCK_SIZE << CQSPI_REG_SIZE_BLOCK_LSB);
+	writel(reg, reg_base + CQSPI_REG_SIZE);
+
+	/* Configure the remap address register, no remap */
+	writel(0, reg_base + CQSPI_REG_REMAP);
+
+	/* Disable all interrupts */
+	writel(0, reg_base + CQSPI_REG_IRQMASK);
+
+	cadence_qspi_apb_controller_enable(reg_base);
+	return;
+}
+
+static int cadence_qspi_apb_exec_flash_cmd(void *reg_base,
+	unsigned int reg)
+{
+	unsigned int retry = CQSPI_REG_RETRY;
+
+	/* Write the CMDCTRL without start execution. */
+	writel(reg, reg_base + CQSPI_REG_CMDCTRL);
+	/* Start execute */
+	reg |= CQSPI_REG_CMDCTRL_EXECUTE_MASK;
+	writel(reg, reg_base + CQSPI_REG_CMDCTRL);
+
+	while (retry--) {
+		reg = readl(reg_base + CQSPI_REG_CMDCTRL);
+		if ((reg & CQSPI_REG_CMDCTRL_INPROGRESS_MASK) == 0)
+			break;
+		udelay(1);
+	}
+
+	if (!retry) {
+		printf("QSPI: flash command execution timeout\n");
+		return -EIO;
+	}
+
+	/* Polling QSPI idle status. */
+	if (!cadence_qspi_wait_idle(reg_base))
+		return -EIO;
+
+	return 0;
+}
+
+/* For command RDID, RDSR. */
+int cadence_qspi_apb_command_read(void *reg_base,
+	unsigned int cmdlen, const u8 *cmdbuf, unsigned int rxlen,
+	u8 *rxbuf)
+{
+	unsigned int reg;
+	unsigned int read_len;
+	int status;
+
+	if (!cmdlen || rxlen > CQSPI_STIG_DATA_LEN_MAX || rxbuf == NULL) {
+		printf("QSPI: Invalid input arguments cmdlen %d "
+			"rxlen %d\n", cmdlen, rxlen);
+		return -EINVAL;
+	}
+
+	reg = cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB;
+
+	reg |= (0x1 << CQSPI_REG_CMDCTRL_RD_EN_LSB);
+
+	/* 0 means 1 byte. */
+	reg |= (((rxlen - 1) & CQSPI_REG_CMDCTRL_RD_BYTES_MASK)
+		<< CQSPI_REG_CMDCTRL_RD_BYTES_LSB);
+	status = cadence_qspi_apb_exec_flash_cmd(reg_base, reg);
+	if (status != 0)
+		return status;
+
+	reg = readl(reg_base + CQSPI_REG_CMDREADDATALOWER);
+
+	/* Put the read value into rx_buf */
+	read_len = (rxlen > 4) ? 4 : rxlen;
+	memcpy(rxbuf, &reg, read_len);
+	rxbuf += read_len;
+
+	if (rxlen > 4) {
+		reg = readl(reg_base + CQSPI_REG_CMDREADDATAUPPER);
+
+		read_len = rxlen - read_len;
+		memcpy(rxbuf, &reg, read_len);
+	}
+	return 0;
+}
+
+/* For commands: WRSR, WREN, WRDI, CHIP_ERASE, BE, etc. */
+int cadence_qspi_apb_command_write(void *reg_base, unsigned int cmdlen,
+	const u8 *cmdbuf, unsigned int txlen,  const u8 *txbuf)
+{
+	unsigned int reg = 0;
+	unsigned int addr_value;
+	unsigned int wr_data;
+	unsigned int wr_len;
+
+	if (!cmdlen || cmdlen > 5 || txlen > 8 || cmdbuf == NULL) {
+		printf("QSPI: Invalid input arguments cmdlen %d txlen %d\n",
+			cmdlen, txlen);
+		return -EINVAL;
+	}
+
+	reg |= cmdbuf[0] << CQSPI_REG_CMDCTRL_OPCODE_LSB;
+
+	if (cmdlen == 4 || cmdlen == 5) {
+		/* Command with address */
+		reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB);
+		/* Number of bytes to write. */
+		reg |= ((cmdlen - 2) & CQSPI_REG_CMDCTRL_ADD_BYTES_MASK)
+			<< CQSPI_REG_CMDCTRL_ADD_BYTES_LSB;
+		/* Get address */
+		addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1],
+			cmdlen >= 5 ? 4 : 3);
+
+		writel(addr_value, reg_base + CQSPI_REG_CMDADDRESS);
+	}
+
+	if (txlen) {
+		/* writing data = yes */
+		reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB);
+		reg |= ((txlen - 1) & CQSPI_REG_CMDCTRL_WR_BYTES_MASK)
+			<< CQSPI_REG_CMDCTRL_WR_BYTES_LSB;
+
+		wr_len = txlen > 4 ? 4 : txlen;
+		memcpy(&wr_data, txbuf, wr_len);
+		writel(wr_data, reg_base +
+			CQSPI_REG_CMDWRITEDATALOWER);
+
+		if (txlen > 4) {
+			txbuf += wr_len;
+			wr_len = txlen - wr_len;
+			memcpy(&wr_data, txbuf, wr_len);
+			writel(wr_data, reg_base +
+				CQSPI_REG_CMDWRITEDATAUPPER);
+		}
+	}
+
+	/* Execute the command */
+	return cadence_qspi_apb_exec_flash_cmd(reg_base, reg);
+}
+
+/* Opcode + Address (3/4 bytes) + dummy bytes (0-4 bytes) */
+int cadence_qspi_apb_indirect_read_setup(void *reg_base,
+	unsigned int ahb_phy_addr, unsigned int cmdlen, const u8 *cmdbuf)
+{
+	unsigned int reg;
+	unsigned int rd_reg;
+	unsigned int addr_value;
+	unsigned int dummy_clk;
+	unsigned int dummy_bytes;
+	unsigned int addr_bytes;
+
+	/*
+	 * Identify addr_byte. All NOR flash device drivers are using fast read
+	 * which always expecting 1 dummy byte, 1 cmd byte and 3/4 addr byte.
+	 * With that, the length is in value of 5 or 6. Only FRAM chip from
+	 * ramtron using normal read (which won't need dummy byte).
+	 * Unlikely NOR flash using normal read due to performance issue.
+	 */
+	if (cmdlen >= 5)
+		/* to cater fast read where cmd + addr + dummy */
+		addr_bytes = cmdlen - 2;
+	else
+		/* for normal read (only ramtron as of now) */
+		addr_bytes = cmdlen - 1;
+
+	/* Setup the indirect trigger address */
+	writel((ahb_phy_addr & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
+		reg_base + CQSPI_REG_INDIRECTTRIGGER);
+
+	/* Configure SRAM partition for read. */
+	writel(CQSPI_REG_SRAM_PARTITION_RD, reg_base +
+		CQSPI_REG_SRAMPARTITION);
+
+	/* Configure the opcode */
+	rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
+
+#if (CONFIG_SPI_FLASH_QUAD == 1)
+	/* Instruction and address at DQ0, data@DQ0-3. */
+	rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
+#endif
+
+	/* Get address */
+	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
+	writel(addr_value, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
+
+	/* The remaining lenght is dummy bytes. */
+	dummy_bytes = cmdlen - addr_bytes - 1;
+	if (dummy_bytes) {
+
+		if (dummy_bytes > CQSPI_DUMMY_BYTES_MAX)
+			dummy_bytes = CQSPI_DUMMY_BYTES_MAX;
+
+		rd_reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
+#if defined(CONFIG_SPL_SPI_XIP) && defined(CONFIG_SPL_BUILD)
+		writel(0x0, reg_base + CQSPI_REG_MODE_BIT);
+#else
+		writel(0xFF, reg_base + CQSPI_REG_MODE_BIT);
+#endif
+
+		/* Convert to clock cycles. */
+		dummy_clk = dummy_bytes * CQSPI_DUMMY_CLKS_PER_BYTE;
+		/* Need to minus the mode byte (8 clocks). */
+		dummy_clk -= CQSPI_DUMMY_CLKS_PER_BYTE;
+
+		if (dummy_clk)
+			rd_reg |= (dummy_clk & CQSPI_REG_RD_INSTR_DUMMY_MASK)
+				<< CQSPI_REG_RD_INSTR_DUMMY_LSB;
+	}
+
+	writel(rd_reg, reg_base + CQSPI_REG_RD_INSTR);
+
+	/* set device size */
+	reg = readl(reg_base + CQSPI_REG_SIZE);
+	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
+	reg |= (addr_bytes - 1);
+	writel(reg, reg_base + CQSPI_REG_SIZE);
+	return 0;
+}
+
+int cadence_qspi_apb_indirect_read_execute(void *reg_base,
+	void *ahb_base_addr, unsigned int rxlen, u8 *rxbuf)
+{
+	unsigned int reg;
+
+	writel(rxlen, reg_base + CQSPI_REG_INDIRECTRDBYTES);
+
+	/* Start the indirect read transfer */
+	writel(CQSPI_REG_INDIRECTRD_START_MASK,
+			reg_base + CQSPI_REG_INDIRECTRD);
+
+	if (qspi_read_sram_fifo_poll(reg_base, (void *)rxbuf,
+				(const void *)ahb_base_addr, rxlen)) {
+		goto failrd;
+	}
+
+	/* Check flash indirect controller */
+	reg = readl(reg_base + CQSPI_REG_INDIRECTRD);
+	if (!(reg & CQSPI_REG_INDIRECTRD_DONE_MASK)) {
+		reg = readl(reg_base + CQSPI_REG_INDIRECTRD);
+		printf("QSPI: indirect completion status "
+			"error with reg 0x%08x\n", reg);
+		goto failrd;
+	}
+
+	/* Clear indirect completion status */
+	writel(CQSPI_REG_INDIRECTRD_DONE_MASK,
+		reg_base + CQSPI_REG_INDIRECTRD);
+	return 0;
+
+failrd:
+	/* Cancel the indirect read */
+	writel(CQSPI_REG_INDIRECTRD_CANCEL_MASK,
+		reg_base + CQSPI_REG_INDIRECTRD);
+	return -1;
+}
+
+/* Opcode + Address (3/4 bytes) */
+int cadence_qspi_apb_indirect_write_setup(void *reg_base,
+	unsigned int ahb_phy_addr, unsigned int cmdlen, const u8 *cmdbuf)
+{
+	unsigned int reg;
+	unsigned int addr_bytes = cmdlen > 4 ? 4 : 3;
+
+	if (cmdlen < 4 || cmdbuf == NULL) {
+		printf("QSPI: iInvalid input argument, len %d cmdbuf 0x%08x\n",
+			cmdlen, (unsigned int)cmdbuf);
+		return -EINVAL;
+	}
+	/* Setup the indirect trigger address */
+	writel((ahb_phy_addr & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
+		reg_base + CQSPI_REG_INDIRECTTRIGGER);
+
+	writel(CQSPI_REG_SRAM_PARTITION_WR,
+		reg_base + CQSPI_REG_SRAMPARTITION);
+
+	/* Configure the opcode */
+	reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB;
+	writel(reg, reg_base + CQSPI_REG_WR_INSTR);
+
+	/* Setup write address. */
+	reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
+	writel(reg, reg_base + CQSPI_REG_INDIRECTWRSTARTADDR);
+
+	reg = readl(reg_base + CQSPI_REG_SIZE);
+	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
+	reg |= (addr_bytes - 1);
+	writel(reg, reg_base + CQSPI_REG_SIZE);
+	return 0;
+}
+
+int cadence_qspi_apb_indirect_write_execute(void *reg_base,
+	void *ahb_base_addr, unsigned int txlen, const u8 *txbuf)
+{
+	unsigned int reg = 0;
+	unsigned int retry;
+
+	/* Configure the indirect read transfer bytes */
+	writel(txlen, reg_base + CQSPI_REG_INDIRECTWRBYTES);
+
+	/* Start the indirect write transfer */
+	writel(CQSPI_REG_INDIRECTWR_START_MASK,
+			reg_base + CQSPI_REG_INDIRECTWR);
+
+	if (qpsi_write_sram_fifo_push(reg_base, ahb_base_addr,
+		(const void *)txbuf, txlen)) {
+		goto failwr;
+	}
+
+	/* Wait until last write is completed (FIFO empty) */
+	retry = CQSPI_REG_RETRY;
+	while (retry--) {
+		reg = CQSPI_GET_WR_SRAM_LEVEL(reg_base);
+		if (reg == 0)
+			break;
+
+		udelay(1);
+	}
+	if (reg != 0) {
+		printf("QSPI: timeout for indirect write\n");
+		goto failwr;
+	}
+
+	/* Check flash indirect controller status */
+	retry = CQSPI_REG_RETRY;
+	while (retry--) {
+		reg = readl(reg_base + CQSPI_REG_INDIRECTWR);
+		if (reg & CQSPI_REG_INDIRECTWR_DONE_MASK)
+			break;
+		udelay(1);
+	}
+	if (!(reg & CQSPI_REG_INDIRECTWR_DONE_MASK)) {
+		printf("QSPI: indirect completion "
+			"status error with reg 0x%08x\n", reg);
+		goto failwr;
+	}
+
+	/* Clear indirect completion status */
+	writel(CQSPI_REG_INDIRECTWR_DONE_MASK,
+		reg_base + CQSPI_REG_INDIRECTWR);
+	return 0;
+
+failwr:
+	/* Cancel the indirect write */
+	writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
+			reg_base + CQSPI_REG_INDIRECTWR);
+	return -1;
+}
+
+void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy)
+{
+	unsigned int reg;
+
+	/* enter XiP mode immediately and enable direct mode */
+	reg = readl(reg_base + CQSPI_REG_CONFIG);
+	reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
+	reg |= CQSPI_REG_CONFIG_DIRECT_MASK;
+	reg |= CQSPI_REG_CONFIG_XIP_IMM_MASK;
+	writel(reg, reg_base + CQSPI_REG_CONFIG);
+
+	/* keep the XiP mode */
+	writel(xip_dummy, reg_base + CQSPI_REG_MODE_BIT);
+
+	/* Enable mode bit@devrd */
+	reg = readl(reg_base + CQSPI_REG_RD_INSTR);
+	reg |= (1 << CQSPI_REG_RD_INSTR_MODE_EN_LSB);
+	writel(reg, reg_base + CQSPI_REG_RD_INSTR);
+}
-- 
2.1.1

[U-Boot] [PATCH 2/4 v3] arm: socfpga: Add Cadence QSPI support to config header

[Stefan Roese]

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Vince Bridgers <vbridger@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 include/configs/socfpga_cyclone5_common.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/include/configs/socfpga_cyclone5_common.h b/include/configs/socfpga_cyclone5_common.h
index 3f8f91f..d183510 100644
--- a/include/configs/socfpga_cyclone5_common.h
+++ b/include/configs/socfpga_cyclone5_common.h
@@ -142,6 +142,37 @@
 #endif
 
 /*
+ * QSPI support
+ */
+#ifdef CONFIG_CMD_SF
+#define CONFIG_CADENCE_QSPI
+#define CONFIG_CQSPI_BASE		(SOCFPGA_QSPI_ADDRESS)
+#define CONFIG_CQSPI_AHB_BASE		(SOCFPGA_QSPIDATA_ADDRESS)
+#define CONFIG_SPI_FLASH		/* SPI flash subsystem */
+#define CONFIG_SPI_FLASH_STMICRO	/* Micron/Numonyx flash */
+#define CONFIG_SPI_FLASH_SPANSION	/* Spansion flash */
+#define CONFIG_SPI_FLASH_MTD
+/* Flash device info */
+#define CONFIG_SF_DEFAULT_SPEED		(50000000)
+#define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
+#define CONFIG_SPI_FLASH_QUAD		(1)
+/* QSPI reference clock */
+#ifndef __ASSEMBLY__
+unsigned int cm_get_qspi_controller_clk_hz(void);
+#define CONFIG_CQSPI_REF_CLK		cm_get_qspi_controller_clk_hz()
+#endif
+/* QSPI page size and block size */
+#define CONFIG_CQSPI_PAGE_SIZE		(256)
+#define CONFIG_CQSPI_BLOCK_SIZE		(16)
+/* QSPI Delay timing */
+#define CONFIG_CQSPI_TSHSL_NS		(200)
+#define CONFIG_CQSPI_TSD2D_NS		(255)
+#define CONFIG_CQSPI_TCHSH_NS		(20)
+#define CONFIG_CQSPI_TSLCH_NS		(20)
+#define CONFIG_CQSPI_DECODER		(0)
+#endif	/* CONFIG_CMD_SF */
+
+/*
  * Serial Driver
  */
 #define CONFIG_SYS_NS16550
-- 
2.1.1

[U-Boot] [PATCH 3/4 v3] arm: socfpga: Don't define CONFIG_SPI_FLASH_QUAD

[Stefan Roese]

This define is currently not supported in mainline U-Boot. So don't
define it. Otherwise the drivers doesn't work correctly.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Vince Bridgers <vbridger@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 include/configs/socfpga_cyclone5_common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/socfpga_cyclone5_common.h b/include/configs/socfpga_cyclone5_common.h
index d183510..5ef6936 100644
--- a/include/configs/socfpga_cyclone5_common.h
+++ b/include/configs/socfpga_cyclone5_common.h
@@ -155,7 +155,9 @@
 /* Flash device info */
 #define CONFIG_SF_DEFAULT_SPEED		(50000000)
 #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_3
+#if 0 /* not supported in mainline right now */
 #define CONFIG_SPI_FLASH_QUAD		(1)
+#endif
 /* QSPI reference clock */
 #ifndef __ASSEMBLY__
 unsigned int cm_get_qspi_controller_clk_hz(void);
-- 
2.1.1

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Stefan Roese]

This is needed for the SoCFPGA booting from SPI NOR flash
e.g. (N25Q256A). With these changes, the SoCrates can boot and
re-boot (reset) from SPI NOR flash without any problems.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Vince Bridgers <vbridger@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Michael Trimarchi <michael@amarulasolutions.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 4d148d1..85b2677 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -355,6 +355,37 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
 		}
 	}
 
+#ifdef CONFIG_SPI_N25Q256A_RESET
+#define CMD_RESET_ENABLE	0x66
+#define CMD_RESET_MEMORY	0x99
+	/*
+	 * This is needed for the SoCFPGA booting from SPI NOR flash
+	 * e.g. (N25Q256A). Additionally its necessary to change
+	 * this line in the Linux SPI NOR flash driver:
+	 *
+	 * { "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512,
+	 *    SECT_4K | SHUTDOWN_3BYTE) },
+	 *
+	 * Add SHUTDOWN_3BYTE here.
+	 *
+	 * With these changes, the SoCrates can boot and re-boot
+	 * (reset) from SPI NOR flash without any problems.
+	 */
+	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	printf("SF: Device software reset\n");
+#endif
+
 #ifdef CONFIG_OF_CONTROL
 	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
 		debug("SF: FDT decode error\n");
-- 
2.1.1

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> This is needed for the SoCFPGA booting from SPI NOR flash
> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> re-boot (reset) from SPI NOR flash without any problems.

Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your 
board please?

[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Stefan Roese]

On 01.10.2014 20:25, Marek Vasut wrote:
> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
>> This is needed for the SoCFPGA booting from SPI NOR flash
>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
>> re-boot (reset) from SPI NOR flash without any problems.
>
> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your
> board please?
>
> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot

Yes. This seems to be that case. But I can't change it right now. So 
this "solution" with the soft-reset is better than nothing.

Thanks,
Stefan

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> On 01.10.2014 20:25, Marek Vasut wrote:
>>
>> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
>>>
>>> This is needed for the SoCFPGA booting from SPI NOR flash
>>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
>>> re-boot (reset) from SPI NOR flash without any problems.
>>
>>
>> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
>> your
>> board please?
>>
>> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
>
>
> Yes. This seems to be that case. But I can't change it right now. So this
> "solution" with the soft-reset is better than nothing.

If this is some think that must require, any possibility to this
resetting prior to u-boot?
like preloader or in first stage boot loader or something.

I feel this is mostly a kind of hardware treat, and looks like it's
touching generic sf framework
which is not so good.

thanks!
-- 
Jagan.

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Stefan Roese]

On 01.10.2014 21:04, Jagan Teki wrote:
>>>> This is needed for the SoCFPGA booting from SPI NOR flash
>>>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
>>>> re-boot (reset) from SPI NOR flash without any problems.
>>>
>>>
>>> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
>>> your
>>> board please?
>>>
>>> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
>>
>>
>> Yes. This seems to be that case. But I can't change it right now. So this
>> "solution" with the soft-reset is better than nothing.
>
> If this is some think that must require, any possibility to this
> resetting prior to u-boot?
> like preloader or in first stage boot loader or something.

Perhaps I was not clear with the intention of this patch. Its more to 
show how the problem with SPI flash on this platform / board (SoCrates 
in this case) can be solved. So that others have a reference. Thats why 
I marked it as RFC. Its not really meant for inclusion into mainline.

The real solution is a board rework. If not possible, the preloader 
should be changed. As I don't have access to the preloader code right 
now, this "solution" (I know, its more a hack) didn't seem too bad.

> I feel this is mostly a kind of hardware treat, and looks like it's
> touching generic sf framework
> which is not so good.

Full ack on this. So please don't pull in into mainline.

Thanks,
Stefan

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Wed 2014-10-01 21:25:12, Stefan Roese wrote:
> On 01.10.2014 21:04, Jagan Teki wrote:
> >>>>This is needed for the SoCFPGA booting from SPI NOR flash
> >>>>e.g. (N25Q256A). With these changes, the SoCrates can boot and
> >>>>re-boot (reset) from SPI NOR flash without any problems.
> >>>
> >>>
> >>>Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
> >>>your
> >>>board please?
> >>>
> >>>[1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> >>
> >>
> >>Yes. This seems to be that case. But I can't change it right now. So this
> >>"solution" with the soft-reset is better than nothing.
> >
> >If this is some think that must require, any possibility to this
> >resetting prior to u-boot?
> >like preloader or in first stage boot loader or something.
> 
> Perhaps I was not clear with the intention of this patch. Its more to show
> how the problem with SPI flash on this platform / board (SoCrates in this
> case) can be solved. So that others have a reference. Thats why I marked it
> as RFC. Its not really meant for inclusion into mainline.
> 
> The real solution is a board rework. If not possible, the preloader should
> be changed. As I don't have access to the preloader code right now, this
> "solution" (I know, its more a hack) didn't seem too bad.

For the record, I do not think preloader is good place for such
workaround. Preloader works with SDRAM, and should load real u-boot as
fast as possible. If it does not need to touch SPI (it does not,
right?) it should not need to work around bugs there.

Actually, if SPI driver is not compiled into the u-boot, kernel will
have to do the workaround.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Stefan Roese]

On 02.10.2014 01:07, Pavel Machek wrote:
> On Wed 2014-10-01 21:25:12, Stefan Roese wrote:
>> On 01.10.2014 21:04, Jagan Teki wrote:
>>>>>> This is needed for the SoCFPGA booting from SPI NOR flash
>>>>>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
>>>>>> re-boot (reset) from SPI NOR flash without any problems.
>>>>>
>>>>>
>>>>> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
>>>>> your
>>>>> board please?
>>>>>
>>>>> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
>>>>
>>>>
>>>> Yes. This seems to be that case. But I can't change it right now. So this
>>>> "solution" with the soft-reset is better than nothing.
>>>
>>> If this is some think that must require, any possibility to this
>>> resetting prior to u-boot?
>>> like preloader or in first stage boot loader or something.
>>
>> Perhaps I was not clear with the intention of this patch. Its more to show
>> how the problem with SPI flash on this platform / board (SoCrates in this
>> case) can be solved. So that others have a reference. Thats why I marked it
>> as RFC. Its not really meant for inclusion into mainline.
>>
>> The real solution is a board rework. If not possible, the preloader should
>> be changed. As I don't have access to the preloader code right now, this
>> "solution" (I know, its more a hack) didn't seem too bad.
>
> For the record, I do not think preloader is good place for such
> workaround. Preloader works with SDRAM, and should load real u-boot as
> fast as possible. If it does not need to touch SPI (it does not,
> right?) it should not need to work around bugs there.

In this case, where the board boots from SPI NOR flash, the Preloader 
(SPL U-Boot version) does use SPI. To load the main U-Boot image from 
the SPI NOR flash.

My current best guess is that this define in the Preloader (older SPL 
U-Boot version) causes these problems:

#define CONFIG_SPI_FLASH_QUAD		(1)

Once I have full access to the Preloader source (in a few days 
hopefully) I can verify this.

Thanks,
Stefan

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Wednesday, October 01, 2014 at 09:04:48 PM, Jagan Teki wrote:
> On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> > On 01.10.2014 20:25, Marek Vasut wrote:
> >> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> >>> This is needed for the SoCFPGA booting from SPI NOR flash
> >>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> >>> re-boot (reset) from SPI NOR flash without any problems.
> >> 
> >> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
> >> your
> >> board please?
> >> 
> >> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> > 
> > Yes. This seems to be that case. But I can't change it right now. So this
> > "solution" with the soft-reset is better than nothing.
> 
> If this is some think that must require, any possibility to this
> resetting prior to u-boot?
> like preloader or in first stage boot loader or something.

You do understand, that this is a hardware bug on one particular board, right ? 
This can _not_ be reliably solved in software, not ever. I keep seeing people 
implementing one such workaround after the other in linux-mtd list, but sooner
or later, they discover that their workaround is not reliable. Without proper 
reset logic in place, a system simply cannot reliably reboot, since it has no
way to put all the hardware into defined state.

Also, the N25Qxxx chips are particularly crappy in this aspect, since they tend 
to happily get stuck in some weird undefined state during reboot. Proper reset
logic solves this issue, see the link above please.

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Thu 2014-10-02 04:47:23, Marek Vasut wrote:
> On Wednesday, October 01, 2014 at 09:04:48 PM, Jagan Teki wrote:
> > On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> > > On 01.10.2014 20:25, Marek Vasut wrote:
> > >> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> > >>> This is needed for the SoCFPGA booting from SPI NOR flash
> > >>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> > >>> re-boot (reset) from SPI NOR flash without any problems.
> > >> 
> > >> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
> > >> your
> > >> board please?
> > >> 
> > >> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> > > 
> > > Yes. This seems to be that case. But I can't change it right now. So this
> > > "solution" with the soft-reset is better than nothing.
> > 
> > If this is some think that must require, any possibility to this
> > resetting prior to u-boot?
> > like preloader or in first stage boot loader or something.
> 
> You do understand, that this is a hardware bug on one particular board, right ? 
> This can _not_ be reliably solved in software, not ever. I keep seeing people 
> implementing one such workaround after the other in linux-mtd list, but sooner
> or later, they discover that their workaround is not reliable. Without proper 
> reset logic in place, a system simply cannot reliably reboot, since it has no
> way to put all the hardware into defined state.

Well, if you have >16M flash and if you need bootrom to work with it.

AFAICT, as long as you avoid using SPI from bootrom (socrates will happily run
from SD card, for example), reliable operation should be possible. And you can still use
SPI from Linux and u-boot...
									Pavel

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Thursday, October 02, 2014 at 10:40:52 AM, Pavel Machek wrote:
> On Thu 2014-10-02 04:47:23, Marek Vasut wrote:
> > On Wednesday, October 01, 2014 at 09:04:48 PM, Jagan Teki wrote:
> > > On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> > > > On 01.10.2014 20:25, Marek Vasut wrote:
> > > >> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> > > >>> This is needed for the SoCFPGA booting from SPI NOR flash
> > > >>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> > > >>> re-boot (reset) from SPI NOR flash without any problems.
> > > >> 
> > > >> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply
> > > >> to your
> > > >> board please?
> > > >> 
> > > >> [1]
> > > >> http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> > > > 
> > > > Yes. This seems to be that case. But I can't change it right now. So
> > > > this "solution" with the soft-reset is better than nothing.
> > > 
> > > If this is some think that must require, any possibility to this
> > > resetting prior to u-boot?
> > > like preloader or in first stage boot loader or something.
> > 
> > You do understand, that this is a hardware bug on one particular board,
> > right ? This can _not_ be reliably solved in software, not ever. I keep
> > seeing people implementing one such workaround after the other in
> > linux-mtd list, but sooner or later, they discover that their workaround
> > is not reliable. Without proper reset logic in place, a system simply
> > cannot reliably reboot, since it has no way to put all the hardware into
> > defined state.
> 
> Well, if you have >16M flash and if you need bootrom to work with it.
> 
> AFAICT, as long as you avoid using SPI from bootrom (socrates will happily
> run from SD card, for example), reliable operation should be possible. And
> you can still use SPI from Linux and u-boot...

SD has the same problem if you don't have proper reset logic for it ;-)

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Thu 2014-10-02 00:34:48, Jagan Teki wrote:
> On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> > On 01.10.2014 20:25, Marek Vasut wrote:
> >>
> >> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> >>>
> >>> This is needed for the SoCFPGA booting from SPI NOR flash
> >>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> >>> re-boot (reset) from SPI NOR flash without any problems.
> >>
> >>
> >> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
> >> your
> >> board please?
> >>
> >> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> >
> >
> > Yes. This seems to be that case. But I can't change it right now. So this
> > "solution" with the soft-reset is better than nothing.
> 
> If this is some think that must require, any possibility to this
> resetting prior to u-boot?
> like preloader or in first stage boot loader or something.

u-boot-spl 2013.01/altera set it up like this for us; and mainline
u-boot-spl does not work on socfpga... that's why we need to do it
here, and that's why you don't see it on your board.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Saturday, April 25, 2015 at 09:48:31 PM, Pavel Machek wrote:
> On Thu 2014-10-02 00:34:48, Jagan Teki wrote:
> > On 2 October 2014 00:27, Stefan Roese <sr@denx.de> wrote:
> > > On 01.10.2014 20:25, Marek Vasut wrote:
> > >> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> > >>> This is needed for the SoCFPGA booting from SPI NOR flash
> > >>> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> > >>> re-boot (reset) from SPI NOR flash without any problems.
> > >> 
> > >> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to
> > >> your
> > >> board please?
> > >> 
> > >> [1] http://www.rocketboards.org/foswiki/Documentation/SocBoardQspiBoot
> > > 
> > > Yes. This seems to be that case. But I can't change it right now. So
> > > this "solution" with the soft-reset is better than nothing.
> > 
> > If this is some think that must require, any possibility to this
> > resetting prior to u-boot?
> > like preloader or in first stage boot loader or something.
> 
> u-boot-spl 2013.01/altera set it up like this for us; and mainline
> u-boot-spl does not work on socfpga... that's why we need to do it
> here, and that's why you don't see it on your board.

As discussed in person, please rebase and repost.

You're right that it's a good idea to add an option to restart the
SPI NOR in software if needed be.

Best regards,
Marek Vasut

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

This is needed for the SoCFPGA booting from SPI NOR flash
e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not
available).

Signed-off-by: Stefan Roese <sr@denx.de>
Signed-off-by: Pavel Machek <pavel@denx.de>

---

Ported to today's u-boot version.

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 201471c..f7cfbd9 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 		}
 	}
 
+#ifdef CONFIG_SPI_N25Q256A_RESET
+#define CMD_RESET_ENABLE 0x66
+#define CMD_RESET_MEMORY 0x99
+	/*
+	 * This is needed for the SoCFPGA booting from SPI NOR flash
+	 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
+	 * working on that board) sets 4-byt addressing mode.
+	 *
+	 * Additionally it may be good idea to change
+	 * this line in the Linux SPI NOR flash driver:
+	 *
+	 * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
+	 *    SECT_4K | SHUTDOWN_3BYTE) },
+	 *
+	 * Add SHUTDOWN_3BYTE here.
+	 */
+	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	printf("SF: Device software reset\n");
+#endif
 #ifdef CONFIG_OF_CONTROL
 	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
 		debug("SF: FDT decode error\n");

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> This is needed for the SoCFPGA booting from SPI NOR flash
> e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not
> available).
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> ---
> 
> Ported to today's u-boot version.
> 
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 201471c..f7cfbd9 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> struct spi_flash *flash) }
>  	}
> 
> +#ifdef CONFIG_SPI_N25Q256A_RESET

Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
used would have similar issue.

This should also be documented in README I believe. It'd be nice if you
added diffstat into your patches as it makes things easier during review.

> +#define CMD_RESET_ENABLE 0x66
> +#define CMD_RESET_MEMORY 0x99
> +	/*
> +	 * This is needed for the SoCFPGA booting from SPI NOR flash
> +	 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
> +	 * working on that board) sets 4-byt addressing mode.
> +	 *
> +	 * Additionally it may be good idea to change
> +	 * this line in the Linux SPI NOR flash driver:

Please submit a patch for Linux then. But this will be extremely
crappy and unreliable solution, so you should at least make the
kernel spit some warning upon shutdown so people are aware they
are doing something terribly wrong.

> +	 * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> +	 *    SECT_4K | SHUTDOWN_3BYTE) },
> +	 *
> +	 * Add SHUTDOWN_3BYTE here.
> +	 */
> +	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> +	if (ret) {
> +		printf("SF: Failed issue reset command\n");

I thought this was just a reset-enable command. If this command
fails, user won't be able to tell which of these two failed, so
it's a bad idea to use the same error message for both.

> +		goto err_read_id;
> +	}
> +
> +	ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> +	if (ret) {
> +		printf("SF: Failed issue reset command\n");
> +		goto err_read_id;
> +	}
> +
> +	printf("SF: Device software reset\n");
> +#endif
>  #ifdef CONFIG_OF_CONTROL
>  	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
>  		debug("SF: FDT decode error\n");

Best regards,
Marek Vasut

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > index 201471c..f7cfbd9 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> > struct spi_flash *flash) }
> >  	}
> > 
> > +#ifdef CONFIG_SPI_N25Q256A_RESET
> 
> Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
> used would have similar issue.

I'm pretty sure some Micron parts use different interface.

> It'd be nice if you
> added diffstat into your patches as it makes things easier during
> review.

Yes, it also makes patch harder to create (as it is tricky to
hand-edit the patches), and having diffstat for a patch that fits on a
screen is just stupid.

> > +	 * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> > +	 *    SECT_4K | SHUTDOWN_3BYTE) },
> > +	 *
> > +	 * Add SHUTDOWN_3BYTE here.
> > +	 */
> > +	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > +	if (ret) {
> > +		printf("SF: Failed issue reset command\n");
> 
> I thought this was just a reset-enable command. If this command
> fails, user won't be able to tell which of these two failed, so
> it's a bad idea to use the same error message for both.

Ok.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Friday, May 01, 2015 at 04:49:37 PM, Pavel Machek wrote:
> On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> > On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > > index 201471c..f7cfbd9 100644
> > > --- a/drivers/mtd/spi/sf_probe.c
> > > +++ b/drivers/mtd/spi/sf_probe.c
> > > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> > > struct spi_flash *flash) }
> > > 
> > >  	}
> > > 
> > > +#ifdef CONFIG_SPI_N25Q256A_RESET
> > 
> > Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
> > used would have similar issue.
> 
> I'm pretty sure some Micron parts use different interface.

Which ones ?

> > It'd be nice if you
> > added diffstat into your patches as it makes things easier during
> > review.
> 
> Yes, it also makes patch harder to create (as it is tricky to
> hand-edit the patches)

git format-patch automatically inserts the diffstat for you.

> , and having diffstat for a patch that fits on a
> screen is just stupid.

It's a 2-line diffstat for this patch. If you get a 1-screen big
diffstat, then your patch is most likely wrong.

> > > +	 * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> > > +	 *    SECT_4K | SHUTDOWN_3BYTE) },
> > > +	 *
> > > +	 * Add SHUTDOWN_3BYTE here.
> > > +	 */
> > > +	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > > +	if (ret) {
> > > +		printf("SF: Failed issue reset command\n");
> > 
> > I thought this was just a reset-enable command. If this command
> > fails, user won't be able to tell which of these two failed, so
> > it's a bad idea to use the same error message for both.
> 
> Ok.
> 									Pavel

Best regards,
Marek Vasut

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Fri 2015-05-01 19:26:34, Marek Vasut wrote:
> On Friday, May 01, 2015 at 04:49:37 PM, Pavel Machek wrote:
> > On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> > > On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > > > index 201471c..f7cfbd9 100644
> > > > --- a/drivers/mtd/spi/sf_probe.c
> > > > +++ b/drivers/mtd/spi/sf_probe.c
> > > > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> > > > struct spi_flash *flash) }
> > > > 
> > > >  	}
> > > > 
> > > > +#ifdef CONFIG_SPI_N25Q256A_RESET
> > > 
> > > Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
> > > used would have similar issue.
> > 
> > I'm pretty sure some Micron parts use different interface.
> 
> Which ones ?

N25Q128?

> > > It'd be nice if you
> > > added diffstat into your patches as it makes things easier during
> > > review.
> > 
> > Yes, it also makes patch harder to create (as it is tricky to
> > hand-edit the patches)
> 
> git format-patch automatically inserts the diffstat for you.

As I have already told you, I'm not using git to submit my patches.

> > , and having diffstat for a patch that fits on a
> > screen is just stupid.
> 
> It's a 2-line diffstat for this patch. If you get a 1-screen big
> diffstat, then your patch is most likely wrong.

As I said, diffstat for patch that fits on screen is useless and
stupid.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv2] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
version working on that board) sets 4-byte addressing mode.

Signed-off-by: Pavel Machek <pavel@denx.de>

---
Diff to v2: Add README.

diff --git a/README b/README
index ee65fdb..69f9e5b 100644
--- a/README
+++ b/README
@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support
 		Timeout for waiting until spi transfer completed.
 		default: (CONFIG_SYS_HZ/100)     /* 10 ms */
 
+		CONFIG_SPI_N25Q256A_RESET
+		Reset SPI NOR flash. Needed when preloader sets it to 4-byte
+		mode, for example.
+
 - FPGA Support: CONFIG_FPGA
 
 		Enables FPGA subsystem.
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 201471c..6b39ce8 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 		}
 	}
 
+#ifdef CONFIG_SPI_N25Q256A_RESET
+#define CMD_RESET_ENABLE 0x66
+#define CMD_RESET_MEMORY 0x99
+	/*
+	 * This is needed for the SoCFPGA booting from SPI NOR flash
+	 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
+	 * working on that board) sets 4-byte addressing mode.
+	 */
+	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue enable reset command\n");
+		goto err_read_id;
+	}
+
+	ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	printf("SF: Device software reset\n");
+#endif
 #ifdef CONFIG_OF_CONTROL
 	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
 		debug("SF: FDT decode error\n");

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv2] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Sunday, May 10, 2015 at 11:15:41 AM, Pavel Machek wrote:
> Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> version working on that board) sets 4-byte addressing mode.
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>

I'm still convinced this should be MICRON specific, not N25Q256 specific .

Best regards,
Marek Vasut

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
version working on that board) sets 4-byte addressing mode.

Signed-off-by: Pavel Machek <pavel@denx.de>

---
Diff to v3: Marek insists this is Micron specific, so lets rename the
config option.

diff --git a/README b/README
index ee65fdb..69f9e5b 100644
--- a/README
+++ b/README
@@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support
 		Timeout for waiting until spi transfer completed.
 		default: (CONFIG_SYS_HZ/100)     /* 10 ms */
 
+		CONFIG_SPI_MICRON_RESET
+		Reset SPI NOR flash. Needed when preloader sets it to 4-byte
+		mode, for example.
+
 - FPGA Support: CONFIG_FPGA
 
 		Enables FPGA subsystem.
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 201471c..6b39ce8 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 		}
 	}
 
+#ifdef CONFIG_SPI_MICRON_RESET
+#define CMD_RESET_ENABLE 0x66
+#define CMD_RESET_MEMORY 0x99
+	/*
+	 * This is needed for the SoCFPGA booting from SPI NOR flash
+	 * e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version
+	 * working on that board) sets 4-byte addressing mode.
+	 */
+	ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue enable reset command\n");
+		goto err_read_id;
+	}
+
+	ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+	if (ret) {
+		printf("SF: Failed issue reset command\n");
+		goto err_read_id;
+	}
+
+	printf("SF: Device software reset\n");
+#endif
 #ifdef CONFIG_OF_CONTROL
 	if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
 		debug("SF: FDT decode error\n");


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 10 May 2015 at 16:19, Pavel Machek <pavel@denx.de> wrote:
> On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
> Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> version working on that board) sets 4-byte addressing mode.
>
> Signed-off-by: Pavel Machek <pavel@denx.de>
>
> ---
> Diff to v3: Marek insists this is Micron specific, so lets rename the
> config option.
>
> diff --git a/README b/README
> index ee65fdb..69f9e5b 100644
> --- a/README
> +++ b/README
> @@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support
>                 Timeout for waiting until spi transfer completed.
>                 default: (CONFIG_SYS_HZ/100)     /* 10 ms */
>
> +               CONFIG_SPI_MICRON_RESET
> +               Reset SPI NOR flash. Needed when preloader sets it to 4-byte
> +               mode, for example.
> +
>  - FPGA Support: CONFIG_FPGA
>
>                 Enables FPGA subsystem.
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 201471c..6b39ce8 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
>                 }
>         }
>
> +#ifdef CONFIG_SPI_MICRON_RESET
> +#define CMD_RESET_ENABLE 0x66
> +#define CMD_RESET_MEMORY 0x99
> +       /*
> +        * This is needed for the SoCFPGA booting from SPI NOR flash
> +        * e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version
> +        * working on that board) sets 4-byte addressing mode.
> +        */
> +       ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> +       if (ret) {
> +               printf("SF: Failed issue enable reset command\n");
> +               goto err_read_id;
> +       }
> +
> +       ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> +       if (ret) {
> +               printf("SF: Failed issue reset command\n");
> +               goto err_read_id;
> +       }
> +
> +       printf("SF: Device software reset\n");
> +#endif
>  #ifdef CONFIG_OF_CONTROL
>         if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
>                 debug("SF: FDT decode error\n");
>

As per my early comments on this thread, I'm not happy with this approach of
being added flash specific in generic code by simply added like this.

I encounter similar issue before and I simply reverted [1]
http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364b

Please think in a different perceptive like adding any flags to specific vendor
with specific part, but that also require proper tested.

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Sunday, May 10, 2015 at 02:24:01 PM, Jagan Teki wrote:
> On 10 May 2015 at 16:19, Pavel Machek <pavel@denx.de> wrote:
> > On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
> > Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> > version working on that board) sets 4-byte addressing mode.
> > 
> > Signed-off-by: Pavel Machek <pavel@denx.de>

So, I took one more look into the datasheet [1]. With this chip, which
is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't
care about it at all.

But you actually do care about it if your hardware is broken and you use
the N25Q256A [2] part. If the problem really is just between the U-Boot
SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such
that it reads FSR (Flag Status Register, see page 27, table 17, bit 0).
This bit tells you whether the part is in 3-byte or 4-byte mode and you
can read this register in either mode.

Same applies to N25Q512A [3], FSR bit 0 indicates the mode.

[1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_128mb_1_8v_65nm.pdf
[2] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_256mb_3v_65nm.pdf
[3] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_512mb_1ce_3v_65nm.pdf

Best regards,
Marek Vasut

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Sun 2015-05-10 18:25:59, Marek Vasut wrote:
> On Sunday, May 10, 2015 at 02:24:01 PM, Jagan Teki wrote:
> > On 10 May 2015 at 16:19, Pavel Machek <pavel@denx.de> wrote:
> > > On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
> > > Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> > > version working on that board) sets 4-byte addressing mode.
> > > 
> > > Signed-off-by: Pavel Machek <pavel@denx.de>
> 
> So, I took one more look into the datasheet [1]. With this chip, which
> is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't
> care about it at all.
> 
> But you actually do care about it if your hardware is broken and you use
> the N25Q256A [2] part. If the problem really is just between the
> U-Boot

No, I actually care whenever I use the U-boot SPL 2013, which is the
only option today. And I explained it to you already. (Additionally,
hardware might be broken. That has nothing to do with _this_ problem).

> SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such
> that it reads FSR (Flag Status Register, see page 27, table 17, bit 0).
> This bit tells you whether the part is in 3-byte or 4-byte mode and you
> can read this register in either mode.

How does reading Micron-specific register help with code being too
Micron-specific is unclear to me.

								Pavel
> 
> Same applies to N25Q512A [3], FSR bit 0 indicates the mode.
> 
> [1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
> flash/serial-nor/n25q/n25q_128mb_1_8v_65nm.pdf
> [2] https://www.micron.com/~/media/documents/products/data-sheet/nor-
> flash/serial-nor/n25q/n25q_256mb_3v_65nm.pdf
> [3] https://www.micron.com/~/media/documents/products/data-sheet/nor-
> flash/serial-nor/n25q/n25q_512mb_1ce_3v_65nm.pdf
> 
> Best regards,
> Marek Vasut

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Sunday, May 10, 2015 at 07:43:52 PM, Pavel Machek wrote:
> On Sun 2015-05-10 18:25:59, Marek Vasut wrote:
> > On Sunday, May 10, 2015 at 02:24:01 PM, Jagan Teki wrote:
> > > On 10 May 2015 at 16:19, Pavel Machek <pavel@denx.de> wrote:
> > > > On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
> > > > Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> > > > version working on that board) sets 4-byte addressing mode.
> > > > 
> > > > Signed-off-by: Pavel Machek <pavel@denx.de>
> > 
> > So, I took one more look into the datasheet [1]. With this chip, which
> > is 128Mbit (16MByte), you don't even use 4-byte addressing so you don't
> > care about it at all.
> > 
> > But you actually do care about it if your hardware is broken and you use
> > the N25Q256A [2] part. If the problem really is just between the
> > U-Boot
> 
> No, I actually care whenever I use the U-boot SPL 2013, which is the
> only option today. And I explained it to you already. (Additionally,
> hardware might be broken. That has nothing to do with _this_ problem).

OK

> > SPL and U-Boot (which I doubt btw), you can probably augment U-Boot such
> > that it reads FSR (Flag Status Register, see page 27, table 17, bit 0).
> > This bit tells you whether the part is in 3-byte or 4-byte mode and you
> > can read this register in either mode.
> 
> How does reading Micron-specific register help with code being too
> Micron-specific is unclear to me.

Just add a micron-specific hook and check the register on Micron parts
only. This would eliminate the need for new CONFIG_foo_bar option, which
is always a plus.

Best regards,
Marek Vasut

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Sun 2015-05-10 17:54:01, Jagan Teki wrote:
> On 10 May 2015 at 16:19, Pavel Machek <pavel@denx.de> wrote:
> > On Sun 2015-05-10 11:15:41, Pavel Machek wrote:
> > Add reset for N25Q256A SPI NOR, as U-Boot SPL 2013-socfpga (only
> > version working on that board) sets 4-byte addressing mode.
> >
> > Signed-off-by: Pavel Machek <pavel@denx.de>
> >
> > ---
> > Diff to v3: Marek insists this is Micron specific, so lets rename the
> > config option.
> >
> > diff --git a/README b/README
> > index ee65fdb..69f9e5b 100644
> > --- a/README
> > +++ b/README
> > @@ -2775,6 +2775,10 @@ CBFS (Coreboot Filesystem) support
> >                 Timeout for waiting until spi transfer completed.
> >                 default: (CONFIG_SYS_HZ/100)     /* 10 ms */
> >
> > +               CONFIG_SPI_MICRON_RESET
> > +               Reset SPI NOR flash. Needed when preloader sets it to 4-byte
> > +               mode, for example.
> > +
> >  - FPGA Support: CONFIG_FPGA
> >
> >                 Enables FPGA subsystem.
> > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > index 201471c..6b39ce8 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -347,6 +348,28 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
> >                 }
> >         }
> >
> > +#ifdef CONFIG_SPI_MICRON_RESET
> > +#define CMD_RESET_ENABLE 0x66
> > +#define CMD_RESET_MEMORY 0x99
> > +       /*
> > +        * This is needed for the SoCFPGA booting from SPI NOR flash
> > +        * e.g. (MICRON), as U-Boot SPL 2013-socfpga (only version
> > +        * working on that board) sets 4-byte addressing mode.
> > +        */
> > +       ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > +       if (ret) {
> > +               printf("SF: Failed issue enable reset command\n");
> > +               goto err_read_id;
> > +       }
> > +
> > +       ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> > +       if (ret) {
> > +               printf("SF: Failed issue reset command\n");
> > +               goto err_read_id;
> > +       }
> > +
> > +       printf("SF: Device software reset\n");
> > +#endif
> >  #ifdef CONFIG_OF_CONTROL
> >         if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
> >                 debug("SF: FDT decode error\n");
> >
> 
> As per my early comments on this thread, I'm not happy with this approach of
> being added flash specific in generic code by simply added like this.
> 
> I encounter similar issue before and I simply reverted [1]
> http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d34ae648364b
> 
> Please think in a different perceptive like adding any flags to specific vendor
> with specific part, but that also require proper tested.

Ok, could you explain how you'd like to have it solved? Should we just
call spi_flash_cmd() from socfpga-specific code?

But I'm pretty sure similar issue will be encountered on different
boards, so it would be good to have it in shared place. Do you want me
to create sf_probe_micron and move it there?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:

[...]

> > As per my early comments on this thread, I'm not happy with this approach
> > of being added flash specific in generic code by simply added like this.
> > 
> > I encounter similar issue before and I simply reverted [1]
> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
> > 34ae648364b
> > 
> > Please think in a different perceptive like adding any flags to specific
> > vendor with specific part, but that also require proper tested.
> 
> Ok, could you explain how you'd like to have it solved? Should we just
> call spi_flash_cmd() from socfpga-specific code?
> 
> But I'm pretty sure similar issue will be encountered on different
> boards, so it would be good to have it in shared place. Do you want me
> to create sf_probe_micron and move it there?

I'd just add a flag into the SPI NOR table and in the code, I'd check
if the flag is set and if so, trigger the Micron-specific code. That
ought to be simple and it doesn't introduce any new config options.

Best regards,
Marek Vasut

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
> On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
>
> [...]
>
>> > As per my early comments on this thread, I'm not happy with this approach
>> > of being added flash specific in generic code by simply added like this.
>> >
>> > I encounter similar issue before and I simply reverted [1]
>> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
>> > 34ae648364b
>> >
>> > Please think in a different perceptive like adding any flags to specific
>> > vendor with specific part, but that also require proper tested.
>>
>> Ok, could you explain how you'd like to have it solved? Should we just
>> call spi_flash_cmd() from socfpga-specific code?
>>
>> But I'm pretty sure similar issue will be encountered on different
>> boards, so it would be good to have it in shared place. Do you want me
>> to create sf_probe_micron and move it there?
>
> I'd just add a flag into the SPI NOR table and in the code, I'd check
> if the flag is set and if so, trigger the Micron-specific code. That
> ought to be simple and it doesn't introduce any new config options.

Ok, solutions - what about Linux same issue or this requirement should be only
for u-boot or bootloaders?

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
> >
> > [...]
> >
> >> > As per my early comments on this thread, I'm not happy with this approach
> >> > of being added flash specific in generic code by simply added like this.
> >> >
> >> > I encounter similar issue before and I simply reverted [1]
> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
> >> > 34ae648364b
> >> >
> >> > Please think in a different perceptive like adding any flags to specific
> >> > vendor with specific part, but that also require proper tested.
> >>
> >> Ok, could you explain how you'd like to have it solved? Should we just
> >> call spi_flash_cmd() from socfpga-specific code?
> >>
> >> But I'm pretty sure similar issue will be encountered on different
> >> boards, so it would be good to have it in shared place. Do you want me
> >> to create sf_probe_micron and move it there?
> >
> > I'd just add a flag into the SPI NOR table and in the code, I'd check
> > if the flag is set and if so, trigger the Micron-specific code. That
> > ought to be simple and it doesn't introduce any new config options.
> 
> Ok, solutions - what about Linux same issue or this requirement should be only
> for u-boot or bootloaders?

Linux probably has same issue.

Do you like Marek's "add a flag to table" proposal?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
> On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
>> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
>> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
>> >
>> > [...]
>> >
>> >> > As per my early comments on this thread, I'm not happy with this approach
>> >> > of being added flash specific in generic code by simply added like this.
>> >> >
>> >> > I encounter similar issue before and I simply reverted [1]
>> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
>> >> > 34ae648364b
>> >> >
>> >> > Please think in a different perceptive like adding any flags to specific
>> >> > vendor with specific part, but that also require proper tested.
>> >>
>> >> Ok, could you explain how you'd like to have it solved? Should we just
>> >> call spi_flash_cmd() from socfpga-specific code?
>> >>
>> >> But I'm pretty sure similar issue will be encountered on different
>> >> boards, so it would be good to have it in shared place. Do you want me
>> >> to create sf_probe_micron and move it there?
>> >
>> > I'd just add a flag into the SPI NOR table and in the code, I'd check
>> > if the flag is set and if so, trigger the Micron-specific code. That
>> > ought to be simple and it doesn't introduce any new config options.
>>
>> Ok, solutions - what about Linux same issue or this requirement should be only
>> for u-boot or bootloaders?
>
> Linux probably has same issue.

Please check the same.

>
> Do you like Marek's "add a flag to table" proposal?

Please send the patch to both the ML's.

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
> >> >
> >> > [...]
> >> >
> >> >> > As per my early comments on this thread, I'm not happy with this approach
> >> >> > of being added flash specific in generic code by simply added like this.
> >> >> >
> >> >> > I encounter similar issue before and I simply reverted [1]
> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
> >> >> > 34ae648364b
> >> >> >
> >> >> > Please think in a different perceptive like adding any flags to specific
> >> >> > vendor with specific part, but that also require proper tested.
> >> >>
> >> >> Ok, could you explain how you'd like to have it solved? Should we just
> >> >> call spi_flash_cmd() from socfpga-specific code?
> >> >>
> >> >> But I'm pretty sure similar issue will be encountered on different
> >> >> boards, so it would be good to have it in shared place. Do you want me
> >> >> to create sf_probe_micron and move it there?
> >> >
> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
> >> > if the flag is set and if so, trigger the Micron-specific code. That
> >> > ought to be simple and it doesn't introduce any new config options.
> >>
> >> Ok, solutions - what about Linux same issue or this requirement should be only
> >> for u-boot or bootloaders?
> >
> > Linux probably has same issue.
> 
> Please check the same.
> 
> >
> > Do you like Marek's "add a flag to table" proposal?
> 
> Please send the patch to both the ML's.

Which mailing lists do you mean?

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 11 May 2015 at 14:09, Pavel Machek <pavel@denx.de> wrote:
> On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
>> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
>> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
>> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
>> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
>> >> >
>> >> > [...]
>> >> >
>> >> >> > As per my early comments on this thread, I'm not happy with this approach
>> >> >> > of being added flash specific in generic code by simply added like this.
>> >> >> >
>> >> >> > I encounter similar issue before and I simply reverted [1]
>> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
>> >> >> > 34ae648364b
>> >> >> >
>> >> >> > Please think in a different perceptive like adding any flags to specific
>> >> >> > vendor with specific part, but that also require proper tested.
>> >> >>
>> >> >> Ok, could you explain how you'd like to have it solved? Should we just
>> >> >> call spi_flash_cmd() from socfpga-specific code?
>> >> >>
>> >> >> But I'm pretty sure similar issue will be encountered on different
>> >> >> boards, so it would be good to have it in shared place. Do you want me
>> >> >> to create sf_probe_micron and move it there?
>> >> >
>> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
>> >> > if the flag is set and if so, trigger the Micron-specific code. That
>> >> > ought to be simple and it doesn't introduce any new config options.
>> >>
>> >> Ok, solutions - what about Linux same issue or this requirement should be only
>> >> for u-boot or bootloaders?
>> >
>> > Linux probably has same issue.
>>
>> Please check the same.
>>
>> >
>> > Do you like Marek's "add a flag to table" proposal?
>>
>> Please send the patch to both the ML's.
>
> Which mailing lists do you mean?

I meant to say if something is not working with Linux as well, send the same
patch to both U-Boot and Linux.

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Mon 2015-05-11 14:14:32, Jagan Teki wrote:
> On 11 May 2015 at 14:09, Pavel Machek <pavel@denx.de> wrote:
> > On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
> >> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
> >> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
> >> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
> >> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
> >> >> >
> >> >> > [...]
> >> >> >
> >> >> >> > As per my early comments on this thread, I'm not happy with this approach
> >> >> >> > of being added flash specific in generic code by simply added like this.
> >> >> >> >
> >> >> >> > I encounter similar issue before and I simply reverted [1]
> >> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
> >> >> >> > 34ae648364b
> >> >> >> >
> >> >> >> > Please think in a different perceptive like adding any flags to specific
> >> >> >> > vendor with specific part, but that also require proper tested.
> >> >> >>
> >> >> >> Ok, could you explain how you'd like to have it solved? Should we just
> >> >> >> call spi_flash_cmd() from socfpga-specific code?
> >> >> >>
> >> >> >> But I'm pretty sure similar issue will be encountered on different
> >> >> >> boards, so it would be good to have it in shared place. Do you want me
> >> >> >> to create sf_probe_micron and move it there?
> >> >> >
> >> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
> >> >> > if the flag is set and if so, trigger the Micron-specific code. That
> >> >> > ought to be simple and it doesn't introduce any new config options.
> >> >>
> >> >> Ok, solutions - what about Linux same issue or this requirement should be only
> >> >> for u-boot or bootloaders?
> >> >
> >> > Linux probably has same issue.
> >>
> >> Please check the same.
> >>
> >> >
> >> > Do you like Marek's "add a flag to table" proposal?
> >>
> >> Please send the patch to both the ML's.
> >
> > Which mailing lists do you mean?
> 
> I meant to say if something is not working with Linux as well, send the same
> patch to both U-Boot and Linux.

Linux already handles it, see drivers/mtd/spi-nor/spi-nor.c, functions
spi_nor_scan and set_4byte.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 11 May 2015 at 14:20, Pavel Machek <pavel@denx.de> wrote:
> On Mon 2015-05-11 14:14:32, Jagan Teki wrote:
>> On 11 May 2015 at 14:09, Pavel Machek <pavel@denx.de> wrote:
>> > On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
>> >> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
>> >> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
>> >> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
>> >> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
>> >> >> >
>> >> >> > [...]
>> >> >> >
>> >> >> >> > As per my early comments on this thread, I'm not happy with this approach
>> >> >> >> > of being added flash specific in generic code by simply added like this.
>> >> >> >> >
>> >> >> >> > I encounter similar issue before and I simply reverted [1]
>> >> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
>> >> >> >> > 34ae648364b
>> >> >> >> >
>> >> >> >> > Please think in a different perceptive like adding any flags to specific
>> >> >> >> > vendor with specific part, but that also require proper tested.
>> >> >> >>
>> >> >> >> Ok, could you explain how you'd like to have it solved? Should we just
>> >> >> >> call spi_flash_cmd() from socfpga-specific code?
>> >> >> >>
>> >> >> >> But I'm pretty sure similar issue will be encountered on different
>> >> >> >> boards, so it would be good to have it in shared place. Do you want me
>> >> >> >> to create sf_probe_micron and move it there?
>> >> >> >
>> >> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
>> >> >> > if the flag is set and if so, trigger the Micron-specific code. That
>> >> >> > ought to be simple and it doesn't introduce any new config options.
>> >> >>
>> >> >> Ok, solutions - what about Linux same issue or this requirement should be only
>> >> >> for u-boot or bootloaders?
>> >> >
>> >> > Linux probably has same issue.
>> >>
>> >> Please check the same.
>> >>
>> >> >
>> >> > Do you like Marek's "add a flag to table" proposal?
>> >>
>> >> Please send the patch to both the ML's.
>> >
>> > Which mailing lists do you mean?
>>
>> I meant to say if something is not working with Linux as well, send the same
>> patch to both U-Boot and Linux.
>
> Linux already handles it, see drivers/mtd/spi-nor/spi-nor.c, functions
> spi_nor_scan and set_4byte.

Seems like nothing much doing in micron side
        switch (JEDEC_MFR(info)) {
        case CFI_MFR_ST: /* Micron, actually */
                /* Some Micron need WREN command; all will accept it */
                need_wren = true;

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Mon 2015-05-11 14:35:36, Jagan Teki wrote:
> On 11 May 2015 at 14:20, Pavel Machek <pavel@denx.de> wrote:
> > On Mon 2015-05-11 14:14:32, Jagan Teki wrote:
> >> On 11 May 2015 at 14:09, Pavel Machek <pavel@denx.de> wrote:
> >> > On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
> >> >> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
> >> >> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
> >> >> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
> >> >> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
> >> >> >> >
> >> >> >> > [...]
> >> >> >> >
> >> >> >> >> > As per my early comments on this thread, I'm not happy with this approach
> >> >> >> >> > of being added flash specific in generic code by simply added like this.
> >> >> >> >> >
> >> >> >> >> > I encounter similar issue before and I simply reverted [1]
> >> >> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
> >> >> >> >> > 34ae648364b
> >> >> >> >> >
> >> >> >> >> > Please think in a different perceptive like adding any flags to specific
> >> >> >> >> > vendor with specific part, but that also require proper tested.
> >> >> >> >>
> >> >> >> >> Ok, could you explain how you'd like to have it solved? Should we just
> >> >> >> >> call spi_flash_cmd() from socfpga-specific code?
> >> >> >> >>
> >> >> >> >> But I'm pretty sure similar issue will be encountered on different
> >> >> >> >> boards, so it would be good to have it in shared place. Do you want me
> >> >> >> >> to create sf_probe_micron and move it there?
> >> >> >> >
> >> >> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
> >> >> >> > if the flag is set and if so, trigger the Micron-specific code. That
> >> >> >> > ought to be simple and it doesn't introduce any new config options.
> >> >> >>
> >> >> >> Ok, solutions - what about Linux same issue or this requirement should be only
> >> >> >> for u-boot or bootloaders?
> >> >> >
> >> >> > Linux probably has same issue.
> >> >>
> >> >> Please check the same.
> >> >>
> >> >> >
> >> >> > Do you like Marek's "add a flag to table" proposal?
> >> >>
> >> >> Please send the patch to both the ML's.
> >> >
> >> > Which mailing lists do you mean?
> >>
> >> I meant to say if something is not working with Linux as well, send the same
> >> patch to both U-Boot and Linux.
> >
> > Linux already handles it, see drivers/mtd/spi-nor/spi-nor.c, functions
> > spi_nor_scan and set_4byte.
> 
> Seems like nothing much doing in micron side
>         switch (JEDEC_MFR(info)) {
>         case CFI_MFR_ST: /* Micron, actually */
>                 /* Some Micron need WREN command; all will accept it */
>                 need_wren = true;

Umm. Take a closer look.

There's no break, so it continues below.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 11 May 2015 at 15:26, Pavel Machek <pavel@denx.de> wrote:
> On Mon 2015-05-11 14:35:36, Jagan Teki wrote:
>> On 11 May 2015 at 14:20, Pavel Machek <pavel@denx.de> wrote:
>> > On Mon 2015-05-11 14:14:32, Jagan Teki wrote:
>> >> On 11 May 2015 at 14:09, Pavel Machek <pavel@denx.de> wrote:
>> >> > On Mon 2015-05-11 14:03:06, Jagan Teki wrote:
>> >> >> On 11 May 2015 at 13:59, Pavel Machek <pavel@denx.de> wrote:
>> >> >> > On Mon 2015-05-11 13:35:04, Jagan Teki wrote:
>> >> >> >> On 11 May 2015 at 13:18, Marek Vasut <marex@denx.de> wrote:
>> >> >> >> > On Sunday, May 10, 2015 at 07:53:46 PM, Pavel Machek wrote:
>> >> >> >> >
>> >> >> >> > [...]
>> >> >> >> >
>> >> >> >> >> > As per my early comments on this thread, I'm not happy with this approach
>> >> >> >> >> > of being added flash specific in generic code by simply added like this.
>> >> >> >> >> >
>> >> >> >> >> > I encounter similar issue before and I simply reverted [1]
>> >> >> >> >> > http://git.denx.de/?p=u-boot.git;a=commit;h=122d805fd4bd478bb83536348291d
>> >> >> >> >> > 34ae648364b
>> >> >> >> >> >
>> >> >> >> >> > Please think in a different perceptive like adding any flags to specific
>> >> >> >> >> > vendor with specific part, but that also require proper tested.
>> >> >> >> >>
>> >> >> >> >> Ok, could you explain how you'd like to have it solved? Should we just
>> >> >> >> >> call spi_flash_cmd() from socfpga-specific code?
>> >> >> >> >>
>> >> >> >> >> But I'm pretty sure similar issue will be encountered on different
>> >> >> >> >> boards, so it would be good to have it in shared place. Do you want me
>> >> >> >> >> to create sf_probe_micron and move it there?
>> >> >> >> >
>> >> >> >> > I'd just add a flag into the SPI NOR table and in the code, I'd check
>> >> >> >> > if the flag is set and if so, trigger the Micron-specific code. That
>> >> >> >> > ought to be simple and it doesn't introduce any new config options.
>> >> >> >>
>> >> >> >> Ok, solutions - what about Linux same issue or this requirement should be only
>> >> >> >> for u-boot or bootloaders?
>> >> >> >
>> >> >> > Linux probably has same issue.
>> >> >>
>> >> >> Please check the same.
>> >> >>
>> >> >> >
>> >> >> > Do you like Marek's "add a flag to table" proposal?
>> >> >>
>> >> >> Please send the patch to both the ML's.
>> >> >
>> >> > Which mailing lists do you mean?
>> >>
>> >> I meant to say if something is not working with Linux as well, send the same
>> >> patch to both U-Boot and Linux.
>> >
>> > Linux already handles it, see drivers/mtd/spi-nor/spi-nor.c, functions
>> > spi_nor_scan and set_4byte.
>>
>> Seems like nothing much doing in micron side
>>         switch (JEDEC_MFR(info)) {
>>         case CFI_MFR_ST: /* Micron, actually */
>>                 /* Some Micron need WREN command; all will accept it */
>>                 need_wren = true;
>
> Umm. Take a closer look.
>
> There's no break, so it continues below.

Ohh.. So Linux works for you.. then is it?

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

> >> Seems like nothing much doing in micron side
> >>         switch (JEDEC_MFR(info)) {
> >>         case CFI_MFR_ST: /* Micron, actually */
> >>                 /* Some Micron need WREN command; all will accept it */
> >>                 need_wren = true;
> >
> > Umm. Take a closer look.
> >
> > There's no break, so it continues below.
> 
> Ohh.. So Linux works for you.. then is it?

Well, yes, but this code patch is actually not tested in my case,
because u-boot already resets it for Linux.

Anyway, is this what you'd like to see?
								Pavel

diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 4158e13..5379f18 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -43,6 +43,7 @@ enum {
 	SST_BP		= 1 << 3,
 	SST_WP		= 1 << 4,
 	WR_QPP		= 1 << 5,
+	MICRON_RESET    = 1 << 6,
 };
 
 #define SST_WR		(SST_BP | SST_WP)
diff --git a/drivers/mtd/spi/sf_params.c b/drivers/mtd/spi/sf_params.c
index c12e8c6..45525b5 100644
--- a/drivers/mtd/spi/sf_params.c
+++ b/drivers/mtd/spi/sf_params.c
@@ -85,7 +85,7 @@ const struct spi_flash_params spi_flash_params_table[] = {
 	{"N25Q128A",	   0x20bb18, 0x0,       64 * 1024,   256, RD_FULL,		     WR_QPP},
 	{"N25Q256",	   0x20ba19, 0x0,       64 * 1024,   512, RD_FULL,	   WR_QPP | SECT_4K},
 	{"N25Q256A",	   0x20bb19, 0x0,       64 * 1024,   512, RD_FULL,	   WR_QPP | SECT_4K},
-	{"N25Q512",	   0x20ba20, 0x0,       64 * 1024,  1024, RD_FULL, WR_QPP | E_FSR | SECT_4K},
+	{"N25Q512",	   0x20ba20, 0x0,       64 * 1024,  1024, RD_FULL, WR_QPP | E_FSR | SECT_4K | MICRON_RESET},
 	{"N25Q512A",	   0x20bb20, 0x0,       64 * 1024,  1024, RD_FULL, WR_QPP | E_FSR | SECT_4K},
 	{"N25Q1024",	   0x20ba21, 0x0,       64 * 1024,  2048, RD_FULL, WR_QPP | E_FSR | SECT_4K},
 	{"N25Q1024A",	   0x20bb21, 0x0,       64 * 1024,  2048, RD_FULL, WR_QPP | E_FSR | SECT_4K},
diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 201471c..04666b9 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -8,6 +8,7 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#define DEBUG
 #include <common.h>
 #include <dm.h>
 #include <errno.h>
@@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
 		flash->poll_cmd = CMD_FLAG_STATUS;
 #endif
 
+	if (params->flags & MICRON_RESET) {
+#define CMD_RESET_ENABLE 0x66
+#define CMD_RESET_MEMORY 0x99
+	  int ret;
+		/*
+		 * This is needed for the SoCFPGA booting from SPI NOR flash
+		 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
+		 * working on that board) sets 4-byte addressing mode.
+		 */
+		ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+		if (ret) {
+			printf("SF: Failed issue enable reset command\n");
+			return ret;
+		}
+
+		ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+		if (ret) {
+			printf("SF: Failed issue reset command\n");
+			return ret;			
+		}
+
+		printf("SF: Device software reset\n");
+	}
+
 	/* Configure the BAR - discover bank cmds and read current bank */
 #ifdef CONFIG_SPI_FLASH_BAR
 	u8 curr_bank = 0;
@@ -369,7 +394,7 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 	     ((flash->dual_flash > SF_SINGLE_FLASH) &&
 	     (flash->size > SPI_FLASH_16MB_BOUN << 1))) {
 		puts("SF: Warning - Only lower 16MiB accessible,");
-		puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
+		puts(" For full access, #define CONFIG_SPI_FLASH_BAR\n");
 	}
 #endif
 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Friday, May 15, 2015 at 10:47:46 AM, Pavel Machek wrote:
> > >> Seems like nothing much doing in micron side
> > >> 
> > >>         switch (JEDEC_MFR(info)) {
> > >>         case CFI_MFR_ST: /* Micron, actually */
> > >>         
> > >>                 /* Some Micron need WREN command; all will accept it
> > >>                 */ need_wren = true;
> > > 
> > > Umm. Take a closer look.
> > > 
> > > There's no break, so it continues below.
> > 
> > Ohh.. So Linux works for you.. then is it?
> 
> Well, yes, but this code patch is actually not tested in my case,
> because u-boot already resets it for Linux.
> 
> Anyway, is this what you'd like to see?

Was my idea of checking in which mode the SPI NOR is first discarded ?

[...]

> @@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave
> *spi, u8 *idcode, flash->poll_cmd = CMD_FLAG_STATUS;
>  #endif
> 
> +	if (params->flags & MICRON_RESET) {
> +#define CMD_RESET_ENABLE 0x66
> +#define CMD_RESET_MEMORY 0x99
> +	  int ret;

The indent here seems funny.

I think it'd be better to pull this reset procedure into a separate function.

> +		/*
> +		 * This is needed for the SoCFPGA booting from SPI NOR flash
> +		 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
> +		 * working on that board) sets 4-byte addressing mode.
> +		 */
> +		ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> +		if (ret) {
> +			printf("SF: Failed issue enable reset command\n");
> +			return ret;
> +		}
> +
> +		ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> +		if (ret) {
> +			printf("SF: Failed issue reset command\n");
> +			return ret;
> +		}
> +
> +		printf("SF: Device software reset\n");
> +	}

Otherwise I think it's OK.

[U-Boot] [PATCHv3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Fri 2015-05-15 11:36:11, Marek Vasut wrote:
> On Friday, May 15, 2015 at 10:47:46 AM, Pavel Machek wrote:
> > > >> Seems like nothing much doing in micron side
> > > >> 
> > > >>         switch (JEDEC_MFR(info)) {
> > > >>         case CFI_MFR_ST: /* Micron, actually */
> > > >>         
> > > >>                 /* Some Micron need WREN command; all will accept it
> > > >>                 */ need_wren = true;
> > > > 
> > > > Umm. Take a closer look.
> > > > 
> > > > There's no break, so it continues below.
> > > 
> > > Ohh.. So Linux works for you.. then is it?
> > 
> > Well, yes, but this code patch is actually not tested in my case,
> > because u-boot already resets it for Linux.
> > 
> > Anyway, is this what you'd like to see?
> 
> Was my idea of checking in which mode the SPI NOR is first discarded ?

Yes, better just reset it.

> [...]
> 
> > @@ -233,6 +234,30 @@ static int spi_flash_validate_params(struct spi_slave
> > *spi, u8 *idcode, flash->poll_cmd = CMD_FLAG_STATUS;
> >  #endif
> > 
> > +	if (params->flags & MICRON_RESET) {
> > +#define CMD_RESET_ENABLE 0x66
> > +#define CMD_RESET_MEMORY 0x99
> > +	  int ret;
> 
> The indent here seems funny.
> 
> I think it'd be better to pull this reset procedure into a separate
> function.

Can do, but I need to know if this is what Jagan wanted, first.
									Pavel

> > +		/*
> > +		 * This is needed for the SoCFPGA booting from SPI NOR flash
> > +		 * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
> > +		 * working on that board) sets 4-byte addressing mode.
> > +		 */
> > +		ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > +		if (ret) {
> > +			printf("SF: Failed issue enable reset command\n");
> > +			return ret;
> > +		}
> > +
> > +		ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> > +		if (ret) {
> > +			printf("SF: Failed issue reset command\n");
> > +			return ret;
> > +		}
> > +
> > +		printf("SF: Device software reset\n");
> > +	}
> 
> Otherwise I think it's OK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Marek Vasut]

On Sunday, May 10, 2015 at 11:07:38 AM, Pavel Machek wrote:
> On Fri 2015-05-01 19:26:34, Marek Vasut wrote:
> > On Friday, May 01, 2015 at 04:49:37 PM, Pavel Machek wrote:
> > > On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> > > > On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > > > > diff --git a/drivers/mtd/spi/sf_probe.c
> > > > > b/drivers/mtd/spi/sf_probe.c index 201471c..f7cfbd9 100644
> > > > > --- a/drivers/mtd/spi/sf_probe.c
> > > > > +++ b/drivers/mtd/spi/sf_probe.c
> > > > > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave
> > > > > *spi, struct spi_flash *flash) }
> > > > > 
> > > > >  	}
> > > > > 
> > > > > +#ifdef CONFIG_SPI_N25Q256A_RESET
> > > > 
> > > > Should be CONFIG_SPI_MICRON_RESET, since other parts which can also
> > > > be used would have similar issue.
> > > 
> > > I'm pretty sure some Micron parts use different interface.
> > 
> > Which ones ?
> 
> N25Q128?

According to [1] page 28,  the interface is the same. Did I miss something?

[1] https://www.micron.com/~/media/documents/products/data-sheet/nor-
flash/serial-nor/n25q/n25q_128_3_volt_with_boot_sector.pdf

> > > > It'd be nice if you
> > > > added diffstat into your patches as it makes things easier during
> > > > review.
> > > 
> > > Yes, it also makes patch harder to create (as it is tricky to
> > > hand-edit the patches)
> > 
> > git format-patch automatically inserts the diffstat for you.
> 
> As I have already told you, I'm not using git to submit my patches.

Please fix your tools then.

[...]

Best regards,
Marek Vasut

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Pavel Machek]

On Wed 2014-10-01 20:25:55, Marek Vasut wrote:
> On Wednesday, October 01, 2014 at 05:13:11 PM, Stefan Roese wrote:
> > This is needed for the SoCFPGA booting from SPI NOR flash
> > e.g. (N25Q256A). With these changes, the SoCrates can boot and
> > re-boot (reset) from SPI NOR flash without any problems.
> 
> Seems like your SPI NOR reset logic is buggy. Does any of [1] apply to your 
> board please?

Actually, this has nothing to do with SPI NOR reset logic.

Yes, we need to reset NOR flash during u-boot startup, because
hardware was already initialized to 4-byte addressing by
u-boot-spl. No matter what reset logic does, if it passes u-boot-spl
(and SPI NOR is actually used), it will be in 4-byte mode.

And no, mainline u-boot-spl is not usable on socfpga, and probably will
not be usable for a whill, so u-boot-spl from altera is only option
here.

So.. what to do?

Do you want this patch? Do you want patch that know how to switch back
to 3-byte addressing on this particular flash?

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] [RFC PATCH 4/4 v3] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

[Jagan Teki]

On 1 October 2014 20:43, Stefan Roese <sr@denx.de> wrote:
> This is needed for the SoCFPGA booting from SPI NOR flash
> e.g. (N25Q256A). With these changes, the SoCrates can boot and
> re-boot (reset) from SPI NOR flash without any problems.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@altera.com>
> Cc: Vince Bridgers <vbridger@altera.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Michael Trimarchi <michael@amarulasolutions.com>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> ---
>  drivers/mtd/spi/sf_probe.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 4d148d1..85b2677 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -355,6 +355,37 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
>                 }
>         }
>
> +#ifdef CONFIG_SPI_N25Q256A_RESET
> +#define CMD_RESET_ENABLE       0x66
> +#define CMD_RESET_MEMORY       0x99
> +       /*
> +        * This is needed for the SoCFPGA booting from SPI NOR flash
> +        * e.g. (N25Q256A). Additionally its necessary to change
> +        * this line in the Linux SPI NOR flash driver:
> +        *
> +        * { "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512,
> +        *    SECT_4K | SHUTDOWN_3BYTE) },
> +        *
> +        * Add SHUTDOWN_3BYTE here.
> +        *
> +        * With these changes, the SoCrates can boot and re-boot
> +        * (reset) from SPI NOR flash without any problems.
> +        */
> +       ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> +       if (ret) {
> +               printf("SF: Failed issue reset command\n");
> +               goto err_read_id;
> +       }
> +
> +       ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> +       if (ret) {
> +               printf("SF: Failed issue reset command\n");
> +               goto err_read_id;
> +       }
> +
> +       printf("SF: Device software reset\n");
> +#endif

I have tested N25Q256A on zynq, couldn't encounter this either.
This seems to be SPI flash logic more hardwired to N25Q256A.

Any comments?

> +
>  #ifdef CONFIG_OF_CONTROL
>         if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
>                 debug("SF: FDT decode error\n");
> --
> 2.1.1
>

thanks!
-- 
Jagan.

[U-Boot] [PATCH 0/2 v3] arm: socfpga: Add Cadence QSPI support

[Marek Vasut]

On Wednesday, October 01, 2014 at 05:13:07 PM, Stefan Roese wrote:
> Hi!
> 
> So this is my 3rd posting regarding the Candence SPI driver on SoCFPGA.
> 
> The current status is, that SPI NOR flash works now without problems.
> And dcache is still enabled. The previous disabling was not needed.
> And only caused problems while booting into Linux. No cache flush or
> invalidate is needed. As no DMA engine is involved in this transfer.
> 
> The "solution" I used to get SPI NOR flash support working now
> is implemented in patch 0004 (still marked as RFC). Here a software reset
> is issued on the Micron N25Q256A SPI NOR flash. This seems to solve all
> problems and reading / writing to the SPI NOR flash seems to work now
> just fine. Even rebooting via pushbutton-reset or reset command works.
> 
> I think this version is now in much better state. Thats why I removed
> the WIP from the subject lines.
> 
> Again, this is tested on the EBV SoCrates eval board.

I picked this up into u-boot-socfpga:topic/drivers/qspi-wip-20141003 so it 
doesn't get lost. Also, they are updated to use the latest socfpga stuff,
esp. since the socfpga_cyclone5_common.h was renamed to socfpga_common.h .

Best regards,
Marek Vasut