[PATCH v2 0/2] mtd: spi-nor: add stm32 qspi driver

[PATCH v2 0/2] mtd: spi-nor: add stm32 qspi driver

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch set adds a SPI-NOR driver for stm32 QSPI controller.
It is a specialized SPI interface for serial Flash devices.
It supports 1 or 2 Flash device with single, dual and quad SPI Flash memories.

It can operate in any of the following modes:
-indirect mode: all the operations are performed using the quadspi
  registers
-read memory-mapped mode: the external Flash memory is mapped to the
  microcontroller address space and is seen by the system as if it was
  an internal memory 

Marek, Cyrille
How do you wish process? 
If this patch set is OK for you, I agree for the merge
Cyrille, I working on rebase 
[PATCH v5 0/6] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories
I could send a patch for stm32 as soon as possible, like this you could add my
controller into your patchset. I would be glad to be your stm32 tester :-)

Changes in v2:
-awful construct: s/u32/u8
-add define for timeout
-Use a helper variable
-add comment on "stm32 qspi controller fsize issue"

Ludovic Barre (2):
  dt-bindings: Document the STM32 QSPI bindings
  mtd: spi-nor: add driver for STM32 quad spi flash controller

 .../devicetree/bindings/mtd/stm32-quadspi.txt      |  45 ++
 drivers/mtd/spi-nor/Kconfig                        |   7 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/stm32-quadspi.c                | 690 +++++++++++++++++++++
 4 files changed, 743 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
 create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c

-- 
2.7.4

[PATCH v2 0/2] mtd: spi-nor: add stm32 qspi driver

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>

This patch set adds a SPI-NOR driver for stm32 QSPI controller.
It is a specialized SPI interface for serial Flash devices.
It supports 1 or 2 Flash device with single, dual and quad SPI Flash memories.

It can operate in any of the following modes:
-indirect mode: all the operations are performed using the quadspi
  registers
-read memory-mapped mode: the external Flash memory is mapped to the
  microcontroller address space and is seen by the system as if it was
  an internal memory 

Marek, Cyrille
How do you wish process? 
If this patch set is OK for you, I agree for the merge
Cyrille, I working on rebase 
[PATCH v5 0/6] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories
I could send a patch for stm32 as soon as possible, like this you could add my
controller into your patchset. I would be glad to be your stm32 tester :-)

Changes in v2:
-awful construct: s/u32/u8
-add define for timeout
-Use a helper variable
-add comment on "stm32 qspi controller fsize issue"

Ludovic Barre (2):
  dt-bindings: Document the STM32 QSPI bindings
  mtd: spi-nor: add driver for STM32 quad spi flash controller

 .../devicetree/bindings/mtd/stm32-quadspi.txt      |  45 ++
 drivers/mtd/spi-nor/Kconfig                        |   7 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/stm32-quadspi.c                | 690 +++++++++++++++++++++
 4 files changed, 743 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
 create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds documentation of device tree bindings for the STM32
QSPI controller.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
new file mode 100644
index 0000000..95a8ebd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
@@ -0,0 +1,45 @@
+* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
+
+Required properties:
+- compatible: should be "st,stm32f469-qspi"
+- reg: contains the register location and length.
+  (optional) the memory mapping address and length
+- reg-names: list of the names corresponding to the previous register
+  Should contain "qspi" to register location
+  (optional) "qspi_mm" if read in memory map mode (improve read throughput)
+- interrupts: should contain the interrupt for the device
+- clocks: the phandle of the clock needed by the QSPI controller
+- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
+
+Optional properties:
+- resets: must contain the phandle to the reset controller.
+
+A spi flash must be a child of the nor_flash node and could have some
+properties. Also see jedec,spi-nor.txt.
+
+Required properties:
+- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
+- spi-max-frequency: max frequency of spi bus
+
+Optional property:
+- spi-rx-bus-width: the bus width (number of data wires)
+
+Example:
+
+qspi: qspi@a0001000 {
+	compatible = "st,stm32f469-qspi";
+	reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
+	reg-names = "qspi", "qspi_mm";
+	interrupts = <91>;
+	resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
+	clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_qspi0>;
+
+	flash@0 {
+		reg = <0>;
+		spi-rx-bus-width = <4>;
+		spi-max-frequency = <108000000>;
+		...
+	};
+};
-- 
2.7.4

[PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds documentation of device tree bindings for the STM32
QSPI controller.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
new file mode 100644
index 0000000..95a8ebd
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
@@ -0,0 +1,45 @@
+* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
+
+Required properties:
+- compatible: should be "st,stm32f469-qspi"
+- reg: contains the register location and length.
+  (optional) the memory mapping address and length
+- reg-names: list of the names corresponding to the previous register
+  Should contain "qspi" to register location
+  (optional) "qspi_mm" if read in memory map mode (improve read throughput)
+- interrupts: should contain the interrupt for the device
+- clocks: the phandle of the clock needed by the QSPI controller
+- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
+
+Optional properties:
+- resets: must contain the phandle to the reset controller.
+
+A spi flash must be a child of the nor_flash node and could have some
+properties. Also see jedec,spi-nor.txt.
+
+Required properties:
+- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
+- spi-max-frequency: max frequency of spi bus
+
+Optional property:
+- spi-rx-bus-width: the bus width (number of data wires)
+
+Example:
+
+qspi: qspi@a0001000 {
+	compatible = "st,stm32f469-qspi";
+	reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
+	reg-names = "qspi", "qspi_mm";
+	interrupts = <91>;
+	resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
+	clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_qspi0>;
+
+	flash@0 {
+		reg = <0>;
+		spi-rx-bus-width = <4>;
+		spi-max-frequency = <108000000>;
+		...
+	};
+};
-- 
2.7.4

[PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

The quadspi is a specialized communication interface targeting single,
dual or quad SPI Flash memories.

It can operate in any of the following modes:
-indirect mode: all the operations are performed using the quadspi
 registers
-read memory-mapped mode: the external Flash memory is mapped to the
 microcontroller address space and is seen by the system as if it was
 an internal memory

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mtd/spi-nor/Kconfig         |   7 +
 drivers/mtd/spi-nor/Makefile        |   1 +
 drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
 3 files changed, 698 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 7252087..bfdfb1e 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel-spi-platform.
 
+config SPI_STM32_QUADSPI
+	tristate "STM32 Quad SPI controller"
+	depends on ARCH_STM32
+	help
+	  This enables support for the STM32 Quad SPI controller.
+	  We only connect the NOR to this controller.
+
 endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 72238a7..285aab8 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
 obj-$(CONFIG_SPI_INTEL_SPI)	+= intel-spi.o
 obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM)	+= intel-spi-platform.o
+obj-$(CONFIG_SPI_STM32_QUADSPI)	+= stm32-quadspi.o
\ No newline at end of file
diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
new file mode 100644
index 0000000..7d26e37
--- /dev/null
+++ b/drivers/mtd/spi-nor/stm32-quadspi.c
@@ -0,0 +1,690 @@
+/*
+ * stm32_quadspi.c
+ *
+ * Copyright (C) 2017, Ludovic Barre
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define QUADSPI_CR		0x00
+#define CR_EN			BIT(0)
+#define CR_ABORT		BIT(1)
+#define CR_DMAEN		BIT(2)
+#define CR_TCEN			BIT(3)
+#define CR_SSHIFT		BIT(4)
+#define CR_DFM			BIT(6)
+#define CR_FSEL			BIT(7)
+#define CR_FTHRES_SHIFT		8
+#define CR_FTHRES_MASK		GENMASK(12, 8)
+#define CR_FTHRES(n)		(((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
+#define CR_TEIE			BIT(16)
+#define CR_TCIE			BIT(17)
+#define CR_FTIE			BIT(18)
+#define CR_SMIE			BIT(19)
+#define CR_TOIE			BIT(20)
+#define CR_PRESC_SHIFT		24
+#define CR_PRESC_MASK		GENMASK(31, 24)
+#define CR_PRESC(n)		(((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
+
+#define QUADSPI_DCR		0x04
+#define DCR_CSHT_SHIFT		8
+#define DCR_CSHT_MASK		GENMASK(10, 8)
+#define DCR_CSHT(n)		(((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
+#define DCR_FSIZE_SHIFT		16
+#define DCR_FSIZE_MASK		GENMASK(20, 16)
+#define DCR_FSIZE(n)		(((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
+
+#define QUADSPI_SR		0x08
+#define SR_TEF			BIT(0)
+#define SR_TCF			BIT(1)
+#define SR_FTF			BIT(2)
+#define SR_SMF			BIT(3)
+#define SR_TOF			BIT(4)
+#define SR_BUSY			BIT(5)
+#define SR_FLEVEL_SHIFT		8
+#define SR_FLEVEL_MASK		GENMASK(13, 8)
+
+#define QUADSPI_FCR		0x0c
+#define FCR_CTCF		BIT(1)
+
+#define QUADSPI_DLR		0x10
+
+#define QUADSPI_CCR		0x14
+#define CCR_INST_SHIFT		0
+#define CCR_INST_MASK		GENMASK(7, 0)
+#define CCR_INST(n)		(((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
+#define CCR_IMODE_NONE		(0 << 8)
+#define CCR_IMODE_1		(1 << 8)
+#define CCR_IMODE_2		(2 << 8)
+#define CCR_IMODE_4		(3 << 8)
+#define CCR_ADMODE_NONE		(0 << 10)
+#define CCR_ADMODE_1		(1 << 10)
+#define CCR_ADMODE_2		(2 << 10)
+#define CCR_ADMODE_4		(3 << 10)
+#define CCR_ADSIZE_SHIFT	12
+#define CCR_ADSIZE_MASK		GENMASK(13, 12)
+#define CCR_ADSIZE(n)		(((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
+#define CCR_ABMODE_NONE		(0 << 14)
+#define CCR_ABMODE_1		(1 << 14)
+#define CCR_ABMODE_2		(2 << 14)
+#define CCR_ABMODE_4		(3 << 14)
+#define CCR_ABSIZE_8		(0 << 16)
+#define CCR_ABSIZE_16		(1 << 16)
+#define CCR_ABSIZE_24		(2 << 16)
+#define CCR_ABSIZE_32		(3 << 16)
+#define CCR_DCYC_SHIFT		18
+#define CCR_DCYC_MASK		GENMASK(22, 18)
+#define CCR_DCYC(n)		(((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
+#define CCR_DMODE_NONE		(0 << 24)
+#define CCR_DMODE_1		(1 << 24)
+#define CCR_DMODE_2		(2 << 24)
+#define CCR_DMODE_4		(3 << 24)
+#define CCR_FMODE_INDW		(0 << 26)
+#define CCR_FMODE_INDR		(1 << 26)
+#define CCR_FMODE_APM		(2 << 26)
+#define CCR_FMODE_MM		(3 << 26)
+
+#define QUADSPI_AR		0x18
+#define QUADSPI_ABR		0x1c
+#define QUADSPI_DR		0x20
+#define QUADSPI_PSMKR		0x24
+#define QUADSPI_PSMAR		0x28
+#define QUADSPI_PIR		0x2c
+#define QUADSPI_LPTR		0x30
+#define LPTR_DFT_TIMEOUT	0x10
+
+#define STM32_MAX_MMAP_SZ	SZ_256M
+#define STM32_MAX_NORCHIP	2
+
+#define STM32_QSPI_FIFO_TIMEOUT_US 30000
+#define STM32_QSPI_BUSY_TIMEOUT_US 100000
+
+struct stm32_qspi_flash {
+	struct spi_nor nor;
+	u32 cs;
+	u32 fsize;
+	u32 presc;
+	struct stm32_qspi *qspi;
+};
+
+struct stm32_qspi {
+	struct device *dev;
+	void __iomem *io_base;
+	void __iomem *mm_base;
+	u32 nor_num;
+	struct clk *clk;
+	u32 clk_rate;
+	struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
+	u32 read_mode;
+	struct completion cmd_completion;
+
+	/*
+	 * to protect device configuration, could be different between
+	 * 2 flash access (bk1, bk2)
+	 */
+	struct mutex lock;
+};
+
+struct stm32_qspi_cmd {
+	struct {
+		u8 addr_width;
+		u8 dummy;
+		u8 data;
+	} conf;
+
+	u8 opcode;
+	u32 framemode;
+	u32 qspimode;
+	u32 addr;
+	size_t len;
+	void *buf;
+};
+
+static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
+{
+	u32 cr;
+	int err = 0;
+
+	if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
+		return 0;
+
+	reinit_completion(&qspi->cmd_completion);
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
+
+	if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
+						       msecs_to_jiffies(1000)))
+		err = -ETIMEDOUT;
+
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+	return err;
+}
+
+static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
+{
+	u32 sr;
+
+	return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
+					  !(sr & SR_BUSY), 10,
+					  STM32_QSPI_BUSY_TIMEOUT_US);
+}
+
+static void stm32_qspi_set_framemode(struct spi_nor *nor,
+				     struct stm32_qspi_cmd *cmd, bool read)
+{
+	u32 dmode = CCR_DMODE_1;
+
+	cmd->framemode = CCR_IMODE_1;
+
+	if (read) {
+		switch (nor->flash_read) {
+		case SPI_NOR_NORMAL:
+		case SPI_NOR_FAST:
+			dmode = CCR_DMODE_1;
+			break;
+		case SPI_NOR_DUAL:
+			dmode = CCR_DMODE_2;
+			break;
+		case SPI_NOR_QUAD:
+			dmode = CCR_DMODE_4;
+			break;
+		}
+	}
+
+	cmd->framemode |= cmd->conf.data ? dmode : 0;
+	cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
+}
+
+static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
+{
+	*val = readb_relaxed(addr);
+}
+
+static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
+{
+	writeb_relaxed(*val, addr);
+}
+
+static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
+			      const struct stm32_qspi_cmd *cmd)
+{
+	void (*tx_fifo)(u8 *, void __iomem *);
+	u32 len = cmd->len, sr;
+	u8 *buf = cmd->buf;
+	int ret;
+
+	if (cmd->qspimode == CCR_FMODE_INDW)
+		tx_fifo = stm32_qspi_write_fifo;
+	else
+		tx_fifo = stm32_qspi_read_fifo;
+
+	while (len--) {
+		ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
+						 sr, (sr & SR_FTF),10,
+						 STM32_QSPI_FIFO_TIMEOUT_US);
+		if (ret) {
+			dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
+			break;
+		}
+		tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
+	}
+
+	return ret;
+}
+
+static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
+			    const struct stm32_qspi_cmd *cmd)
+{
+	memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
+	return 0;
+}
+
+static int stm32_qspi_tx(struct stm32_qspi *qspi,
+			 const struct stm32_qspi_cmd *cmd)
+{
+	if (!cmd->conf.data)
+		return 0;
+
+	if (cmd->qspimode == CCR_FMODE_MM)
+		return stm32_qspi_tx_mm(qspi, cmd);
+
+	return stm32_qspi_tx_poll(qspi, cmd);
+}
+
+static int stm32_qspi_send(struct stm32_qspi_flash *flash,
+			   const struct stm32_qspi_cmd *cmd)
+{
+	struct stm32_qspi *qspi = flash->qspi;
+	u32 ccr, dcr, cr;
+	int err;
+
+	err = stm32_qspi_wait_nobusy(qspi);
+	if (err)
+		goto abort;
+
+	dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
+	dcr |= DCR_FSIZE(flash->fsize);
+	writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
+
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	cr &= ~CR_PRESC_MASK & ~CR_FSEL;
+	cr |= CR_PRESC(flash->presc);
+	cr |= flash->cs ? CR_FSEL : 0;
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+	if (cmd->conf.data)
+		writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
+
+	ccr = cmd->framemode | cmd->qspimode;
+
+	if (cmd->conf.dummy)
+		ccr |= CCR_DCYC(cmd->conf.dummy);
+
+	if (cmd->conf.addr_width)
+		ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
+
+	ccr |= CCR_INST(cmd->opcode);
+	writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
+
+	if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
+		writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
+
+	err = stm32_qspi_tx(qspi, cmd);
+	if (err)
+		goto abort;
+
+	if (cmd->qspimode != CCR_FMODE_MM) {
+		err = stm32_qspi_wait_cmd(qspi);
+		if (err)
+			goto abort;
+		writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
+	}
+
+	return err;
+
+abort:
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+	dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
+	return err;
+}
+
+static int stm32_qspi_read_reg(struct spi_nor *nor,
+			       u8 opcode, u8 *buf, int len)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "read_reg: cmd:%.2x buf:%p len:%d\n", opcode, buf, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = opcode;
+	cmd.conf.data = 1;
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = CCR_FMODE_INDR;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
+				u8 *buf, int len)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "write_reg: cmd:%.2x buf:%p len:%d\n", opcode, buf, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = opcode;
+	cmd.conf.data = (buf && len > 0);
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
+			       u_char *buf)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+	struct stm32_qspi_cmd cmd;
+	int err;
+
+	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
+		nor->read_opcode, buf, (u32)from, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->read_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)from;
+	cmd.conf.data = 1;
+	cmd.conf.dummy = nor->read_dummy;
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = qspi->read_mode;
+
+	stm32_qspi_set_framemode(nor, &cmd, true);
+	err = stm32_qspi_send(flash, &cmd);
+
+	return err ? err : len;
+}
+
+static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+				const u_char *buf)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+	int err;
+
+	dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
+		nor->program_opcode, buf, (u32)to, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->program_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)to;
+	cmd.conf.data = 1;
+	cmd.len = len;
+	cmd.buf = (void *)buf;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+	err = stm32_qspi_send(flash, &cmd);
+
+	return err ? err : len;
+}
+
+static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->erase_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)offs;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
+{
+	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
+	u32 cr, sr, fcr = 0;
+
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
+
+	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
+		/* tx complete */
+		fcr |= FCR_CTCF;
+		complete(&qspi->cmd_completion);
+	} else {
+		dev_info(qspi->dev, "spurious interrupt\n");
+	}
+
+	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
+
+	return IRQ_HANDLED;
+}
+
+static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+
+	mutex_lock(&qspi->lock);
+	return 0;
+}
+
+static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+
+	mutex_unlock(&qspi->lock);
+}
+
+static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
+				  struct device_node *np)
+{
+	u32 width, flash_read, presc, cs_num, max_rate = 0;
+	struct stm32_qspi_flash *flash;
+	struct mtd_info *mtd;
+	int ret;
+
+	of_property_read_u32(np, "reg", &cs_num);
+	if (cs_num >= STM32_MAX_NORCHIP)
+		return -EINVAL;
+
+	of_property_read_u32(np, "spi-max-frequency", &max_rate);
+	if (!max_rate)
+		return -EINVAL;
+
+	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
+
+	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
+		width = 1;
+
+	if (width == 4)
+		flash_read = SPI_NOR_QUAD;
+	else if (width == 2)
+		flash_read = SPI_NOR_DUAL;
+	else if (width == 1)
+		flash_read = SPI_NOR_NORMAL;
+	else
+		return -EINVAL;
+
+	flash = &qspi->flash[cs_num];
+	flash->qspi = qspi;
+	flash->cs = cs_num;
+	flash->presc = presc;
+
+	flash->nor.dev = qspi->dev;
+	spi_nor_set_flash_node(&flash->nor, np);
+	flash->nor.priv = flash;
+	mtd = &flash->nor.mtd;
+	mtd->priv = &flash->nor;
+
+	flash->nor.read = stm32_qspi_read;
+	flash->nor.write = stm32_qspi_write;
+	flash->nor.erase = stm32_qspi_erase;
+	flash->nor.read_reg = stm32_qspi_read_reg;
+	flash->nor.write_reg = stm32_qspi_write_reg;
+	flash->nor.prepare = stm32_qspi_prep;
+	flash->nor.unprepare = stm32_qspi_unprep;
+
+	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
+
+	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
+		       | CR_EN, qspi->io_base + QUADSPI_CR);
+
+	/*
+	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
+	 * which define the size of nor flash.
+	 * if fsize is NULL, the controller can't sent spi-nor command.
+	 * set a temporary value just to discover the nor flash with
+	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
+	 */
+	flash->fsize = 25;
+
+	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
+	if (ret) {
+		dev_err(qspi->dev, "device scan failed\n");
+		return ret;
+	}
+
+	/* number of bytes in Flash memory = 2^[FSIZE+1] */
+	flash->fsize = __fls(mtd->size) - 1;
+
+	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		dev_err(qspi->dev, "mtd device parse failed\n");
+		return ret;
+	}
+
+	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
+		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
+
+	return 0;
+}
+
+static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
+{
+	int i;
+
+	for (i = 0; i < STM32_MAX_NORCHIP; i++) {
+		if (qspi->flash[i].qspi)
+			mtd_device_unregister(&qspi->flash[i].nor.mtd);
+	}
+}
+
+static int stm32_qspi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *flash_np;
+	struct reset_control *rstc;
+	struct stm32_qspi *qspi;
+	struct resource *res;
+	int ret, irq;
+
+	qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
+	if (!qspi)
+		return -ENOMEM;
+
+	qspi->nor_num = of_get_child_count(dev->of_node);
+	if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
+		return -ENODEV;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
+	qspi->io_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(qspi->io_base))
+		return PTR_ERR(qspi->io_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
+	if (res && resource_size(res) <= STM32_MAX_MMAP_SZ)
+		qspi->mm_base = devm_ioremap_resource(dev, res);
+
+	qspi->read_mode = IS_ERR_OR_NULL(qspi->mm_base) ?
+		CCR_FMODE_INDR : CCR_FMODE_MM;
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
+			       dev_name(dev), qspi);
+	if (ret) {
+		dev_err(dev, "failed to request irq\n");
+		return ret;
+	}
+
+	init_completion(&qspi->cmd_completion);
+
+	qspi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(qspi->clk))
+		return PTR_ERR(qspi->clk);
+
+	qspi->clk_rate = clk_get_rate(qspi->clk);
+	if (!qspi->clk_rate)
+		return -EINVAL;
+
+	ret = clk_prepare_enable(qspi->clk);
+	if (ret) {
+		dev_err(dev, "can not enable the clock\n");
+		return ret;
+	}
+
+	rstc = devm_reset_control_get(dev, NULL);
+	if (!IS_ERR(rstc)) {
+		reset_control_assert(rstc);
+		udelay(2);
+		reset_control_deassert(rstc);
+	}
+
+	qspi->dev = dev;
+	platform_set_drvdata(pdev, qspi);
+	mutex_init(&qspi->lock);
+
+	for_each_available_child_of_node(dev->of_node, flash_np) {
+		ret = stm32_qspi_flash_setup(qspi, flash_np);
+		if (ret) {
+			dev_err(dev, "unable to setup flash chip\n");
+			goto err_flash;
+		}
+	}
+
+	return 0;
+
+err_flash:
+	mutex_destroy(&qspi->lock);
+	stm32_qspi_mtd_free(qspi);
+
+	clk_disable_unprepare(qspi->clk);
+	return ret;
+}
+
+static int stm32_qspi_remove(struct platform_device *pdev)
+{
+	struct stm32_qspi *qspi = platform_get_drvdata(pdev);
+
+	/* disable qspi */
+	writel_relaxed(0, qspi->io_base + QUADSPI_CR);
+
+	stm32_qspi_mtd_free(qspi);
+	mutex_destroy(&qspi->lock);
+
+	clk_disable_unprepare(qspi->clk);
+	return 0;
+}
+
+static const struct of_device_id stm32_qspi_match[] = {
+	{.compatible = "st,stm32f469-qspi"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, stm32_qspi_match);
+
+static struct platform_driver stm32_qspi_driver = {
+	.probe	= stm32_qspi_probe,
+	.remove	= stm32_qspi_remove,
+	.driver	= {
+		.name = "stm32-quadspi",
+		.of_match_table = stm32_qspi_match,
+	},
+};
+module_platform_driver(stm32_qspi_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic Barre]

From: Ludovic Barre <ludovic.barre@st.com>

The quadspi is a specialized communication interface targeting single,
dual or quad SPI Flash memories.

It can operate in any of the following modes:
-indirect mode: all the operations are performed using the quadspi
 registers
-read memory-mapped mode: the external Flash memory is mapped to the
 microcontroller address space and is seen by the system as if it was
 an internal memory

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 drivers/mtd/spi-nor/Kconfig         |   7 +
 drivers/mtd/spi-nor/Makefile        |   1 +
 drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
 3 files changed, 698 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 7252087..bfdfb1e 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -106,4 +106,11 @@ config SPI_INTEL_SPI_PLATFORM
 	  To compile this driver as a module, choose M here: the module
 	  will be called intel-spi-platform.
 
+config SPI_STM32_QUADSPI
+	tristate "STM32 Quad SPI controller"
+	depends on ARCH_STM32
+	help
+	  This enables support for the STM32 Quad SPI controller.
+	  We only connect the NOR to this controller.
+
 endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index 72238a7..285aab8 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
 obj-$(CONFIG_SPI_INTEL_SPI)	+= intel-spi.o
 obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM)	+= intel-spi-platform.o
+obj-$(CONFIG_SPI_STM32_QUADSPI)	+= stm32-quadspi.o
\ No newline at end of file
diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c
new file mode 100644
index 0000000..7d26e37
--- /dev/null
+++ b/drivers/mtd/spi-nor/stm32-quadspi.c
@@ -0,0 +1,690 @@
+/*
+ * stm32_quadspi.c
+ *
+ * Copyright (C) 2017, Ludovic Barre
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#define QUADSPI_CR		0x00
+#define CR_EN			BIT(0)
+#define CR_ABORT		BIT(1)
+#define CR_DMAEN		BIT(2)
+#define CR_TCEN			BIT(3)
+#define CR_SSHIFT		BIT(4)
+#define CR_DFM			BIT(6)
+#define CR_FSEL			BIT(7)
+#define CR_FTHRES_SHIFT		8
+#define CR_FTHRES_MASK		GENMASK(12, 8)
+#define CR_FTHRES(n)		(((n) << CR_FTHRES_SHIFT) & CR_FTHRES_MASK)
+#define CR_TEIE			BIT(16)
+#define CR_TCIE			BIT(17)
+#define CR_FTIE			BIT(18)
+#define CR_SMIE			BIT(19)
+#define CR_TOIE			BIT(20)
+#define CR_PRESC_SHIFT		24
+#define CR_PRESC_MASK		GENMASK(31, 24)
+#define CR_PRESC(n)		(((n) << CR_PRESC_SHIFT) & CR_PRESC_MASK)
+
+#define QUADSPI_DCR		0x04
+#define DCR_CSHT_SHIFT		8
+#define DCR_CSHT_MASK		GENMASK(10, 8)
+#define DCR_CSHT(n)		(((n) << DCR_CSHT_SHIFT) & DCR_CSHT_MASK)
+#define DCR_FSIZE_SHIFT		16
+#define DCR_FSIZE_MASK		GENMASK(20, 16)
+#define DCR_FSIZE(n)		(((n) << DCR_FSIZE_SHIFT) & DCR_FSIZE_MASK)
+
+#define QUADSPI_SR		0x08
+#define SR_TEF			BIT(0)
+#define SR_TCF			BIT(1)
+#define SR_FTF			BIT(2)
+#define SR_SMF			BIT(3)
+#define SR_TOF			BIT(4)
+#define SR_BUSY			BIT(5)
+#define SR_FLEVEL_SHIFT		8
+#define SR_FLEVEL_MASK		GENMASK(13, 8)
+
+#define QUADSPI_FCR		0x0c
+#define FCR_CTCF		BIT(1)
+
+#define QUADSPI_DLR		0x10
+
+#define QUADSPI_CCR		0x14
+#define CCR_INST_SHIFT		0
+#define CCR_INST_MASK		GENMASK(7, 0)
+#define CCR_INST(n)		(((n) << CCR_INST_SHIFT) & CCR_INST_MASK)
+#define CCR_IMODE_NONE		(0 << 8)
+#define CCR_IMODE_1		(1 << 8)
+#define CCR_IMODE_2		(2 << 8)
+#define CCR_IMODE_4		(3 << 8)
+#define CCR_ADMODE_NONE		(0 << 10)
+#define CCR_ADMODE_1		(1 << 10)
+#define CCR_ADMODE_2		(2 << 10)
+#define CCR_ADMODE_4		(3 << 10)
+#define CCR_ADSIZE_SHIFT	12
+#define CCR_ADSIZE_MASK		GENMASK(13, 12)
+#define CCR_ADSIZE(n)		(((n) << CCR_ADSIZE_SHIFT) & CCR_ADSIZE_MASK)
+#define CCR_ABMODE_NONE		(0 << 14)
+#define CCR_ABMODE_1		(1 << 14)
+#define CCR_ABMODE_2		(2 << 14)
+#define CCR_ABMODE_4		(3 << 14)
+#define CCR_ABSIZE_8		(0 << 16)
+#define CCR_ABSIZE_16		(1 << 16)
+#define CCR_ABSIZE_24		(2 << 16)
+#define CCR_ABSIZE_32		(3 << 16)
+#define CCR_DCYC_SHIFT		18
+#define CCR_DCYC_MASK		GENMASK(22, 18)
+#define CCR_DCYC(n)		(((n) << CCR_DCYC_SHIFT) & CCR_DCYC_MASK)
+#define CCR_DMODE_NONE		(0 << 24)
+#define CCR_DMODE_1		(1 << 24)
+#define CCR_DMODE_2		(2 << 24)
+#define CCR_DMODE_4		(3 << 24)
+#define CCR_FMODE_INDW		(0 << 26)
+#define CCR_FMODE_INDR		(1 << 26)
+#define CCR_FMODE_APM		(2 << 26)
+#define CCR_FMODE_MM		(3 << 26)
+
+#define QUADSPI_AR		0x18
+#define QUADSPI_ABR		0x1c
+#define QUADSPI_DR		0x20
+#define QUADSPI_PSMKR		0x24
+#define QUADSPI_PSMAR		0x28
+#define QUADSPI_PIR		0x2c
+#define QUADSPI_LPTR		0x30
+#define LPTR_DFT_TIMEOUT	0x10
+
+#define STM32_MAX_MMAP_SZ	SZ_256M
+#define STM32_MAX_NORCHIP	2
+
+#define STM32_QSPI_FIFO_TIMEOUT_US 30000
+#define STM32_QSPI_BUSY_TIMEOUT_US 100000
+
+struct stm32_qspi_flash {
+	struct spi_nor nor;
+	u32 cs;
+	u32 fsize;
+	u32 presc;
+	struct stm32_qspi *qspi;
+};
+
+struct stm32_qspi {
+	struct device *dev;
+	void __iomem *io_base;
+	void __iomem *mm_base;
+	u32 nor_num;
+	struct clk *clk;
+	u32 clk_rate;
+	struct stm32_qspi_flash flash[STM32_MAX_NORCHIP];
+	u32 read_mode;
+	struct completion cmd_completion;
+
+	/*
+	 * to protect device configuration, could be different between
+	 * 2 flash access (bk1, bk2)
+	 */
+	struct mutex lock;
+};
+
+struct stm32_qspi_cmd {
+	struct {
+		u8 addr_width;
+		u8 dummy;
+		u8 data;
+	} conf;
+
+	u8 opcode;
+	u32 framemode;
+	u32 qspimode;
+	u32 addr;
+	size_t len;
+	void *buf;
+};
+
+static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
+{
+	u32 cr;
+	int err = 0;
+
+	if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
+		return 0;
+
+	reinit_completion(&qspi->cmd_completion);
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
+
+	if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
+						       msecs_to_jiffies(1000)))
+		err = -ETIMEDOUT;
+
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+	return err;
+}
+
+static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
+{
+	u32 sr;
+
+	return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
+					  !(sr & SR_BUSY), 10,
+					  STM32_QSPI_BUSY_TIMEOUT_US);
+}
+
+static void stm32_qspi_set_framemode(struct spi_nor *nor,
+				     struct stm32_qspi_cmd *cmd, bool read)
+{
+	u32 dmode = CCR_DMODE_1;
+
+	cmd->framemode = CCR_IMODE_1;
+
+	if (read) {
+		switch (nor->flash_read) {
+		case SPI_NOR_NORMAL:
+		case SPI_NOR_FAST:
+			dmode = CCR_DMODE_1;
+			break;
+		case SPI_NOR_DUAL:
+			dmode = CCR_DMODE_2;
+			break;
+		case SPI_NOR_QUAD:
+			dmode = CCR_DMODE_4;
+			break;
+		}
+	}
+
+	cmd->framemode |= cmd->conf.data ? dmode : 0;
+	cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
+}
+
+static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
+{
+	*val = readb_relaxed(addr);
+}
+
+static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
+{
+	writeb_relaxed(*val, addr);
+}
+
+static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
+			      const struct stm32_qspi_cmd *cmd)
+{
+	void (*tx_fifo)(u8 *, void __iomem *);
+	u32 len = cmd->len, sr;
+	u8 *buf = cmd->buf;
+	int ret;
+
+	if (cmd->qspimode == CCR_FMODE_INDW)
+		tx_fifo = stm32_qspi_write_fifo;
+	else
+		tx_fifo = stm32_qspi_read_fifo;
+
+	while (len--) {
+		ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
+						 sr, (sr & SR_FTF),10,
+						 STM32_QSPI_FIFO_TIMEOUT_US);
+		if (ret) {
+			dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
+			break;
+		}
+		tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
+	}
+
+	return ret;
+}
+
+static int stm32_qspi_tx_mm(struct stm32_qspi *qspi,
+			    const struct stm32_qspi_cmd *cmd)
+{
+	memcpy_fromio(cmd->buf, qspi->mm_base + cmd->addr, cmd->len);
+	return 0;
+}
+
+static int stm32_qspi_tx(struct stm32_qspi *qspi,
+			 const struct stm32_qspi_cmd *cmd)
+{
+	if (!cmd->conf.data)
+		return 0;
+
+	if (cmd->qspimode == CCR_FMODE_MM)
+		return stm32_qspi_tx_mm(qspi, cmd);
+
+	return stm32_qspi_tx_poll(qspi, cmd);
+}
+
+static int stm32_qspi_send(struct stm32_qspi_flash *flash,
+			   const struct stm32_qspi_cmd *cmd)
+{
+	struct stm32_qspi *qspi = flash->qspi;
+	u32 ccr, dcr, cr;
+	int err;
+
+	err = stm32_qspi_wait_nobusy(qspi);
+	if (err)
+		goto abort;
+
+	dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
+	dcr |= DCR_FSIZE(flash->fsize);
+	writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
+
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	cr &= ~CR_PRESC_MASK & ~CR_FSEL;
+	cr |= CR_PRESC(flash->presc);
+	cr |= flash->cs ? CR_FSEL : 0;
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+	if (cmd->conf.data)
+		writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
+
+	ccr = cmd->framemode | cmd->qspimode;
+
+	if (cmd->conf.dummy)
+		ccr |= CCR_DCYC(cmd->conf.dummy);
+
+	if (cmd->conf.addr_width)
+		ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
+
+	ccr |= CCR_INST(cmd->opcode);
+	writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
+
+	if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
+		writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
+
+	err = stm32_qspi_tx(qspi, cmd);
+	if (err)
+		goto abort;
+
+	if (cmd->qspimode != CCR_FMODE_MM) {
+		err = stm32_qspi_wait_cmd(qspi);
+		if (err)
+			goto abort;
+		writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
+	}
+
+	return err;
+
+abort:
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR) | CR_ABORT;
+	writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
+
+	dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
+	return err;
+}
+
+static int stm32_qspi_read_reg(struct spi_nor *nor,
+			       u8 opcode, u8 *buf, int len)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "read_reg: cmd:%.2x buf:%p len:%d\n", opcode, buf, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = opcode;
+	cmd.conf.data = 1;
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = CCR_FMODE_INDR;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static int stm32_qspi_write_reg(struct spi_nor *nor, u8 opcode,
+				u8 *buf, int len)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "write_reg: cmd:%.2x buf:%p len:%d\n", opcode, buf, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = opcode;
+	cmd.conf.data = (buf && len > 0);
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
+			       u_char *buf)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+	struct stm32_qspi_cmd cmd;
+	int err;
+
+	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
+		nor->read_opcode, buf, (u32)from, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->read_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)from;
+	cmd.conf.data = 1;
+	cmd.conf.dummy = nor->read_dummy;
+	cmd.len = len;
+	cmd.buf = buf;
+	cmd.qspimode = qspi->read_mode;
+
+	stm32_qspi_set_framemode(nor, &cmd, true);
+	err = stm32_qspi_send(flash, &cmd);
+
+	return err ? err : len;
+}
+
+static ssize_t stm32_qspi_write(struct spi_nor *nor, loff_t to, size_t len,
+				const u_char *buf)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+	int err;
+
+	dev_dbg(dev, "write(%#.2x): buf:%p to:%#.8x len:%#x\n",
+		nor->program_opcode, buf, (u32)to, len);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->program_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)to;
+	cmd.conf.data = 1;
+	cmd.len = len;
+	cmd.buf = (void *)buf;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+	err = stm32_qspi_send(flash, &cmd);
+
+	return err ? err : len;
+}
+
+static int stm32_qspi_erase(struct spi_nor *nor, loff_t offs)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct device *dev = flash->qspi->dev;
+	struct stm32_qspi_cmd cmd;
+
+	dev_dbg(dev, "erase(%#.2x):offs:%#x\n", nor->erase_opcode, (u32)offs);
+
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.opcode = nor->erase_opcode;
+	cmd.conf.addr_width = nor->addr_width;
+	cmd.addr = (u32)offs;
+	cmd.qspimode = CCR_FMODE_INDW;
+
+	stm32_qspi_set_framemode(nor, &cmd, false);
+
+	return stm32_qspi_send(flash, &cmd);
+}
+
+static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
+{
+	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
+	u32 cr, sr, fcr = 0;
+
+	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
+	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
+
+	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
+		/* tx complete */
+		fcr |= FCR_CTCF;
+		complete(&qspi->cmd_completion);
+	} else {
+		dev_info(qspi->dev, "spurious interrupt\n");
+	}
+
+	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
+
+	return IRQ_HANDLED;
+}
+
+static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+
+	mutex_lock(&qspi->lock);
+	return 0;
+}
+
+static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
+{
+	struct stm32_qspi_flash *flash = nor->priv;
+	struct stm32_qspi *qspi = flash->qspi;
+
+	mutex_unlock(&qspi->lock);
+}
+
+static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
+				  struct device_node *np)
+{
+	u32 width, flash_read, presc, cs_num, max_rate = 0;
+	struct stm32_qspi_flash *flash;
+	struct mtd_info *mtd;
+	int ret;
+
+	of_property_read_u32(np, "reg", &cs_num);
+	if (cs_num >= STM32_MAX_NORCHIP)
+		return -EINVAL;
+
+	of_property_read_u32(np, "spi-max-frequency", &max_rate);
+	if (!max_rate)
+		return -EINVAL;
+
+	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
+
+	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
+		width = 1;
+
+	if (width == 4)
+		flash_read = SPI_NOR_QUAD;
+	else if (width == 2)
+		flash_read = SPI_NOR_DUAL;
+	else if (width == 1)
+		flash_read = SPI_NOR_NORMAL;
+	else
+		return -EINVAL;
+
+	flash = &qspi->flash[cs_num];
+	flash->qspi = qspi;
+	flash->cs = cs_num;
+	flash->presc = presc;
+
+	flash->nor.dev = qspi->dev;
+	spi_nor_set_flash_node(&flash->nor, np);
+	flash->nor.priv = flash;
+	mtd = &flash->nor.mtd;
+	mtd->priv = &flash->nor;
+
+	flash->nor.read = stm32_qspi_read;
+	flash->nor.write = stm32_qspi_write;
+	flash->nor.erase = stm32_qspi_erase;
+	flash->nor.read_reg = stm32_qspi_read_reg;
+	flash->nor.write_reg = stm32_qspi_write_reg;
+	flash->nor.prepare = stm32_qspi_prep;
+	flash->nor.unprepare = stm32_qspi_unprep;
+
+	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
+
+	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
+		       | CR_EN, qspi->io_base + QUADSPI_CR);
+
+	/*
+	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
+	 * which define the size of nor flash.
+	 * if fsize is NULL, the controller can't sent spi-nor command.
+	 * set a temporary value just to discover the nor flash with
+	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
+	 */
+	flash->fsize = 25;
+
+	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
+	if (ret) {
+		dev_err(qspi->dev, "device scan failed\n");
+		return ret;
+	}
+
+	/* number of bytes in Flash memory = 2^[FSIZE+1] */
+	flash->fsize = __fls(mtd->size) - 1;
+
+	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
+
+	ret = mtd_device_register(mtd, NULL, 0);
+	if (ret) {
+		dev_err(qspi->dev, "mtd device parse failed\n");
+		return ret;
+	}
+
+	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
+		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
+
+	return 0;
+}
+
+static void stm32_qspi_mtd_free(struct stm32_qspi *qspi)
+{
+	int i;
+
+	for (i = 0; i < STM32_MAX_NORCHIP; i++) {
+		if (qspi->flash[i].qspi)
+			mtd_device_unregister(&qspi->flash[i].nor.mtd);
+	}
+}
+
+static int stm32_qspi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *flash_np;
+	struct reset_control *rstc;
+	struct stm32_qspi *qspi;
+	struct resource *res;
+	int ret, irq;
+
+	qspi = devm_kzalloc(dev, sizeof(*qspi), GFP_KERNEL);
+	if (!qspi)
+		return -ENOMEM;
+
+	qspi->nor_num = of_get_child_count(dev->of_node);
+	if (!qspi->nor_num || qspi->nor_num > STM32_MAX_NORCHIP)
+		return -ENODEV;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi");
+	qspi->io_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(qspi->io_base))
+		return PTR_ERR(qspi->io_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm");
+	if (res && resource_size(res) <= STM32_MAX_MMAP_SZ)
+		qspi->mm_base = devm_ioremap_resource(dev, res);
+
+	qspi->read_mode = IS_ERR_OR_NULL(qspi->mm_base) ?
+		CCR_FMODE_INDR : CCR_FMODE_MM;
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0,
+			       dev_name(dev), qspi);
+	if (ret) {
+		dev_err(dev, "failed to request irq\n");
+		return ret;
+	}
+
+	init_completion(&qspi->cmd_completion);
+
+	qspi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(qspi->clk))
+		return PTR_ERR(qspi->clk);
+
+	qspi->clk_rate = clk_get_rate(qspi->clk);
+	if (!qspi->clk_rate)
+		return -EINVAL;
+
+	ret = clk_prepare_enable(qspi->clk);
+	if (ret) {
+		dev_err(dev, "can not enable the clock\n");
+		return ret;
+	}
+
+	rstc = devm_reset_control_get(dev, NULL);
+	if (!IS_ERR(rstc)) {
+		reset_control_assert(rstc);
+		udelay(2);
+		reset_control_deassert(rstc);
+	}
+
+	qspi->dev = dev;
+	platform_set_drvdata(pdev, qspi);
+	mutex_init(&qspi->lock);
+
+	for_each_available_child_of_node(dev->of_node, flash_np) {
+		ret = stm32_qspi_flash_setup(qspi, flash_np);
+		if (ret) {
+			dev_err(dev, "unable to setup flash chip\n");
+			goto err_flash;
+		}
+	}
+
+	return 0;
+
+err_flash:
+	mutex_destroy(&qspi->lock);
+	stm32_qspi_mtd_free(qspi);
+
+	clk_disable_unprepare(qspi->clk);
+	return ret;
+}
+
+static int stm32_qspi_remove(struct platform_device *pdev)
+{
+	struct stm32_qspi *qspi = platform_get_drvdata(pdev);
+
+	/* disable qspi */
+	writel_relaxed(0, qspi->io_base + QUADSPI_CR);
+
+	stm32_qspi_mtd_free(qspi);
+	mutex_destroy(&qspi->lock);
+
+	clk_disable_unprepare(qspi->clk);
+	return 0;
+}
+
+static const struct of_device_id stm32_qspi_match[] = {
+	{.compatible = "st,stm32f469-qspi"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, stm32_qspi_match);
+
+static struct platform_driver stm32_qspi_driver = {
+	.probe	= stm32_qspi_probe,
+	.remove	= stm32_qspi_remove,
+	.driver	= {
+		.name = "stm32-quadspi",
+		.of_match_table = stm32_qspi_match,
+	},
+};
+module_platform_driver(stm32_qspi_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_AUTHOR("Ludovic Barre <ludovic.barre@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 quad spi driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Rob Herring]

On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
> 
> This patch adds documentation of device tree bindings for the STM32
> QSPI controller.
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
> new file mode 100644
> index 0000000..95a8ebd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
> @@ -0,0 +1,45 @@
> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
> +
> +Required properties:
> +- compatible: should be "st,stm32f469-qspi"
> +- reg: contains the register location and length.
> +  (optional) the memory mapping address and length

Why optional? Either the h/w has it or doesn't. If some chips don't, 
they should have a different compatible string.

> +- reg-names: list of the names corresponding to the previous register
> +  Should contain "qspi" to register location
> +  (optional) "qspi_mm" if read in memory map mode (improve read throughput)
> +- interrupts: should contain the interrupt for the device
> +- clocks: the phandle of the clock needed by the QSPI controller
> +- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
> +
> +Optional properties:
> +- resets: must contain the phandle to the reset controller.
> +
> +A spi flash must be a child of the nor_flash node and could have some
> +properties. Also see jedec,spi-nor.txt.
> +
> +Required properties:
> +- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
> +- spi-max-frequency: max frequency of spi bus
> +
> +Optional property:
> +- spi-rx-bus-width: the bus width (number of data wires)

Just "see ../spi/spi-bus.txt" for the description

> +
> +Example:
> +
> +qspi: qspi@a0001000 {

spi@...

> +	compatible = "st,stm32f469-qspi";
> +	reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
> +	reg-names = "qspi", "qspi_mm";
> +	interrupts = <91>;
> +	resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
> +	clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_qspi0>;
> +
> +	flash@0 {
> +		reg = <0>;
> +		spi-rx-bus-width = <4>;
> +		spi-max-frequency = <108000000>;
> +		...
> +	};
> +};
> -- 
> 2.7.4
> 

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Ludovic BARRE]

Hi Rob

thanks for review
my comments below

br
Ludo

On 04/03/2017 06:57 PM, Rob Herring wrote:
> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch adds documentation of device tree bindings for the STM32
>> QSPI controller.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>>   .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45 ++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>> new file mode 100644
>> index 0000000..95a8ebd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>> @@ -0,0 +1,45 @@
>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>> +
>> +Required properties:
>> +- compatible: should be "st,stm32f469-qspi"
>> +- reg: contains the register location and length.
>> +  (optional) the memory mapping address and length
> Why optional? Either the h/w has it or doesn't. If some chips don't,
> they should have a different compatible string.
in fact, the stm32 qspi controller can operate in any of the following 
modes:
-indirect mode: all the operations are performed using the qspi registers
with read/write.
-read memory-mapped mode: the external Flash memory is mapped to the
  microcontroller address space and is seen by the system as if it was
  an internal memory (use memcpy_fromio). this mode improve read throughput

if qspi_mm is defined the qspi controller use read memory-mapped mode
else the controller transfers in indirect mode.
>> +- reg-names: list of the names corresponding to the previous register
>> +  Should contain "qspi" to register location
>> +  (optional) "qspi_mm" if read in memory map mode (improve read throughput)
>> +- interrupts: should contain the interrupt for the device
>> +- clocks: the phandle of the clock needed by the QSPI controller
>> +- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
>> +
>> +Optional properties:
>> +- resets: must contain the phandle to the reset controller.
>> +
>> +A spi flash must be a child of the nor_flash node and could have some
>> +properties. Also see jedec,spi-nor.txt.
>> +
>> +Required properties:
>> +- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
>> +- spi-max-frequency: max frequency of spi bus
>> +
>> +Optional property:
>> +- spi-rx-bus-width: the bus width (number of data wires)
> Just "see ../spi/spi-bus.txt" for the description
ok
>
>> +
>> +Example:
>> +
>> +qspi: qspi@a0001000 {
> spi@...
ok
>> +	compatible = "st,stm32f469-qspi";
>> +	reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
>> +	reg-names = "qspi", "qspi_mm";
>> +	interrupts = <91>;
>> +	resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
>> +	clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_qspi0>;
>> +
>> +	flash@0 {
>> +		reg = <0>;
>> +		spi-rx-bus-width = <4>;
>> +		spi-max-frequency = <108000000>;
>> +		...
>> +	};
>> +};
>> -- 
>> 2.7.4
>>

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Ludovic BARRE]

Hi Rob

thanks for review
my comments below

br
Ludo

On 04/03/2017 06:57 PM, Rob Herring wrote:
> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>
>> This patch adds documentation of device tree bindings for the STM32
>> QSPI controller.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>> ---
>>   .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45 ++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>> new file mode 100644
>> index 0000000..95a8ebd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>> @@ -0,0 +1,45 @@
>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>> +
>> +Required properties:
>> +- compatible: should be "st,stm32f469-qspi"
>> +- reg: contains the register location and length.
>> +  (optional) the memory mapping address and length
> Why optional? Either the h/w has it or doesn't. If some chips don't,
> they should have a different compatible string.
in fact, the stm32 qspi controller can operate in any of the following 
modes:
-indirect mode: all the operations are performed using the qspi registers
with read/write.
-read memory-mapped mode: the external Flash memory is mapped to the
  microcontroller address space and is seen by the system as if it was
  an internal memory (use memcpy_fromio). this mode improve read throughput

if qspi_mm is defined the qspi controller use read memory-mapped mode
else the controller transfers in indirect mode.
>> +- reg-names: list of the names corresponding to the previous register
>> +  Should contain "qspi" to register location
>> +  (optional) "qspi_mm" if read in memory map mode (improve read throughput)
>> +- interrupts: should contain the interrupt for the device
>> +- clocks: the phandle of the clock needed by the QSPI controller
>> +- A pinctrl must be defined to set pins in mode of operation for QSPI transfer
>> +
>> +Optional properties:
>> +- resets: must contain the phandle to the reset controller.
>> +
>> +A spi flash must be a child of the nor_flash node and could have some
>> +properties. Also see jedec,spi-nor.txt.
>> +
>> +Required properties:
>> +- reg: chip-Select number (QSPI controller may connect 2 nor flashes)
>> +- spi-max-frequency: max frequency of spi bus
>> +
>> +Optional property:
>> +- spi-rx-bus-width: the bus width (number of data wires)
> Just "see ../spi/spi-bus.txt" for the description
ok
>
>> +
>> +Example:
>> +
>> +qspi: qspi@a0001000 {
> spi@...
ok
>> +	compatible = "st,stm32f469-qspi";
>> +	reg = <0xa0001000 0x1000>, <0x90000000 0x10000000>;
>> +	reg-names = "qspi", "qspi_mm";
>> +	interrupts = <91>;
>> +	resets = <&rcc STM32F4_AHB3_RESET(QSPI)>;
>> +	clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>;
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&pinctrl_qspi0>;
>> +
>> +	flash@0 {
>> +		reg = <0>;
>> +		spi-rx-bus-width = <4>;
>> +		spi-max-frequency = <108000000>;
>> +		...
>> +	};
>> +};
>> -- 
>> 2.7.4
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Rob Herring]

On Tue, Apr 4, 2017 at 2:28 AM, Ludovic BARRE <ludovic.barre@st.com> wrote:
> Hi Rob
>
> thanks for review
> my comments below
>
> br
> Ludo
>
> On 04/03/2017 06:57 PM, Rob Herring wrote:
>>
>> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>>>
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> This patch adds documentation of device tree bindings for the STM32
>>> QSPI controller.
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>> ---
>>>   .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45
>>> ++++++++++++++++++++++
>>>   1 file changed, 45 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> new file mode 100644
>>> index 0000000..95a8ebd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> @@ -0,0 +1,45 @@
>>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>>> +
>>> +Required properties:
>>> +- compatible: should be "st,stm32f469-qspi"
>>> +- reg: contains the register location and length.
>>> +  (optional) the memory mapping address and length
>>
>> Why optional? Either the h/w has it or doesn't. If some chips don't,
>> they should have a different compatible string.
>
> in fact, the stm32 qspi controller can operate in any of the following
> modes:
> -indirect mode: all the operations are performed using the qspi registers
> with read/write.
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory (use memcpy_fromio). this mode improve read throughput
>
> if qspi_mm is defined the qspi controller use read memory-mapped mode
> else the controller transfers in indirect mode.

You should always have the memory region defined because that's what
the h/w has. If you want another property to select the mode, then
perhaps that's fine. But why? Can't the OS figure out which to use?
Why would you ever not use memory mapped mode unless the driver
doesn't yet support it?

Rob

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Rob Herring]

On Tue, Apr 4, 2017 at 2:28 AM, Ludovic BARRE <ludovic.barre-qxv4g6HH51o@public.gmane.org> wrote:
> Hi Rob
>
> thanks for review
> my comments below
>
> br
> Ludo
>
> On 04/03/2017 06:57 PM, Rob Herring wrote:
>>
>> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>>>
>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>
>>> This patch adds documentation of device tree bindings for the STM32
>>> QSPI controller.
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>> ---
>>>   .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45
>>> ++++++++++++++++++++++
>>>   1 file changed, 45 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> new file mode 100644
>>> index 0000000..95a8ebd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>> @@ -0,0 +1,45 @@
>>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>>> +
>>> +Required properties:
>>> +- compatible: should be "st,stm32f469-qspi"
>>> +- reg: contains the register location and length.
>>> +  (optional) the memory mapping address and length
>>
>> Why optional? Either the h/w has it or doesn't. If some chips don't,
>> they should have a different compatible string.
>
> in fact, the stm32 qspi controller can operate in any of the following
> modes:
> -indirect mode: all the operations are performed using the qspi registers
> with read/write.
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory (use memcpy_fromio). this mode improve read throughput
>
> if qspi_mm is defined the qspi controller use read memory-mapped mode
> else the controller transfers in indirect mode.

You should always have the memory region defined because that's what
the h/w has. If you want another property to select the mode, then
perhaps that's fine. But why? Can't the OS figure out which to use?
Why would you ever not use memory mapped mode unless the driver
doesn't yet support it?

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] dt-bindings: Document the STM32 QSPI bindings

[Ludovic BARRE]

On 04/04/2017 02:20 PM, Rob Herring wrote:
> On Tue, Apr 4, 2017 at 2:28 AM, Ludovic BARRE <ludovic.barre@st.com> wrote:
>> Hi Rob
>>
>> thanks for review
>> my comments below
>>
>> br
>> Ludo
>>
>> On 04/03/2017 06:57 PM, Rob Herring wrote:
>>> On Fri, Mar 31, 2017 at 07:02:03PM +0200, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch adds documentation of device tree bindings for the STM32
>>>> QSPI controller.
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>>    .../devicetree/bindings/mtd/stm32-quadspi.txt      | 45
>>>> ++++++++++++++++++++++
>>>>    1 file changed, 45 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> new file mode 100644
>>>> index 0000000..95a8ebd
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt
>>>> @@ -0,0 +1,45 @@
>>>> +* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI)
>>>> +
>>>> +Required properties:
>>>> +- compatible: should be "st,stm32f469-qspi"
>>>> +- reg: contains the register location and length.
>>>> +  (optional) the memory mapping address and length
>>> Why optional? Either the h/w has it or doesn't. If some chips don't,
>>> they should have a different compatible string.
>> in fact, the stm32 qspi controller can operate in any of the following
>> modes:
>> -indirect mode: all the operations are performed using the qspi registers
>> with read/write.
>> -read memory-mapped mode: the external Flash memory is mapped to the
>>   microcontroller address space and is seen by the system as if it was
>>   an internal memory (use memcpy_fromio). this mode improve read throughput
>>
>> if qspi_mm is defined the qspi controller use read memory-mapped mode
>> else the controller transfers in indirect mode.
> You should always have the memory region defined because that's what
> the h/w has. If you want another property to select the mode, then
> perhaps that's fine. But why? Can't the OS figure out which to use?
> Why would you ever not use memory mapped mode unless the driver
> doesn't yet support it?
ok, I always map the memory region (qspi_mm is now required).
if the nor-flash is more bigger than "qspi memory region", I force to use
the indirect mode.
> Rob

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Marek Vasut]

On 03/31/2017 07:02 PM, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre@st.com>
> 
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
> 
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
>  registers
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory
> 
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
>  drivers/mtd/spi-nor/Kconfig         |   7 +
>  drivers/mtd/spi-nor/Makefile        |   1 +
>  drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 698 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
> 

[...]

> +struct stm32_qspi_flash {
> +	struct spi_nor nor;
> +	u32 cs;
> +	u32 fsize;
> +	u32 presc;
> +	struct stm32_qspi *qspi;
> +};

[...]

> +struct stm32_qspi_cmd {
> +	struct {
> +		u8 addr_width;
> +		u8 dummy;
> +		u8 data;
> +	} conf;

Is there any benefit in having this structure here or could you just
make the struct stm32_qspi_cmd flat ?

> +	u8 opcode;
> +	u32 framemode;
> +	u32 qspimode;
> +	u32 addr;
> +	size_t len;
> +	void *buf;
> +};

[...]

> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
> +			       u_char *buf)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +	struct stm32_qspi_cmd cmd;
> +	int err;
> +
> +	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
> +		nor->read_opcode, buf, (u32)from, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = nor->read_opcode;
> +	cmd.conf.addr_width = nor->addr_width;
> +	cmd.addr = (u32)from;

loff_t (from) can be 64bit ... how do we handle this ?

> +	cmd.conf.data = 1;
> +	cmd.conf.dummy = nor->read_dummy;
> +	cmd.len = len;
> +	cmd.buf = buf;
> +	cmd.qspimode = qspi->read_mode;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, true);
> +	err = stm32_qspi_send(flash, &cmd);
> +
> +	return err ? err : len;
> +}

[...]

> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
> +{
> +	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
> +	u32 cr, sr, fcr = 0;
> +
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> +	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
> +
> +	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
> +		/* tx complete */
> +		fcr |= FCR_CTCF;
> +		complete(&qspi->cmd_completion);
> +	} else {
> +		dev_info(qspi->dev, "spurious interrupt\n");

You probably want to ratelimit this one ...

> +	}
> +
> +	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_lock(&qspi->lock);
> +	return 0;
> +}
> +
> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_unlock(&qspi->lock);
> +}
> +
> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
> +				  struct device_node *np)
> +{
> +	u32 width, flash_read, presc, cs_num, max_rate = 0;
> +	struct stm32_qspi_flash *flash;
> +	struct mtd_info *mtd;
> +	int ret;
> +
> +	of_property_read_u32(np, "reg", &cs_num);
> +	if (cs_num >= STM32_MAX_NORCHIP)
> +		return -EINVAL;
> +
> +	of_property_read_u32(np, "spi-max-frequency", &max_rate);
> +	if (!max_rate)
> +		return -EINVAL;
> +
> +	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
> +
> +	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
> +		width = 1;
> +
> +	if (width == 4)
> +		flash_read = SPI_NOR_QUAD;
> +	else if (width == 2)
> +		flash_read = SPI_NOR_DUAL;
> +	else if (width == 1)
> +		flash_read = SPI_NOR_NORMAL;
> +	else
> +		return -EINVAL;
> +
> +	flash = &qspi->flash[cs_num];
> +	flash->qspi = qspi;
> +	flash->cs = cs_num;
> +	flash->presc = presc;
> +
> +	flash->nor.dev = qspi->dev;
> +	spi_nor_set_flash_node(&flash->nor, np);
> +	flash->nor.priv = flash;
> +	mtd = &flash->nor.mtd;
> +	mtd->priv = &flash->nor;
> +
> +	flash->nor.read = stm32_qspi_read;
> +	flash->nor.write = stm32_qspi_write;
> +	flash->nor.erase = stm32_qspi_erase;
> +	flash->nor.read_reg = stm32_qspi_read_reg;
> +	flash->nor.write_reg = stm32_qspi_write_reg;
> +	flash->nor.prepare = stm32_qspi_prep;
> +	flash->nor.unprepare = stm32_qspi_unprep;
> +
> +	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> +	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> +		       | CR_EN, qspi->io_base + QUADSPI_CR);
> +
> +	/*
> +	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> +	 * which define the size of nor flash.
> +	 * if fsize is NULL, the controller can't sent spi-nor command.
> +	 * set a temporary value just to discover the nor flash with
> +	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
> +	 */

Is 25 the smallest value ? Use a macro for this ...

> +	flash->fsize = 25;
> +
> +	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
> +	if (ret) {
> +		dev_err(qspi->dev, "device scan failed\n");
> +		return ret;
> +	}
> +
> +	/* number of bytes in Flash memory = 2^[FSIZE+1] */
> +	flash->fsize = __fls(mtd->size) - 1;
> +
> +	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> +	ret = mtd_device_register(mtd, NULL, 0);
> +	if (ret) {
> +		dev_err(qspi->dev, "mtd device parse failed\n");
> +		return ret;
> +	}
> +
> +	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> +		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> +	return 0;
> +}

[...]

-- 
Best regards,
Marek Vasut

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Marek Vasut]

On 03/31/2017 07:02 PM, Ludovic Barre wrote:
> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> 
> The quadspi is a specialized communication interface targeting single,
> dual or quad SPI Flash memories.
> 
> It can operate in any of the following modes:
> -indirect mode: all the operations are performed using the quadspi
>  registers
> -read memory-mapped mode: the external Flash memory is mapped to the
>  microcontroller address space and is seen by the system as if it was
>  an internal memory
> 
> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/mtd/spi-nor/Kconfig         |   7 +
>  drivers/mtd/spi-nor/Makefile        |   1 +
>  drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 698 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
> 

[...]

> +struct stm32_qspi_flash {
> +	struct spi_nor nor;
> +	u32 cs;
> +	u32 fsize;
> +	u32 presc;
> +	struct stm32_qspi *qspi;
> +};

[...]

> +struct stm32_qspi_cmd {
> +	struct {
> +		u8 addr_width;
> +		u8 dummy;
> +		u8 data;
> +	} conf;

Is there any benefit in having this structure here or could you just
make the struct stm32_qspi_cmd flat ?

> +	u8 opcode;
> +	u32 framemode;
> +	u32 qspimode;
> +	u32 addr;
> +	size_t len;
> +	void *buf;
> +};

[...]

> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
> +			       u_char *buf)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +	struct stm32_qspi_cmd cmd;
> +	int err;
> +
> +	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
> +		nor->read_opcode, buf, (u32)from, len);
> +
> +	memset(&cmd, 0, sizeof(cmd));
> +	cmd.opcode = nor->read_opcode;
> +	cmd.conf.addr_width = nor->addr_width;
> +	cmd.addr = (u32)from;

loff_t (from) can be 64bit ... how do we handle this ?

> +	cmd.conf.data = 1;
> +	cmd.conf.dummy = nor->read_dummy;
> +	cmd.len = len;
> +	cmd.buf = buf;
> +	cmd.qspimode = qspi->read_mode;
> +
> +	stm32_qspi_set_framemode(nor, &cmd, true);
> +	err = stm32_qspi_send(flash, &cmd);
> +
> +	return err ? err : len;
> +}

[...]

> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
> +{
> +	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
> +	u32 cr, sr, fcr = 0;
> +
> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
> +	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
> +
> +	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
> +		/* tx complete */
> +		fcr |= FCR_CTCF;
> +		complete(&qspi->cmd_completion);
> +	} else {
> +		dev_info(qspi->dev, "spurious interrupt\n");

You probably want to ratelimit this one ...

> +	}
> +
> +	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_lock(&qspi->lock);
> +	return 0;
> +}
> +
> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> +	struct stm32_qspi_flash *flash = nor->priv;
> +	struct stm32_qspi *qspi = flash->qspi;
> +
> +	mutex_unlock(&qspi->lock);
> +}
> +
> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
> +				  struct device_node *np)
> +{
> +	u32 width, flash_read, presc, cs_num, max_rate = 0;
> +	struct stm32_qspi_flash *flash;
> +	struct mtd_info *mtd;
> +	int ret;
> +
> +	of_property_read_u32(np, "reg", &cs_num);
> +	if (cs_num >= STM32_MAX_NORCHIP)
> +		return -EINVAL;
> +
> +	of_property_read_u32(np, "spi-max-frequency", &max_rate);
> +	if (!max_rate)
> +		return -EINVAL;
> +
> +	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
> +
> +	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
> +		width = 1;
> +
> +	if (width == 4)
> +		flash_read = SPI_NOR_QUAD;
> +	else if (width == 2)
> +		flash_read = SPI_NOR_DUAL;
> +	else if (width == 1)
> +		flash_read = SPI_NOR_NORMAL;
> +	else
> +		return -EINVAL;
> +
> +	flash = &qspi->flash[cs_num];
> +	flash->qspi = qspi;
> +	flash->cs = cs_num;
> +	flash->presc = presc;
> +
> +	flash->nor.dev = qspi->dev;
> +	spi_nor_set_flash_node(&flash->nor, np);
> +	flash->nor.priv = flash;
> +	mtd = &flash->nor.mtd;
> +	mtd->priv = &flash->nor;
> +
> +	flash->nor.read = stm32_qspi_read;
> +	flash->nor.write = stm32_qspi_write;
> +	flash->nor.erase = stm32_qspi_erase;
> +	flash->nor.read_reg = stm32_qspi_read_reg;
> +	flash->nor.write_reg = stm32_qspi_write_reg;
> +	flash->nor.prepare = stm32_qspi_prep;
> +	flash->nor.unprepare = stm32_qspi_unprep;
> +
> +	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> +	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> +		       | CR_EN, qspi->io_base + QUADSPI_CR);
> +
> +	/*
> +	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> +	 * which define the size of nor flash.
> +	 * if fsize is NULL, the controller can't sent spi-nor command.
> +	 * set a temporary value just to discover the nor flash with
> +	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
> +	 */

Is 25 the smallest value ? Use a macro for this ...

> +	flash->fsize = 25;
> +
> +	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
> +	if (ret) {
> +		dev_err(qspi->dev, "device scan failed\n");
> +		return ret;
> +	}
> +
> +	/* number of bytes in Flash memory = 2^[FSIZE+1] */
> +	flash->fsize = __fls(mtd->size) - 1;
> +
> +	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> +	ret = mtd_device_register(mtd, NULL, 0);
> +	if (ret) {
> +		dev_err(qspi->dev, "mtd device parse failed\n");
> +		return ret;
> +	}
> +
> +	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> +		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> +	return 0;
> +}

[...]

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic BARRE]

On 04/07/2017 01:55 AM, Marek Vasut wrote:
> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> The quadspi is a specialized communication interface targeting single,
>> dual or quad SPI Flash memories.
>>
>> It can operate in any of the following modes:
>> -indirect mode: all the operations are performed using the quadspi
>>   registers
>> -read memory-mapped mode: the external Flash memory is mapped to the
>>   microcontroller address space and is seen by the system as if it was
>>   an internal memory
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>>   drivers/mtd/spi-nor/Kconfig         |   7 +
>>   drivers/mtd/spi-nor/Makefile        |   1 +
>>   drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 698 insertions(+)
>>   create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>
> [...]
>
>> +struct stm32_qspi_flash {
>> +	struct spi_nor nor;
>> +	u32 cs;
>> +	u32 fsize;
>> +	u32 presc;
>> +	struct stm32_qspi *qspi;
>> +};
> [...]
>
>> +struct stm32_qspi_cmd {
>> +	struct {
>> +		u8 addr_width;
>> +		u8 dummy;
>> +		u8 data;
>> +	} conf;
> Is there any benefit in having this structure here or could you just
> make the struct stm32_qspi_cmd flat ?
no benefit, it was just to regroup,  so I can do a flat structure
>
>> +	u8 opcode;
>> +	u32 framemode;
>> +	u32 qspimode;
>> +	u32 addr;
>> +	size_t len;
>> +	void *buf;
>> +};
> [...]
>
>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
>> +			       u_char *buf)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +	struct stm32_qspi_cmd cmd;
>> +	int err;
>> +
>> +	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>> +		nor->read_opcode, buf, (u32)from, len);
>> +
>> +	memset(&cmd, 0, sizeof(cmd));
>> +	cmd.opcode = nor->read_opcode;
>> +	cmd.conf.addr_width = nor->addr_width;
>> +	cmd.addr = (u32)from;
> loff_t (from) can be 64bit ... how do we handle this ?
I'm surprise by the question,
the SPI NOR device uses 3 Bytes or 4 bytes address mode.
So, the stm32 qspi controller has a 32 bit register for NOR address.
On the other hand the framework and other drivers used this variable 
(from) like
a 32 bits.
>
>> +	cmd.conf.data = 1;
>> +	cmd.conf.dummy = nor->read_dummy;
>> +	cmd.len = len;
>> +	cmd.buf = buf;
>> +	cmd.qspimode = qspi->read_mode;
>> +
>> +	stm32_qspi_set_framemode(nor, &cmd, true);
>> +	err = stm32_qspi_send(flash, &cmd);
>> +
>> +	return err ? err : len;
>> +}
> [...]
>
>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>> +{
>> +	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>> +	u32 cr, sr, fcr = 0;
>> +
>> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>> +	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>> +
>> +	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>> +		/* tx complete */
>> +		fcr |= FCR_CTCF;
>> +		complete(&qspi->cmd_completion);
>> +	} else {
>> +		dev_info(qspi->dev, "spurious interrupt\n");
> You probably want to ratelimit this one ...
yes it's better if there is an issue.
>
>> +	}
>> +
>> +	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +
>> +	mutex_lock(&qspi->lock);
>> +	return 0;
>> +}
>> +
>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +
>> +	mutex_unlock(&qspi->lock);
>> +}
>> +
>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>> +				  struct device_node *np)
>> +{
>> +	u32 width, flash_read, presc, cs_num, max_rate = 0;
>> +	struct stm32_qspi_flash *flash;
>> +	struct mtd_info *mtd;
>> +	int ret;
>> +
>> +	of_property_read_u32(np, "reg", &cs_num);
>> +	if (cs_num >= STM32_MAX_NORCHIP)
>> +		return -EINVAL;
>> +
>> +	of_property_read_u32(np, "spi-max-frequency", &max_rate);
>> +	if (!max_rate)
>> +		return -EINVAL;
>> +
>> +	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>> +
>> +	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>> +		width = 1;
>> +
>> +	if (width == 4)
>> +		flash_read = SPI_NOR_QUAD;
>> +	else if (width == 2)
>> +		flash_read = SPI_NOR_DUAL;
>> +	else if (width == 1)
>> +		flash_read = SPI_NOR_NORMAL;
>> +	else
>> +		return -EINVAL;
>> +
>> +	flash = &qspi->flash[cs_num];
>> +	flash->qspi = qspi;
>> +	flash->cs = cs_num;
>> +	flash->presc = presc;
>> +
>> +	flash->nor.dev = qspi->dev;
>> +	spi_nor_set_flash_node(&flash->nor, np);
>> +	flash->nor.priv = flash;
>> +	mtd = &flash->nor.mtd;
>> +	mtd->priv = &flash->nor;
>> +
>> +	flash->nor.read = stm32_qspi_read;
>> +	flash->nor.write = stm32_qspi_write;
>> +	flash->nor.erase = stm32_qspi_erase;
>> +	flash->nor.read_reg = stm32_qspi_read_reg;
>> +	flash->nor.write_reg = stm32_qspi_write_reg;
>> +	flash->nor.prepare = stm32_qspi_prep;
>> +	flash->nor.unprepare = stm32_qspi_unprep;
>> +
>> +	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>> +
>> +	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>> +		       | CR_EN, qspi->io_base + QUADSPI_CR);
>> +
>> +	/*
>> +	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>> +	 * which define the size of nor flash.
>> +	 * if fsize is NULL, the controller can't sent spi-nor command.
>> +	 * set a temporary value just to discover the nor flash with
>> +	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
>> +	 */
> Is 25 the smallest value ? Use a macro for this ...
25 is an arbitrary choice, I will define a smallest value
>
>> +	flash->fsize = 25;
>> +
>> +	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>> +	if (ret) {
>> +		dev_err(qspi->dev, "device scan failed\n");
>> +		return ret;
>> +	}
>> +
>> +	/* number of bytes in Flash memory = 2^[FSIZE+1] */
>> +	flash->fsize = __fls(mtd->size) - 1;
>> +
>> +	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>> +
>> +	ret = mtd_device_register(mtd, NULL, 0);
>> +	if (ret) {
>> +		dev_err(qspi->dev, "mtd device parse failed\n");
>> +		return ret;
>> +	}
>> +
>> +	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>> +		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>> +
>> +	return 0;
>> +}
> [...]
>

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic BARRE]

On 04/07/2017 01:55 AM, Marek Vasut wrote:
> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>
>> The quadspi is a specialized communication interface targeting single,
>> dual or quad SPI Flash memories.
>>
>> It can operate in any of the following modes:
>> -indirect mode: all the operations are performed using the quadspi
>>   registers
>> -read memory-mapped mode: the external Flash memory is mapped to the
>>   microcontroller address space and is seen by the system as if it was
>>   an internal memory
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>> ---
>>   drivers/mtd/spi-nor/Kconfig         |   7 +
>>   drivers/mtd/spi-nor/Makefile        |   1 +
>>   drivers/mtd/spi-nor/stm32-quadspi.c | 690 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 698 insertions(+)
>>   create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>
> [...]
>
>> +struct stm32_qspi_flash {
>> +	struct spi_nor nor;
>> +	u32 cs;
>> +	u32 fsize;
>> +	u32 presc;
>> +	struct stm32_qspi *qspi;
>> +};
> [...]
>
>> +struct stm32_qspi_cmd {
>> +	struct {
>> +		u8 addr_width;
>> +		u8 dummy;
>> +		u8 data;
>> +	} conf;
> Is there any benefit in having this structure here or could you just
> make the struct stm32_qspi_cmd flat ?
no benefit, it was just to regroup,  so I can do a flat structure
>
>> +	u8 opcode;
>> +	u32 framemode;
>> +	u32 qspimode;
>> +	u32 addr;
>> +	size_t len;
>> +	void *buf;
>> +};
> [...]
>
>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from, size_t len,
>> +			       u_char *buf)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +	struct stm32_qspi_cmd cmd;
>> +	int err;
>> +
>> +	dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>> +		nor->read_opcode, buf, (u32)from, len);
>> +
>> +	memset(&cmd, 0, sizeof(cmd));
>> +	cmd.opcode = nor->read_opcode;
>> +	cmd.conf.addr_width = nor->addr_width;
>> +	cmd.addr = (u32)from;
> loff_t (from) can be 64bit ... how do we handle this ?
I'm surprise by the question,
the SPI NOR device uses 3 Bytes or 4 bytes address mode.
So, the stm32 qspi controller has a 32 bit register for NOR address.
On the other hand the framework and other drivers used this variable 
(from) like
a 32 bits.
>
>> +	cmd.conf.data = 1;
>> +	cmd.conf.dummy = nor->read_dummy;
>> +	cmd.len = len;
>> +	cmd.buf = buf;
>> +	cmd.qspimode = qspi->read_mode;
>> +
>> +	stm32_qspi_set_framemode(nor, &cmd, true);
>> +	err = stm32_qspi_send(flash, &cmd);
>> +
>> +	return err ? err : len;
>> +}
> [...]
>
>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>> +{
>> +	struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>> +	u32 cr, sr, fcr = 0;
>> +
>> +	cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>> +	sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>> +
>> +	if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>> +		/* tx complete */
>> +		fcr |= FCR_CTCF;
>> +		complete(&qspi->cmd_completion);
>> +	} else {
>> +		dev_info(qspi->dev, "spurious interrupt\n");
> You probably want to ratelimit this one ...
yes it's better if there is an issue.
>
>> +	}
>> +
>> +	writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +
>> +	mutex_lock(&qspi->lock);
>> +	return 0;
>> +}
>> +
>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> +	struct stm32_qspi_flash *flash = nor->priv;
>> +	struct stm32_qspi *qspi = flash->qspi;
>> +
>> +	mutex_unlock(&qspi->lock);
>> +}
>> +
>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>> +				  struct device_node *np)
>> +{
>> +	u32 width, flash_read, presc, cs_num, max_rate = 0;
>> +	struct stm32_qspi_flash *flash;
>> +	struct mtd_info *mtd;
>> +	int ret;
>> +
>> +	of_property_read_u32(np, "reg", &cs_num);
>> +	if (cs_num >= STM32_MAX_NORCHIP)
>> +		return -EINVAL;
>> +
>> +	of_property_read_u32(np, "spi-max-frequency", &max_rate);
>> +	if (!max_rate)
>> +		return -EINVAL;
>> +
>> +	presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>> +
>> +	if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>> +		width = 1;
>> +
>> +	if (width == 4)
>> +		flash_read = SPI_NOR_QUAD;
>> +	else if (width == 2)
>> +		flash_read = SPI_NOR_DUAL;
>> +	else if (width == 1)
>> +		flash_read = SPI_NOR_NORMAL;
>> +	else
>> +		return -EINVAL;
>> +
>> +	flash = &qspi->flash[cs_num];
>> +	flash->qspi = qspi;
>> +	flash->cs = cs_num;
>> +	flash->presc = presc;
>> +
>> +	flash->nor.dev = qspi->dev;
>> +	spi_nor_set_flash_node(&flash->nor, np);
>> +	flash->nor.priv = flash;
>> +	mtd = &flash->nor.mtd;
>> +	mtd->priv = &flash->nor;
>> +
>> +	flash->nor.read = stm32_qspi_read;
>> +	flash->nor.write = stm32_qspi_write;
>> +	flash->nor.erase = stm32_qspi_erase;
>> +	flash->nor.read_reg = stm32_qspi_read_reg;
>> +	flash->nor.write_reg = stm32_qspi_write_reg;
>> +	flash->nor.prepare = stm32_qspi_prep;
>> +	flash->nor.unprepare = stm32_qspi_unprep;
>> +
>> +	writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>> +
>> +	writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>> +		       | CR_EN, qspi->io_base + QUADSPI_CR);
>> +
>> +	/*
>> +	 * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>> +	 * which define the size of nor flash.
>> +	 * if fsize is NULL, the controller can't sent spi-nor command.
>> +	 * set a temporary value just to discover the nor flash with
>> +	 * "spi_nor_scan". After, the right value (mtd->size) can be set.
>> +	 */
> Is 25 the smallest value ? Use a macro for this ...
25 is an arbitrary choice, I will define a smallest value
>
>> +	flash->fsize = 25;
>> +
>> +	ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>> +	if (ret) {
>> +		dev_err(qspi->dev, "device scan failed\n");
>> +		return ret;
>> +	}
>> +
>> +	/* number of bytes in Flash memory = 2^[FSIZE+1] */
>> +	flash->fsize = __fls(mtd->size) - 1;
>> +
>> +	writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>> +
>> +	ret = mtd_device_register(mtd, NULL, 0);
>> +	if (ret) {
>> +		dev_err(qspi->dev, "mtd device parse failed\n");
>> +		return ret;
>> +	}
>> +
>> +	dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>> +		qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>> +
>> +	return 0;
>> +}
> [...]
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Marek Vasut]

On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
> 
> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> The quadspi is a specialized communication interface targeting single,
>>> dual or quad SPI Flash memories.
>>>
>>> It can operate in any of the following modes:
>>> -indirect mode: all the operations are performed using the quadspi
>>>   registers
>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>   microcontroller address space and is seen by the system as if it was
>>>   an internal memory
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>> ---
>>>   drivers/mtd/spi-nor/Kconfig         |   7 +
>>>   drivers/mtd/spi-nor/Makefile        |   1 +
>>>   drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>> ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 698 insertions(+)
>>>   create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>
>> [...]
>>
>>> +struct stm32_qspi_flash {
>>> +    struct spi_nor nor;
>>> +    u32 cs;
>>> +    u32 fsize;
>>> +    u32 presc;
>>> +    struct stm32_qspi *qspi;
>>> +};
>> [...]
>>
>>> +struct stm32_qspi_cmd {
>>> +    struct {
>>> +        u8 addr_width;
>>> +        u8 dummy;
>>> +        u8 data;
>>> +    } conf;
>> Is there any benefit in having this structure here or could you just
>> make the struct stm32_qspi_cmd flat ?
> no benefit, it was just to regroup,  so I can do a flat structure

Well, as you like, but I think it does make sense to just make it flat.

>>> +    u8 opcode;
>>> +    u32 framemode;
>>> +    u32 qspimode;
>>> +    u32 addr;
>>> +    size_t len;
>>> +    void *buf;
>>> +};
>> [...]
>>
>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>> size_t len,
>>> +                   u_char *buf)
>>> +{
>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>> +    struct stm32_qspi *qspi = flash->qspi;
>>> +    struct stm32_qspi_cmd cmd;
>>> +    int err;
>>> +
>>> +    dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>> +        nor->read_opcode, buf, (u32)from, len);
>>> +
>>> +    memset(&cmd, 0, sizeof(cmd));
>>> +    cmd.opcode = nor->read_opcode;
>>> +    cmd.conf.addr_width = nor->addr_width;
>>> +    cmd.addr = (u32)from;
>> loff_t (from) can be 64bit ... how do we handle this ?
> I'm surprise by the question,
> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
> So, the stm32 qspi controller has a 32 bit register for NOR address.
> On the other hand the framework and other drivers used this variable
> (from) like
> a 32 bits.

Hmmm, (rhetorical question) then why do we even use loff_t in the
framework ?

Anyway, this is no problem then.

>>> +    cmd.conf.data = 1;
>>> +    cmd.conf.dummy = nor->read_dummy;
>>> +    cmd.len = len;
>>> +    cmd.buf = buf;
>>> +    cmd.qspimode = qspi->read_mode;
>>> +
>>> +    stm32_qspi_set_framemode(nor, &cmd, true);
>>> +    err = stm32_qspi_send(flash, &cmd);
>>> +
>>> +    return err ? err : len;
>>> +}
>> [...]
>>
>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>> +{
>>> +    struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>> +    u32 cr, sr, fcr = 0;
>>> +
>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>> +    sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>> +
>>> +    if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>> +        /* tx complete */
>>> +        fcr |= FCR_CTCF;
>>> +        complete(&qspi->cmd_completion);
>>> +    } else {
>>> +        dev_info(qspi->dev, "spurious interrupt\n");
>> You probably want to ratelimit this one ...
> yes it's better if there is an issue.

Yep

>>> +    }
>>> +
>>> +    writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>> +
>>> +    return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>> +{
>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>> +    struct stm32_qspi *qspi = flash->qspi;
>>> +
>>> +    mutex_lock(&qspi->lock);
>>> +    return 0;
>>> +}
>>> +
>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>> ops)
>>> +{
>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>> +    struct stm32_qspi *qspi = flash->qspi;
>>> +
>>> +    mutex_unlock(&qspi->lock);
>>> +}
>>> +
>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>> +                  struct device_node *np)
>>> +{
>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>> +    struct stm32_qspi_flash *flash;
>>> +    struct mtd_info *mtd;
>>> +    int ret;
>>> +
>>> +    of_property_read_u32(np, "reg", &cs_num);
>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>> +        return -EINVAL;
>>> +
>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>> +    if (!max_rate)
>>> +        return -EINVAL;
>>> +
>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>> +
>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>> +        width = 1;
>>> +
>>> +    if (width == 4)
>>> +        flash_read = SPI_NOR_QUAD;
>>> +    else if (width == 2)
>>> +        flash_read = SPI_NOR_DUAL;
>>> +    else if (width == 1)
>>> +        flash_read = SPI_NOR_NORMAL;
>>> +    else
>>> +        return -EINVAL;
>>> +
>>> +    flash = &qspi->flash[cs_num];
>>> +    flash->qspi = qspi;
>>> +    flash->cs = cs_num;
>>> +    flash->presc = presc;
>>> +
>>> +    flash->nor.dev = qspi->dev;
>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>> +    flash->nor.priv = flash;
>>> +    mtd = &flash->nor.mtd;
>>> +    mtd->priv = &flash->nor;
>>> +
>>> +    flash->nor.read = stm32_qspi_read;
>>> +    flash->nor.write = stm32_qspi_write;
>>> +    flash->nor.erase = stm32_qspi_erase;
>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>> +    flash->nor.prepare = stm32_qspi_prep;
>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>> +
>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>> +
>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>> +
>>> +    /*
>>> +     * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>>> +     * which define the size of nor flash.
>>> +     * if fsize is NULL, the controller can't sent spi-nor command.
>>> +     * set a temporary value just to discover the nor flash with
>>> +     * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>> +     */
>> Is 25 the smallest value ? Use a macro for this ...
> 25 is an arbitrary choice, I will define a smallest value

Cool, thanks!

-- 
Best regards,
Marek Vasut

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic BARRE]

hi Marek

tomorrow, I send a v3 with your/Rob reviews.

BR

Ludo


On 04/10/2017 06:15 PM, Marek Vasut wrote:
> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> The quadspi is a specialized communication interface targeting single,
>>>> dual or quad SPI Flash memories.
>>>>
>>>> It can operate in any of the following modes:
>>>> -indirect mode: all the operations are performed using the quadspi
>>>>    registers
>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>>    microcontroller address space and is seen by the system as if it was
>>>>    an internal memory
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>>    drivers/mtd/spi-nor/Kconfig         |   7 +
>>>>    drivers/mtd/spi-nor/Makefile        |   1 +
>>>>    drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>> ++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 698 insertions(+)
>>>>    create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>
>>> [...]
>>>
>>>> +struct stm32_qspi_flash {
>>>> +    struct spi_nor nor;
>>>> +    u32 cs;
>>>> +    u32 fsize;
>>>> +    u32 presc;
>>>> +    struct stm32_qspi *qspi;
>>>> +};
>>> [...]
>>>
>>>> +struct stm32_qspi_cmd {
>>>> +    struct {
>>>> +        u8 addr_width;
>>>> +        u8 dummy;
>>>> +        u8 data;
>>>> +    } conf;
>>> Is there any benefit in having this structure here or could you just
>>> make the struct stm32_qspi_cmd flat ?
>> no benefit, it was just to regroup,  so I can do a flat structure
> Well, as you like, but I think it does make sense to just make it flat.
>
>>>> +    u8 opcode;
>>>> +    u32 framemode;
>>>> +    u32 qspimode;
>>>> +    u32 addr;
>>>> +    size_t len;
>>>> +    void *buf;
>>>> +};
>>> [...]
>>>
>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>> size_t len,
>>>> +                   u_char *buf)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +    struct stm32_qspi_cmd cmd;
>>>> +    int err;
>>>> +
>>>> +    dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>> +        nor->read_opcode, buf, (u32)from, len);
>>>> +
>>>> +    memset(&cmd, 0, sizeof(cmd));
>>>> +    cmd.opcode = nor->read_opcode;
>>>> +    cmd.conf.addr_width = nor->addr_width;
>>>> +    cmd.addr = (u32)from;
>>> loff_t (from) can be 64bit ... how do we handle this ?
>> I'm surprise by the question,
>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>> On the other hand the framework and other drivers used this variable
>> (from) like
>> a 32 bits.
> Hmmm, (rhetorical question) then why do we even use loff_t in the
> framework ?
>
> Anyway, this is no problem then.
In fact, the loff_t 64 bit come from mtd interface
(needed to address biggest device constraint) but not needed for spi-nor 
devices.
>>>> +    cmd.conf.data = 1;
>>>> +    cmd.conf.dummy = nor->read_dummy;
>>>> +    cmd.len = len;
>>>> +    cmd.buf = buf;
>>>> +    cmd.qspimode = qspi->read_mode;
>>>> +
>>>> +    stm32_qspi_set_framemode(nor, &cmd, true);
>>>> +    err = stm32_qspi_send(flash, &cmd);
>>>> +
>>>> +    return err ? err : len;
>>>> +}
>>> [...]
>>>
>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>> +{
>>>> +    struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>> +    u32 cr, sr, fcr = 0;
>>>> +
>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> +    sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>> +
>>>> +    if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>> +        /* tx complete */
>>>> +        fcr |= FCR_CTCF;
>>>> +        complete(&qspi->cmd_completion);
>>>> +    } else {
>>>> +        dev_info(qspi->dev, "spurious interrupt\n");
>>> You probably want to ratelimit this one ...
>> yes it's better if there is an issue.
> Yep
>
>>>> +    }
>>>> +
>>>> +    writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>> +
>>>> +    return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> +    mutex_lock(&qspi->lock);
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>> ops)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> +    mutex_unlock(&qspi->lock);
>>>> +}
>>>> +
>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>> +                  struct device_node *np)
>>>> +{
>>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>> +    struct stm32_qspi_flash *flash;
>>>> +    struct mtd_info *mtd;
>>>> +    int ret;
>>>> +
>>>> +    of_property_read_u32(np, "reg", &cs_num);
>>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>>> +        return -EINVAL;
>>>> +
>>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>> +    if (!max_rate)
>>>> +        return -EINVAL;
>>>> +
>>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>> +
>>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>> +        width = 1;
>>>> +
>>>> +    if (width == 4)
>>>> +        flash_read = SPI_NOR_QUAD;
>>>> +    else if (width == 2)
>>>> +        flash_read = SPI_NOR_DUAL;
>>>> +    else if (width == 1)
>>>> +        flash_read = SPI_NOR_NORMAL;
>>>> +    else
>>>> +        return -EINVAL;
>>>> +
>>>> +    flash = &qspi->flash[cs_num];
>>>> +    flash->qspi = qspi;
>>>> +    flash->cs = cs_num;
>>>> +    flash->presc = presc;
>>>> +
>>>> +    flash->nor.dev = qspi->dev;
>>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>>> +    flash->nor.priv = flash;
>>>> +    mtd = &flash->nor.mtd;
>>>> +    mtd->priv = &flash->nor;
>>>> +
>>>> +    flash->nor.read = stm32_qspi_read;
>>>> +    flash->nor.write = stm32_qspi_write;
>>>> +    flash->nor.erase = stm32_qspi_erase;
>>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>>> +    flash->nor.prepare = stm32_qspi_prep;
>>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>>> +
>>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>> +
>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> +    /*
>>>> +     * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>>>> +     * which define the size of nor flash.
>>>> +     * if fsize is NULL, the controller can't sent spi-nor command.
>>>> +     * set a temporary value just to discover the nor flash with
>>>> +     * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>> +     */
>>> Is 25 the smallest value ? Use a macro for this ...
>> 25 is an arbitrary choice, I will define a smallest value
> Cool, thanks!
>

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Ludovic BARRE]

hi Marek

tomorrow, I send a v3 with your/Rob reviews.

BR

Ludo


On 04/10/2017 06:15 PM, Marek Vasut wrote:
> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> The quadspi is a specialized communication interface targeting single,
>>>> dual or quad SPI Flash memories.
>>>>
>>>> It can operate in any of the following modes:
>>>> -indirect mode: all the operations are performed using the quadspi
>>>>    registers
>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>>    microcontroller address space and is seen by the system as if it was
>>>>    an internal memory
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>>    drivers/mtd/spi-nor/Kconfig         |   7 +
>>>>    drivers/mtd/spi-nor/Makefile        |   1 +
>>>>    drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>> ++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 698 insertions(+)
>>>>    create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>
>>> [...]
>>>
>>>> +struct stm32_qspi_flash {
>>>> +    struct spi_nor nor;
>>>> +    u32 cs;
>>>> +    u32 fsize;
>>>> +    u32 presc;
>>>> +    struct stm32_qspi *qspi;
>>>> +};
>>> [...]
>>>
>>>> +struct stm32_qspi_cmd {
>>>> +    struct {
>>>> +        u8 addr_width;
>>>> +        u8 dummy;
>>>> +        u8 data;
>>>> +    } conf;
>>> Is there any benefit in having this structure here or could you just
>>> make the struct stm32_qspi_cmd flat ?
>> no benefit, it was just to regroup,  so I can do a flat structure
> Well, as you like, but I think it does make sense to just make it flat.
>
>>>> +    u8 opcode;
>>>> +    u32 framemode;
>>>> +    u32 qspimode;
>>>> +    u32 addr;
>>>> +    size_t len;
>>>> +    void *buf;
>>>> +};
>>> [...]
>>>
>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>> size_t len,
>>>> +                   u_char *buf)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +    struct stm32_qspi_cmd cmd;
>>>> +    int err;
>>>> +
>>>> +    dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>> +        nor->read_opcode, buf, (u32)from, len);
>>>> +
>>>> +    memset(&cmd, 0, sizeof(cmd));
>>>> +    cmd.opcode = nor->read_opcode;
>>>> +    cmd.conf.addr_width = nor->addr_width;
>>>> +    cmd.addr = (u32)from;
>>> loff_t (from) can be 64bit ... how do we handle this ?
>> I'm surprise by the question,
>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>> On the other hand the framework and other drivers used this variable
>> (from) like
>> a 32 bits.
> Hmmm, (rhetorical question) then why do we even use loff_t in the
> framework ?
>
> Anyway, this is no problem then.
In fact, the loff_t 64 bit come from mtd interface
(needed to address biggest device constraint) but not needed for spi-nor 
devices.
>>>> +    cmd.conf.data = 1;
>>>> +    cmd.conf.dummy = nor->read_dummy;
>>>> +    cmd.len = len;
>>>> +    cmd.buf = buf;
>>>> +    cmd.qspimode = qspi->read_mode;
>>>> +
>>>> +    stm32_qspi_set_framemode(nor, &cmd, true);
>>>> +    err = stm32_qspi_send(flash, &cmd);
>>>> +
>>>> +    return err ? err : len;
>>>> +}
>>> [...]
>>>
>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>> +{
>>>> +    struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>> +    u32 cr, sr, fcr = 0;
>>>> +
>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> +    sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>> +
>>>> +    if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>> +        /* tx complete */
>>>> +        fcr |= FCR_CTCF;
>>>> +        complete(&qspi->cmd_completion);
>>>> +    } else {
>>>> +        dev_info(qspi->dev, "spurious interrupt\n");
>>> You probably want to ratelimit this one ...
>> yes it's better if there is an issue.
> Yep
>
>>>> +    }
>>>> +
>>>> +    writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>> +
>>>> +    return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> +    mutex_lock(&qspi->lock);
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>> ops)
>>>> +{
>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> +    mutex_unlock(&qspi->lock);
>>>> +}
>>>> +
>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>> +                  struct device_node *np)
>>>> +{
>>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>> +    struct stm32_qspi_flash *flash;
>>>> +    struct mtd_info *mtd;
>>>> +    int ret;
>>>> +
>>>> +    of_property_read_u32(np, "reg", &cs_num);
>>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>>> +        return -EINVAL;
>>>> +
>>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>> +    if (!max_rate)
>>>> +        return -EINVAL;
>>>> +
>>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>> +
>>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>> +        width = 1;
>>>> +
>>>> +    if (width == 4)
>>>> +        flash_read = SPI_NOR_QUAD;
>>>> +    else if (width == 2)
>>>> +        flash_read = SPI_NOR_DUAL;
>>>> +    else if (width == 1)
>>>> +        flash_read = SPI_NOR_NORMAL;
>>>> +    else
>>>> +        return -EINVAL;
>>>> +
>>>> +    flash = &qspi->flash[cs_num];
>>>> +    flash->qspi = qspi;
>>>> +    flash->cs = cs_num;
>>>> +    flash->presc = presc;
>>>> +
>>>> +    flash->nor.dev = qspi->dev;
>>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>>> +    flash->nor.priv = flash;
>>>> +    mtd = &flash->nor.mtd;
>>>> +    mtd->priv = &flash->nor;
>>>> +
>>>> +    flash->nor.read = stm32_qspi_read;
>>>> +    flash->nor.write = stm32_qspi_write;
>>>> +    flash->nor.erase = stm32_qspi_erase;
>>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>>> +    flash->nor.prepare = stm32_qspi_prep;
>>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>>> +
>>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>> +
>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> +    /*
>>>> +     * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>>>> +     * which define the size of nor flash.
>>>> +     * if fsize is NULL, the controller can't sent spi-nor command.
>>>> +     * set a temporary value just to discover the nor flash with
>>>> +     * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>> +     */
>>> Is 25 the smallest value ? Use a macro for this ...
>> 25 is an arbitrary choice, I will define a smallest value
> Cool, thanks!
>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Marek Vasut]

On 04/10/2017 06:52 PM, Ludovic BARRE wrote:
> hi Marek
> 
> tomorrow, I send a v3 with your/Rob reviews.

Super, thanks! I'll be pretty busy till Friday, so please keep in mind
the final review might take a bit.

> BR
> 
> Ludo
> 
> 
> On 04/10/2017 06:15 PM, Marek Vasut wrote:
>> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>>
>>>>> The quadspi is a specialized communication interface targeting single,
>>>>> dual or quad SPI Flash memories.
>>>>>
>>>>> It can operate in any of the following modes:
>>>>> -indirect mode: all the operations are performed using the quadspi
>>>>>    registers
>>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>>>    microcontroller address space and is seen by the system as if it
>>>>> was
>>>>>    an internal memory
>>>>>
>>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>>> ---
>>>>>    drivers/mtd/spi-nor/Kconfig         |   7 +
>>>>>    drivers/mtd/spi-nor/Makefile        |   1 +
>>>>>    drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 698 insertions(+)
>>>>>    create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>>
>>>> [...]
>>>>
>>>>> +struct stm32_qspi_flash {
>>>>> +    struct spi_nor nor;
>>>>> +    u32 cs;
>>>>> +    u32 fsize;
>>>>> +    u32 presc;
>>>>> +    struct stm32_qspi *qspi;
>>>>> +};
>>>> [...]
>>>>
>>>>> +struct stm32_qspi_cmd {
>>>>> +    struct {
>>>>> +        u8 addr_width;
>>>>> +        u8 dummy;
>>>>> +        u8 data;
>>>>> +    } conf;
>>>> Is there any benefit in having this structure here or could you just
>>>> make the struct stm32_qspi_cmd flat ?
>>> no benefit, it was just to regroup,  so I can do a flat structure
>> Well, as you like, but I think it does make sense to just make it flat.
>>
>>>>> +    u8 opcode;
>>>>> +    u32 framemode;
>>>>> +    u32 qspimode;
>>>>> +    u32 addr;
>>>>> +    size_t len;
>>>>> +    void *buf;
>>>>> +};
>>>> [...]
>>>>
>>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>>> size_t len,
>>>>> +                   u_char *buf)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +    struct stm32_qspi_cmd cmd;
>>>>> +    int err;
>>>>> +
>>>>> +    dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>>> +        nor->read_opcode, buf, (u32)from, len);
>>>>> +
>>>>> +    memset(&cmd, 0, sizeof(cmd));
>>>>> +    cmd.opcode = nor->read_opcode;
>>>>> +    cmd.conf.addr_width = nor->addr_width;
>>>>> +    cmd.addr = (u32)from;
>>>> loff_t (from) can be 64bit ... how do we handle this ?
>>> I'm surprise by the question,
>>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>>> On the other hand the framework and other drivers used this variable
>>> (from) like
>>> a 32 bits.
>> Hmmm, (rhetorical question) then why do we even use loff_t in the
>> framework ?
>>
>> Anyway, this is no problem then.
> In fact, the loff_t 64 bit come from mtd interface
> (needed to address biggest device constraint) but not needed for spi-nor
> devices.
>>>>> +    cmd.conf.data = 1;
>>>>> +    cmd.conf.dummy = nor->read_dummy;
>>>>> +    cmd.len = len;
>>>>> +    cmd.buf = buf;
>>>>> +    cmd.qspimode = qspi->read_mode;
>>>>> +
>>>>> +    stm32_qspi_set_framemode(nor, &cmd, true);
>>>>> +    err = stm32_qspi_send(flash, &cmd);
>>>>> +
>>>>> +    return err ? err : len;
>>>>> +}
>>>> [...]
>>>>
>>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>>> +{
>>>>> +    struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>>> +    u32 cr, sr, fcr = 0;
>>>>> +
>>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>>> +    sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>>> +
>>>>> +    if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>>> +        /* tx complete */
>>>>> +        fcr |= FCR_CTCF;
>>>>> +        complete(&qspi->cmd_completion);
>>>>> +    } else {
>>>>> +        dev_info(qspi->dev, "spurious interrupt\n");
>>>> You probably want to ratelimit this one ...
>>> yes it's better if there is an issue.
>> Yep
>>
>>>>> +    }
>>>>> +
>>>>> +    writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>>> +
>>>>> +    return IRQ_HANDLED;
>>>>> +}
>>>>> +
>>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +
>>>>> +    mutex_lock(&qspi->lock);
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>>> ops)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +
>>>>> +    mutex_unlock(&qspi->lock);
>>>>> +}
>>>>> +
>>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>>> +                  struct device_node *np)
>>>>> +{
>>>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>>> +    struct stm32_qspi_flash *flash;
>>>>> +    struct mtd_info *mtd;
>>>>> +    int ret;
>>>>> +
>>>>> +    of_property_read_u32(np, "reg", &cs_num);
>>>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>>> +    if (!max_rate)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>>> +
>>>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>>> +        width = 1;
>>>>> +
>>>>> +    if (width == 4)
>>>>> +        flash_read = SPI_NOR_QUAD;
>>>>> +    else if (width == 2)
>>>>> +        flash_read = SPI_NOR_DUAL;
>>>>> +    else if (width == 1)
>>>>> +        flash_read = SPI_NOR_NORMAL;
>>>>> +    else
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    flash = &qspi->flash[cs_num];
>>>>> +    flash->qspi = qspi;
>>>>> +    flash->cs = cs_num;
>>>>> +    flash->presc = presc;
>>>>> +
>>>>> +    flash->nor.dev = qspi->dev;
>>>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>>>> +    flash->nor.priv = flash;
>>>>> +    mtd = &flash->nor.mtd;
>>>>> +    mtd->priv = &flash->nor;
>>>>> +
>>>>> +    flash->nor.read = stm32_qspi_read;
>>>>> +    flash->nor.write = stm32_qspi_write;
>>>>> +    flash->nor.erase = stm32_qspi_erase;
>>>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>>>> +    flash->nor.prepare = stm32_qspi_prep;
>>>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>>>> +
>>>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>>> +
>>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN |
>>>>> CR_SSHIFT
>>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>>> +
>>>>> +    /*
>>>>> +     * in stm32 qspi controller, QUADSPI_DCR register has a fsize
>>>>> field
>>>>> +     * which define the size of nor flash.
>>>>> +     * if fsize is NULL, the controller can't sent spi-nor command.
>>>>> +     * set a temporary value just to discover the nor flash with
>>>>> +     * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>>> +     */
>>>> Is 25 the smallest value ? Use a macro for this ...
>>> 25 is an arbitrary choice, I will define a smallest value
>> Cool, thanks!
>>
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller

[Marek Vasut]

On 04/10/2017 06:52 PM, Ludovic BARRE wrote:
> hi Marek
> 
> tomorrow, I send a v3 with your/Rob reviews.

Super, thanks! I'll be pretty busy till Friday, so please keep in mind
the final review might take a bit.

> BR
> 
> Ludo
> 
> 
> On 04/10/2017 06:15 PM, Marek Vasut wrote:
>> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>>>
>>>>> The quadspi is a specialized communication interface targeting single,
>>>>> dual or quad SPI Flash memories.
>>>>>
>>>>> It can operate in any of the following modes:
>>>>> -indirect mode: all the operations are performed using the quadspi
>>>>>    registers
>>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>>>    microcontroller address space and is seen by the system as if it
>>>>> was
>>>>>    an internal memory
>>>>>
>>>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>>> ---
>>>>>    drivers/mtd/spi-nor/Kconfig         |   7 +
>>>>>    drivers/mtd/spi-nor/Makefile        |   1 +
>>>>>    drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>    3 files changed, 698 insertions(+)
>>>>>    create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>>
>>>> [...]
>>>>
>>>>> +struct stm32_qspi_flash {
>>>>> +    struct spi_nor nor;
>>>>> +    u32 cs;
>>>>> +    u32 fsize;
>>>>> +    u32 presc;
>>>>> +    struct stm32_qspi *qspi;
>>>>> +};
>>>> [...]
>>>>
>>>>> +struct stm32_qspi_cmd {
>>>>> +    struct {
>>>>> +        u8 addr_width;
>>>>> +        u8 dummy;
>>>>> +        u8 data;
>>>>> +    } conf;
>>>> Is there any benefit in having this structure here or could you just
>>>> make the struct stm32_qspi_cmd flat ?
>>> no benefit, it was just to regroup,  so I can do a flat structure
>> Well, as you like, but I think it does make sense to just make it flat.
>>
>>>>> +    u8 opcode;
>>>>> +    u32 framemode;
>>>>> +    u32 qspimode;
>>>>> +    u32 addr;
>>>>> +    size_t len;
>>>>> +    void *buf;
>>>>> +};
>>>> [...]
>>>>
>>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>>> size_t len,
>>>>> +                   u_char *buf)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +    struct stm32_qspi_cmd cmd;
>>>>> +    int err;
>>>>> +
>>>>> +    dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>>> +        nor->read_opcode, buf, (u32)from, len);
>>>>> +
>>>>> +    memset(&cmd, 0, sizeof(cmd));
>>>>> +    cmd.opcode = nor->read_opcode;
>>>>> +    cmd.conf.addr_width = nor->addr_width;
>>>>> +    cmd.addr = (u32)from;
>>>> loff_t (from) can be 64bit ... how do we handle this ?
>>> I'm surprise by the question,
>>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>>> On the other hand the framework and other drivers used this variable
>>> (from) like
>>> a 32 bits.
>> Hmmm, (rhetorical question) then why do we even use loff_t in the
>> framework ?
>>
>> Anyway, this is no problem then.
> In fact, the loff_t 64 bit come from mtd interface
> (needed to address biggest device constraint) but not needed for spi-nor
> devices.
>>>>> +    cmd.conf.data = 1;
>>>>> +    cmd.conf.dummy = nor->read_dummy;
>>>>> +    cmd.len = len;
>>>>> +    cmd.buf = buf;
>>>>> +    cmd.qspimode = qspi->read_mode;
>>>>> +
>>>>> +    stm32_qspi_set_framemode(nor, &cmd, true);
>>>>> +    err = stm32_qspi_send(flash, &cmd);
>>>>> +
>>>>> +    return err ? err : len;
>>>>> +}
>>>> [...]
>>>>
>>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>>> +{
>>>>> +    struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>>> +    u32 cr, sr, fcr = 0;
>>>>> +
>>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>>> +    sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>>> +
>>>>> +    if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>>> +        /* tx complete */
>>>>> +        fcr |= FCR_CTCF;
>>>>> +        complete(&qspi->cmd_completion);
>>>>> +    } else {
>>>>> +        dev_info(qspi->dev, "spurious interrupt\n");
>>>> You probably want to ratelimit this one ...
>>> yes it's better if there is an issue.
>> Yep
>>
>>>>> +    }
>>>>> +
>>>>> +    writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>>> +
>>>>> +    return IRQ_HANDLED;
>>>>> +}
>>>>> +
>>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +
>>>>> +    mutex_lock(&qspi->lock);
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>>> ops)
>>>>> +{
>>>>> +    struct stm32_qspi_flash *flash = nor->priv;
>>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>>> +
>>>>> +    mutex_unlock(&qspi->lock);
>>>>> +}
>>>>> +
>>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>>> +                  struct device_node *np)
>>>>> +{
>>>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>>> +    struct stm32_qspi_flash *flash;
>>>>> +    struct mtd_info *mtd;
>>>>> +    int ret;
>>>>> +
>>>>> +    of_property_read_u32(np, "reg", &cs_num);
>>>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>>> +    if (!max_rate)
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>>> +
>>>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>>> +        width = 1;
>>>>> +
>>>>> +    if (width == 4)
>>>>> +        flash_read = SPI_NOR_QUAD;
>>>>> +    else if (width == 2)
>>>>> +        flash_read = SPI_NOR_DUAL;
>>>>> +    else if (width == 1)
>>>>> +        flash_read = SPI_NOR_NORMAL;
>>>>> +    else
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    flash = &qspi->flash[cs_num];
>>>>> +    flash->qspi = qspi;
>>>>> +    flash->cs = cs_num;
>>>>> +    flash->presc = presc;
>>>>> +
>>>>> +    flash->nor.dev = qspi->dev;
>>>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>>>> +    flash->nor.priv = flash;
>>>>> +    mtd = &flash->nor.mtd;
>>>>> +    mtd->priv = &flash->nor;
>>>>> +
>>>>> +    flash->nor.read = stm32_qspi_read;
>>>>> +    flash->nor.write = stm32_qspi_write;
>>>>> +    flash->nor.erase = stm32_qspi_erase;
>>>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>>>> +    flash->nor.prepare = stm32_qspi_prep;
>>>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>>>> +
>>>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>>> +
>>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN |
>>>>> CR_SSHIFT
>>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>>> +
>>>>> +    /*
>>>>> +     * in stm32 qspi controller, QUADSPI_DCR register has a fsize
>>>>> field
>>>>> +     * which define the size of nor flash.
>>>>> +     * if fsize is NULL, the controller can't sent spi-nor command.
>>>>> +     * set a temporary value just to discover the nor flash with
>>>>> +     * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>>> +     */
>>>> Is 25 the smallest value ? Use a macro for this ...
>>> 25 is an arbitrary choice, I will define a smallest value
>> Cool, thanks!
>>
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html