[PATCH v2 00/10] spi: atmel-quadspi: introduce sam9x60 qspi controller

[PATCH v2 00/10] spi: atmel-quadspi: introduce sam9x60 qspi controller

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Patches from 1 to 8 are minor fixes or cosmetics.
Patches 9 and 10 introduce the sam9x60 qspi controller.

sam9x60 qspi controller tested with sst26vf064b jedec,spi-nor flash.
Backward compatibility test done on sama5d2 qspi controller and
mx25l25635e jedec,spi-nor flash.

The patches are generated on top of for-next branch.

v2:
- cache MR value,
- drop iomem wrappers,
- make "pclk" clock-name mandatory even for sama5d2,
- rework clock handling,
- reorder setting of register values in set_cfg() calls,
- collect R-b tags.

Tudor Ambarus (10):
  spi: atmel-quadspi: cache MR value to avoid a write access
  spi: atmel-quadspi: order header files inclusion alphabetically
  spi: atmel-quadspi: drop wrappers for iomem accesses
  spi: atmel-quadspi: fix naming scheme
  spi: atmel-quadspi: remove unnecessary cast
  spi: atmel-quadspi: return appropriate error code
  spi: atmel-quadspi: switch to SPDX license identifiers
  dt-bindings: spi: atmel-quadspi: update example to new clock binding
  dt-bindings: spi: atmel-quadspi: QuadSPI driver for Microchip SAM9X60
  spi: atmel-quadspi: add support for sam9x60 qspi controller

 .../devicetree/bindings/spi/atmel-quadspi.txt      |  14 +-
 drivers/spi/atmel-quadspi.c                        | 374 +++++++++++++++------
 2 files changed, 281 insertions(+), 107 deletions(-)

-- 
2.9.5

[PATCH v2 01/10] spi: atmel-quadspi: cache MR value to avoid a write access

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Cache MR value to avoid write access when setting the controller
in Serial Memory Mode (SMM). SMM is set in exec_op() and not at
probe time, to let room for future regular SPI support.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: cache MR value instead of moving the write access at probe

 drivers/spi/atmel-quadspi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index ddc712410812..fe05aee5d845 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -155,6 +155,7 @@ struct atmel_qspi {
 	struct clk		*clk;
 	struct platform_device	*pdev;
 	u32			pending;
+	u32			mr;
 	struct completion	cmd_completion;
 };
 
@@ -238,7 +239,9 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	icr = QSPI_ICR_INST(op->cmd.opcode);
 	ifr = QSPI_IFR_INSTEN;
 
-	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
+	/* Set the QSPI controller in Serial Memory Mode */
+	if (!(aq->mr & QSPI_MR_SMM))
+		qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
 
 	mode = find_mode(op);
 	if (mode < 0)
-- 
2.9.5

[PATCH v2 02/10] spi: atmel-quadspi: order header files inclusion alphabetically

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Cosmetic change, no functional change.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---
v2: collect R-b

 drivers/spi/atmel-quadspi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index fe05aee5d845..c986daa895a1 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -22,16 +22,15 @@
  * This driver is based on drivers/mtd/spi-nor/fsl-quadspi.c from Freescale.
  */
 
-#include <linux/kernel.h>
 #include <linux/clk.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
-#include <linux/of.h>
-
 #include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
 #include <linux/spi/spi-mem.h>
 
 /* QSPI register offsets */
-- 
2.9.5

[PATCH v2 03/10] spi: atmel-quadspi: drop wrappers for iomem accesses

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The wrappers hid that the accesses are relaxed. Drop them.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: new patch

 drivers/spi/atmel-quadspi.c | 47 +++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index c986daa895a1..3a7648ea07e8 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -175,17 +175,6 @@ static const struct qspi_mode sama5d2_qspi_modes[] = {
 	{ 4, 4, 4, QSPI_IFR_WIDTH_QUAD_CMD },
 };
 
-/* Register access functions */
-static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
-{
-	return readl_relaxed(aq->regs + reg);
-}
-
-static inline void qspi_writel(struct atmel_qspi *aq, u32 reg, u32 value)
-{
-	writel_relaxed(value, aq->regs + reg);
-}
-
 static inline bool is_compatible(const struct spi_mem_op *op,
 				 const struct qspi_mode *mode)
 {
@@ -229,6 +218,7 @@ static bool atmel_qspi_supports_op(struct spi_mem *mem,
 static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct atmel_qspi *aq = spi_controller_get_devdata(mem->spi->master);
+	void __iomem *base = aq->regs;
 	int mode;
 	u32 dummy_cycles = 0;
 	u32 iar, icr, ifr, sr;
@@ -240,7 +230,7 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	/* Set the QSPI controller in Serial Memory Mode */
 	if (!(aq->mr & QSPI_MR_SMM))
-		qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
+		writel_relaxed(QSPI_MR_SMM, base + QSPI_MR);
 
 	mode = find_mode(op);
 	if (mode < 0)
@@ -298,17 +288,17 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		ifr |= QSPI_IFR_TFRTYP_TRSFR_WRITE;
 
 	/* Clear pending interrupts */
-	(void)qspi_readl(aq, QSPI_SR);
+	(void)readl_relaxed(base + QSPI_SR);
 
 	/* Set QSPI Instruction Frame registers */
-	qspi_writel(aq, QSPI_IAR, iar);
-	qspi_writel(aq, QSPI_ICR, icr);
-	qspi_writel(aq, QSPI_IFR, ifr);
+	writel_relaxed(iar, base + QSPI_IAR);
+	writel_relaxed(icr, base + QSPI_ICR);
+	writel_relaxed(ifr, base + QSPI_IFR);
 
 	/* Skip to the final steps if there is no data */
 	if (op->data.nbytes) {
 		/* Dummy read of QSPI_IFR to synchronize APB and AHB accesses */
-		(void)qspi_readl(aq, QSPI_IFR);
+		(void)readl_relaxed(base + QSPI_IFR);
 
 		/* Send/Receive data */
 		if (op->data.dir == SPI_MEM_DATA_IN)
@@ -319,22 +309,22 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 				op->data.buf.out, op->data.nbytes);
 
 		/* Release the chip-select */
-		qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
+		writel_relaxed(QSPI_CR_LASTXFER, base + QSPI_CR);
 	}
 
 	/* Poll INSTRuction End status */
-	sr = qspi_readl(aq, QSPI_SR);
+	sr = readl_relaxed(base + QSPI_SR);
 	if ((sr & QSPI_SR_CMD_COMPLETED) == QSPI_SR_CMD_COMPLETED)
 		return err;
 
 	/* Wait for INSTRuction End interrupt */
 	reinit_completion(&aq->cmd_completion);
 	aq->pending = sr & QSPI_SR_CMD_COMPLETED;
-	qspi_writel(aq, QSPI_IER, QSPI_SR_CMD_COMPLETED);
+	writel_relaxed(QSPI_SR_CMD_COMPLETED, base + QSPI_IER);
 	if (!wait_for_completion_timeout(&aq->cmd_completion,
 					 msecs_to_jiffies(1000)))
 		err = -ETIMEDOUT;
-	qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
+	writel_relaxed(QSPI_SR_CMD_COMPLETED, base + QSPI_IDR);
 
 	return err;
 }
@@ -373,18 +363,20 @@ static int atmel_qspi_setup(struct spi_device *spi)
 		scbr--;
 
 	scr = QSPI_SCR_SCBR(scbr);
-	qspi_writel(aq, QSPI_SCR, scr);
+	writel_relaxed(scr, aq->regs + QSPI_SCR);
 
 	return 0;
 }
 
 static int atmel_qspi_init(struct atmel_qspi *aq)
 {
+	void __iomem *base = aq->regs;
+
 	/* Reset the QSPI controller */
-	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
+	writel_relaxed(QSPI_CR_SWRST, base + QSPI_CR);
 
 	/* Enable the QSPI controller */
-	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
+	writel_relaxed(QSPI_CR_QSPIEN, base + QSPI_CR);
 
 	return 0;
 }
@@ -392,10 +384,11 @@ static int atmel_qspi_init(struct atmel_qspi *aq)
 static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
 {
 	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
+	void __iomem *base = aq->regs;
 	u32 status, mask, pending;
 
-	status = qspi_readl(aq, QSPI_SR);
-	mask = qspi_readl(aq, QSPI_IMR);
+	status = readl_relaxed(base + QSPI_SR);
+	mask = readl_relaxed(base + QSPI_IMR);
 	pending = status & mask;
 
 	if (!pending)
@@ -501,7 +494,7 @@ static int atmel_qspi_remove(struct platform_device *pdev)
 	struct atmel_qspi *aq = spi_controller_get_devdata(ctrl);
 
 	spi_unregister_controller(ctrl);
-	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
+	writel_relaxed(QSPI_CR_QSPIDIS, aq->regs + QSPI_CR);
 	clk_disable_unprepare(aq->clk);
 	return 0;
 }
-- 
2.9.5

[PATCH v2 04/10] spi: atmel-quadspi: fix naming scheme

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Let general names to core drivers.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2: update after the removing of iomem access wrappers

 drivers/spi/atmel-quadspi.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 3a7648ea07e8..4105ded68f44 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -158,14 +158,14 @@ struct atmel_qspi {
 	struct completion	cmd_completion;
 };
 
-struct qspi_mode {
+struct atmel_qspi_mode {
 	u8 cmd_buswidth;
 	u8 addr_buswidth;
 	u8 data_buswidth;
 	u32 config;
 };
 
-static const struct qspi_mode sama5d2_qspi_modes[] = {
+static const struct atmel_qspi_mode sama5d2_qspi_modes[] = {
 	{ 1, 1, 1, QSPI_IFR_WIDTH_SINGLE_BIT_SPI },
 	{ 1, 1, 2, QSPI_IFR_WIDTH_DUAL_OUTPUT },
 	{ 1, 1, 4, QSPI_IFR_WIDTH_QUAD_OUTPUT },
@@ -175,8 +175,8 @@ static const struct qspi_mode sama5d2_qspi_modes[] = {
 	{ 4, 4, 4, QSPI_IFR_WIDTH_QUAD_CMD },
 };
 
-static inline bool is_compatible(const struct spi_mem_op *op,
-				 const struct qspi_mode *mode)
+static inline bool atmel_qspi_is_compatible(const struct spi_mem_op *op,
+					    const struct atmel_qspi_mode *mode)
 {
 	if (op->cmd.buswidth != mode->cmd_buswidth)
 		return false;
@@ -190,12 +190,12 @@ static inline bool is_compatible(const struct spi_mem_op *op,
 	return true;
 }
 
-static int find_mode(const struct spi_mem_op *op)
+static int atmel_qspi_find_mode(const struct spi_mem_op *op)
 {
 	u32 i;
 
 	for (i = 0; i < ARRAY_SIZE(sama5d2_qspi_modes); i++)
-		if (is_compatible(op, &sama5d2_qspi_modes[i]))
+		if (atmel_qspi_is_compatible(op, &sama5d2_qspi_modes[i]))
 			return i;
 
 	return -1;
@@ -204,7 +204,7 @@ static int find_mode(const struct spi_mem_op *op)
 static bool atmel_qspi_supports_op(struct spi_mem *mem,
 				   const struct spi_mem_op *op)
 {
-	if (find_mode(op) < 0)
+	if (atmel_qspi_find_mode(op) < 0)
 		return false;
 
 	/* special case not supported by hardware */
@@ -232,7 +232,7 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	if (!(aq->mr & QSPI_MR_SMM))
 		writel_relaxed(QSPI_MR_SMM, base + QSPI_MR);
 
-	mode = find_mode(op);
+	mode = atmel_qspi_find_mode(op);
 	if (mode < 0)
 		return -ENOTSUPP;
 
-- 
2.9.5

[PATCH v2 05/10] spi: atmel-quadspi: remove unnecessary cast

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The cast is done implicitly.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---
v2: collect R-b

 drivers/spi/atmel-quadspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 4105ded68f44..2db935488270 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -383,7 +383,7 @@ static int atmel_qspi_init(struct atmel_qspi *aq)
 
 static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
 {
-	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
+	struct atmel_qspi *aq = dev_id;
 	void __iomem *base = aq->regs;
 	u32 status, mask, pending;
 
-- 
2.9.5

[PATCH v2 06/10] spi: atmel-quadspi: return appropriate error code

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Return -ENOTSUPP when atmel_qspi_find_mode() fails. Propagate
the error in atmel_qspi_exec_op().

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---
v2: collect R-b

 drivers/spi/atmel-quadspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 2db935488270..1f7cf5d2953a 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -198,7 +198,7 @@ static int atmel_qspi_find_mode(const struct spi_mem_op *op)
 		if (atmel_qspi_is_compatible(op, &sama5d2_qspi_modes[i]))
 			return i;
 
-	return -1;
+	return -ENOTSUPP;
 }
 
 static bool atmel_qspi_supports_op(struct spi_mem *mem,
@@ -234,7 +234,7 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	mode = atmel_qspi_find_mode(op);
 	if (mode < 0)
-		return -ENOTSUPP;
+		return mode;
 
 	ifr |= sama5d2_qspi_modes[mode].config;
 
-- 
2.9.5

[PATCH v2 07/10] spi: atmel-quadspi: switch to SPDX license identifiers

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Adopt the SPDX license identifiers to ease license compliance
management.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---
v2: collect R-b

 drivers/spi/atmel-quadspi.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 1f7cf5d2953a..d3e76acf8517 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Driver for Atmel QSPI Controller
  *
@@ -7,18 +8,6 @@
  * Author: Cyrille Pitchen <cyrille.pitchen@atmel.com>
  * Author: Piotr Bugalski <bugalski.piotr@gmail.com>
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program.  If not, see <http://www.gnu.org/licenses/>.
- *
  * This driver is based on drivers/mtd/spi-nor/fsl-quadspi.c from Freescale.
  */
 
-- 
2.9.5

[PATCH v2 08/10] dt-bindings: spi: atmel-quadspi: update example to new clock binding

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

Introduced in:
commit b60557876849 ("ARM: dts: at91: sama5d2: switch to new clock binding")

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Reviewed-by: Boris Brezillon <bbrezillon@kernel.org>
---
v2: collect R-b

 Documentation/devicetree/bindings/spi/atmel-quadspi.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
index b93c1e2f25dd..e9dae6264d89 100644
--- a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
+++ b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
@@ -19,7 +19,7 @@ spi@f0020000 {
 	reg = <0xf0020000 0x100>, <0xd0000000 0x8000000>;
 	reg-names = "qspi_base", "qspi_mmap";
 	interrupts = <52 IRQ_TYPE_LEVEL_HIGH 7>;
-	clocks = <&spi0_clk>;
+	clocks = <&pmc PMC_TYPE_PERIPHERAL 52>;
 	#address-cells = <1>;
 	#size-cells = <0>;
 	pinctrl-names = "default";
-- 
2.9.5

[PATCH v2 09/10] dt-bindings: spi: atmel-quadspi: QuadSPI driver for Microchip SAM9X60

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The sam9x60 qspi controller uses 2 clocks, one for the peripheral register
access, the other for the qspi core and phy. Both are mandatory.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2:
- make "pclk" mandatory even for sama5d2. Unnamed clk will be
supported in the driver.
- drop unneeded example

 Documentation/devicetree/bindings/spi/atmel-quadspi.txt | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
index e9dae6264d89..f949f9197636 100644
--- a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
+++ b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
@@ -1,14 +1,22 @@
 * Atmel Quad Serial Peripheral Interface (QSPI)
 
 Required properties:
-- compatible:     Should be "atmel,sama5d2-qspi".
+- compatible:     Should be one of the following:
+		  - "atmel,sama5d2-qspi"
+		  - "microchip,sam9x60-qspi"
 - reg:            Should contain the locations and lengths of the base registers
                   and the mapped memory.
 - reg-names:      Should contain the resource reg names:
                   - qspi_base: configuration register address space
                   - qspi_mmap: memory mapped address space
 - interrupts:     Should contain the interrupt for the device.
-- clocks:         The phandle of the clock needed by the QSPI controller.
+- clocks:	  - "atmel,sama5d2-qspi": the phandle of the clock needed by the
+		  QSPI controller.
+		  - "microchip,sam9x60-qspi": should reference the peripheral
+		  and system QSPI clocks.
+- clock-names:    Should contain:
+		  - "pclk" for the peripheral clock
+		  - "qspick" for the system clock, when available (sam9x60)
 - #address-cells: Should be <1>.
 - #size-cells:    Should be <0>.
 
-- 
2.9.5

[PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The sam9x60 qspi controller uses 2 clocks, one for the peripheral register
access, the other for the qspi core and phy. Both are mandatory. It uses
dedicated register for Read Instruction Code Register (RICR) and
Write Instruction Code Register (WICR). ICR/RICR/WICR have identical
fields.

Tested with sst26vf064b jedec,spi-nor flash. Backward compatibility test
done on sama5d2 qspi controller and mx25l25635e jedec,spi-nor flash.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2:
- rework clock handling
- reorder setting of register values in set_cfg() calls -> move functions
  that can fail in the upper part of the function body.

 drivers/spi/atmel-quadspi.c | 296 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 239 insertions(+), 57 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index d3e76acf8517..80c934f3e479 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -19,6 +19,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi-mem.h>
 
@@ -35,7 +36,9 @@
 
 #define QSPI_IAR     0x0030  /* Instruction Address Register */
 #define QSPI_ICR     0x0034  /* Instruction Code Register */
+#define QSPI_WICR    0x0034  /* Write Instruction Code Register */
 #define QSPI_IFR     0x0038  /* Instruction Frame Register */
+#define QSPI_RICR    0x003C  /* Read Instruction Code Register */
 
 #define QSPI_SMR     0x0040  /* Scrambling Mode Register */
 #define QSPI_SKR     0x0044  /* Scrambling Key Register */
@@ -88,7 +91,7 @@
 #define QSPI_SCR_DLYBS_MASK             GENMASK(23, 16)
 #define QSPI_SCR_DLYBS(n)               (((n) << 16) & QSPI_SCR_DLYBS_MASK)
 
-/* Bitfields in QSPI_ICR (Instruction Code Register) */
+/* Bitfields in QSPI_ICR (Read/Write Instruction Code Register) */
 #define QSPI_ICR_INST_MASK              GENMASK(7, 0)
 #define QSPI_ICR_INST(inst)             (((inst) << 0) & QSPI_ICR_INST_MASK)
 #define QSPI_ICR_OPT_MASK               GENMASK(23, 16)
@@ -113,6 +116,8 @@
 #define QSPI_IFR_OPTL_4BIT              (2 << 8)
 #define QSPI_IFR_OPTL_8BIT              (3 << 8)
 #define QSPI_IFR_ADDRL                  BIT(10)
+#define QSPI_IFR_TFRTYP_TRSFR_MEM	BIT(12)
+#define QSPI_IFR_TFRTYP_TRSFR_REG	(0 << 12)
 #define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
 #define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
 #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)
@@ -121,6 +126,8 @@
 #define QSPI_IFR_CRM                    BIT(14)
 #define QSPI_IFR_NBDUM_MASK             GENMASK(20, 16)
 #define QSPI_IFR_NBDUM(n)               (((n) << 16) & QSPI_IFR_NBDUM_MASK)
+#define QSPI_IFR_APBTFRTYP_WRITE	(0 << 24)
+#define QSPI_IFR_APBTFRTYP_READ		BIT(24)
 
 /* Bitfields in QSPI_SMR (Scrambling Mode Register) */
 #define QSPI_SMR_SCREN                  BIT(0)
@@ -137,16 +144,37 @@
 #define QSPI_WPSR_WPVSRC(src)           (((src) << 8) & QSPI_WPSR_WPVSRC)
 
 
+/* Describes register values. */
+struct atmel_qspi_cfg {
+	u32 icr;
+	u32 iar;
+	u32 ifr;
+};
+
+struct atmel_qspi_caps;
+
 struct atmel_qspi {
 	void __iomem		*regs;
 	void __iomem		*mem;
 	struct clk		*clk;
+	struct clk		*qspick;
 	struct platform_device	*pdev;
+	const struct atmel_qspi_caps *caps;
 	u32			pending;
 	u32			mr;
 	struct completion	cmd_completion;
 };
 
+struct atmel_qspi_ops {
+	int (*set_qspi_cfg)(void __iomem *regs, const struct spi_mem_op *op,
+			    struct atmel_qspi_cfg *cfg);
+};
+
+struct atmel_qspi_caps {
+	const struct atmel_qspi_ops *ops;
+	bool has_qspick;
+};
+
 struct atmel_qspi_mode {
 	u8 cmd_buswidth;
 	u8 addr_buswidth;
@@ -204,28 +232,36 @@ static bool atmel_qspi_supports_op(struct spi_mem *mem,
 	return true;
 }
 
-static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+static int atmel_qspi_set_mode(struct atmel_qspi_cfg *cfg,
+			       const struct spi_mem_op *op)
 {
-	struct atmel_qspi *aq = spi_controller_get_devdata(mem->spi->master);
-	void __iomem *base = aq->regs;
-	int mode;
-	u32 dummy_cycles = 0;
-	u32 iar, icr, ifr, sr;
-	int err = 0;
+	int mode = atmel_qspi_find_mode(op);
 
-	iar = 0;
-	icr = QSPI_ICR_INST(op->cmd.opcode);
-	ifr = QSPI_IFR_INSTEN;
-
-	/* Set the QSPI controller in Serial Memory Mode */
-	if (!(aq->mr & QSPI_MR_SMM))
-		writel_relaxed(QSPI_MR_SMM, base + QSPI_MR);
-
-	mode = atmel_qspi_find_mode(op);
 	if (mode < 0)
 		return mode;
+	cfg->ifr = sama5d2_qspi_modes[mode].config;
+	return 0;
+}
 
-	ifr |= sama5d2_qspi_modes[mode].config;
+/*
+ * atmel_qspi_set_address_mode() - set address mode.
+ * @cfg:	contains register values
+ * @op:		describes a SPI memory operation
+ *
+ * The controller allows 24 and 32-bit addressing while NAND-flash requires
+ * 16-bit long. Handling 8-bit long addresses is done using the option field.
+ * For the 16-bit addresses, the workaround depends of the number of requested
+ * dummy bits. If there are 8 or more dummy cycles, the address is shifted and
+ * sent with the first dummy byte. Otherwise opcode is disabled and the first
+ * byte of the address contains the command opcode (works only if the opcode and
+ * address use the same buswidth). The limitation is when the 16-bit address is
+ * used without enough dummy cycles and the opcode is using a different buswidth
+ * than the address.
+ */
+static int atmel_qspi_set_address_mode(struct atmel_qspi_cfg *cfg,
+				       const struct spi_mem_op *op)
+{
+	u32 dummy_cycles = 0;
 
 	if (op->dummy.buswidth && op->dummy.nbytes)
 		dummy_cycles = op->dummy.nbytes * 8 / op->dummy.buswidth;
@@ -235,28 +271,28 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		case 0:
 			break;
 		case 1:
-			ifr |= QSPI_IFR_OPTEN | QSPI_IFR_OPTL_8BIT;
-			icr |= QSPI_ICR_OPT(op->addr.val & 0xff);
+			cfg->ifr |= QSPI_IFR_OPTEN | QSPI_IFR_OPTL_8BIT;
+			cfg->icr = QSPI_ICR_OPT(op->addr.val & 0xff);
 			break;
 		case 2:
 			if (dummy_cycles < 8 / op->addr.buswidth) {
-				ifr &= ~QSPI_IFR_INSTEN;
-				ifr |= QSPI_IFR_ADDREN;
-				iar = (op->cmd.opcode << 16) |
-					(op->addr.val & 0xffff);
+				cfg->ifr &= ~QSPI_IFR_INSTEN;
+				cfg->ifr |= QSPI_IFR_ADDREN;
+				cfg->iar = (op->cmd.opcode << 16) |
+					   (op->addr.val & 0xffff);
 			} else {
-				ifr |= QSPI_IFR_ADDREN;
-				iar = (op->addr.val << 8) & 0xffffff;
+				cfg->ifr |= QSPI_IFR_ADDREN;
+				cfg->iar = (op->addr.val << 8) & 0xffffff;
 				dummy_cycles -= 8 / op->addr.buswidth;
 			}
 			break;
 		case 3:
-			ifr |= QSPI_IFR_ADDREN;
-			iar = op->addr.val & 0xffffff;
+			cfg->ifr |= QSPI_IFR_ADDREN;
+			cfg->iar = op->addr.val & 0xffffff;
 			break;
 		case 4:
-			ifr |= QSPI_IFR_ADDREN | QSPI_IFR_ADDRL;
-			iar = op->addr.val & 0x7ffffff;
+			cfg->ifr |= QSPI_IFR_ADDREN | QSPI_IFR_ADDRL;
+			cfg->iar = op->addr.val & 0x7ffffff;
 			break;
 		default:
 			return -ENOTSUPP;
@@ -265,24 +301,106 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	/* Set number of dummy cycles */
 	if (dummy_cycles)
-		ifr |= QSPI_IFR_NBDUM(dummy_cycles);
+		cfg->ifr |= QSPI_IFR_NBDUM(dummy_cycles);
+
+	return 0;
+}
+
+static int atmel_sama5d2_qspi_set_cfg(void __iomem *base,
+				      const struct spi_mem_op *op,
+				      struct atmel_qspi_cfg *cfg)
+{
+	int ret = atmel_qspi_set_mode(cfg, op);
+
+	if (ret)
+		return ret;
+
+	ret = atmel_qspi_set_address_mode(cfg, op);
+	if (ret)
+		return ret;
+
+	cfg->ifr |= QSPI_IFR_INSTEN;
+	cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
 
 	/* Set data enable */
 	if (op->data.nbytes)
-		ifr |= QSPI_IFR_DATAEN;
+		cfg->ifr |= QSPI_IFR_DATAEN;
 
 	if (op->data.dir == SPI_MEM_DATA_IN && op->data.nbytes)
-		ifr |= QSPI_IFR_TFRTYP_TRSFR_READ;
+		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_READ;
 	else
-		ifr |= QSPI_IFR_TFRTYP_TRSFR_WRITE;
+		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_WRITE;
+
+	/* Clear pending interrupts */
+	(void)readl_relaxed(base + QSPI_SR);
+
+	/* Set QSPI Instruction Frame registers */
+	writel_relaxed(cfg->iar, base + QSPI_IAR);
+	writel_relaxed(cfg->icr, base + QSPI_ICR);
+	writel_relaxed(cfg->ifr, base + QSPI_IFR);
+
+	return 0;
+}
+
+static int atmel_sam9x60_qspi_set_cfg(void __iomem *base,
+				      const struct spi_mem_op *op,
+				      struct atmel_qspi_cfg *cfg)
+{
+	int ret = atmel_qspi_set_mode(cfg, op);
+
+	if (ret)
+		return ret;
+
+	ret = atmel_qspi_set_address_mode(cfg, op);
+	if (ret)
+		return ret;
+
+	cfg->ifr |= QSPI_IFR_INSTEN;
+	cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
+
+	/* Set data enable */
+	if (op->data.nbytes)
+		cfg->ifr |= QSPI_IFR_DATAEN;
+
+	if (!op->addr.nbytes) {
+		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_REG;
+		if (op->data.dir == SPI_MEM_DATA_OUT)
+			cfg->ifr |= QSPI_IFR_APBTFRTYP_WRITE;
+		else
+			cfg->ifr |= QSPI_IFR_APBTFRTYP_READ;
+	} else {
+		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_MEM;
+	}
 
 	/* Clear pending interrupts */
 	(void)readl_relaxed(base + QSPI_SR);
 
 	/* Set QSPI Instruction Frame registers */
-	writel_relaxed(iar, base + QSPI_IAR);
-	writel_relaxed(icr, base + QSPI_ICR);
-	writel_relaxed(ifr, base + QSPI_IFR);
+	writel_relaxed(cfg->iar, base + QSPI_IAR);
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		writel_relaxed(cfg->icr, base + QSPI_ICR);
+	else
+		writel_relaxed(cfg->icr, base + QSPI_RICR);
+	writel_relaxed(cfg->ifr, base + QSPI_IFR);
+
+	return 0;
+}
+
+static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	struct atmel_qspi *aq = spi_controller_get_devdata(mem->spi->master);
+	void __iomem *base = aq->regs;
+	struct atmel_qspi_cfg cfg = {0};
+	u32 sr;
+	int err;
+
+	/* Set the QSPI controller in Serial Memory Mode */
+	if (!(aq->mr & QSPI_MR_SMM))
+		writel_relaxed(QSPI_MR_SMM, base + QSPI_MR);
+
+	err = aq->caps->ops->set_qspi_cfg(base, op, &cfg);
+	if (err)
+		return err;
 
 	/* Skip to the final steps if there is no data */
 	if (op->data.nbytes) {
@@ -291,11 +409,11 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 		/* Send/Receive data */
 		if (op->data.dir == SPI_MEM_DATA_IN)
-			_memcpy_fromio(op->data.buf.in,
-				aq->mem + iar, op->data.nbytes);
+			_memcpy_fromio(op->data.buf.in, aq->mem + cfg.iar,
+				       op->data.nbytes);
 		else
-			_memcpy_toio(aq->mem + iar,
-				op->data.buf.out, op->data.nbytes);
+			_memcpy_toio(aq->mem + cfg.iar, op->data.buf.out,
+				     op->data.nbytes);
 
 		/* Release the chip-select */
 		writel_relaxed(QSPI_CR_LASTXFER, base + QSPI_CR);
@@ -395,9 +513,22 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	struct spi_controller *ctrl;
 	struct atmel_qspi *aq;
 	struct resource *res;
+	const struct atmel_qspi_caps *caps;
+	struct device *dev = &pdev->dev;
 	int irq, err = 0;
 
-	ctrl = spi_alloc_master(&pdev->dev, sizeof(*aq));
+	caps = of_device_get_match_data(dev);
+	if (!caps) {
+		dev_err(dev, "Could not retrieve QSPI caps\n");
+		return -EINVAL;
+	}
+
+	if (!caps->ops->set_qspi_cfg) {
+		dev_err(dev, "Could not retrieve QSPI ops\n");
+		return -EINVAL;
+	}
+
+	ctrl = spi_alloc_master(dev, sizeof(*aq));
 	if (!ctrl)
 		return -ENOMEM;
 
@@ -413,29 +544,33 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 
 	init_completion(&aq->cmd_completion);
 	aq->pdev = pdev;
+	aq->caps = caps;
 
 	/* Map the registers */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_base");
-	aq->regs = devm_ioremap_resource(&pdev->dev, res);
+	aq->regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(aq->regs)) {
-		dev_err(&pdev->dev, "missing registers\n");
+		dev_err(dev, "missing registers\n");
 		err = PTR_ERR(aq->regs);
 		goto exit;
 	}
 
 	/* Map the AHB memory */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mmap");
-	aq->mem = devm_ioremap_resource(&pdev->dev, res);
+	aq->mem = devm_ioremap_resource(dev, res);
 	if (IS_ERR(aq->mem)) {
-		dev_err(&pdev->dev, "missing AHB memory\n");
+		dev_err(dev, "missing AHB memory\n");
 		err = PTR_ERR(aq->mem);
 		goto exit;
 	}
 
 	/* Get the peripheral clock */
-	aq->clk = devm_clk_get(&pdev->dev, NULL);
+	aq->clk = devm_clk_get(dev, "pclk");
+	if (IS_ERR(aq->clk))
+		aq->clk = devm_clk_get(dev, NULL);
+
 	if (IS_ERR(aq->clk)) {
-		dev_err(&pdev->dev, "missing peripheral clock\n");
+		dev_err(dev, "missing peripheral clock\n");
 		err = PTR_ERR(aq->clk);
 		goto exit;
 	}
@@ -443,32 +578,52 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	/* Enable the peripheral clock */
 	err = clk_prepare_enable(aq->clk);
 	if (err) {
-		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
+		dev_err(dev, "failed to enable the peripheral clock\n");
 		goto exit;
 	}
 
+	if (caps->has_qspick) {
+		/* Get the QSPI system clock */
+		aq->qspick = devm_clk_get(dev, "qspick");
+		if (IS_ERR(aq->qspick)) {
+			dev_err(dev, "missing system clock\n");
+			err = PTR_ERR(aq->qspick);
+			goto disable_clk;
+		}
+
+		/* Enable the QSPI system clock */
+		err = clk_prepare_enable(aq->qspick);
+		if (err) {
+			dev_err(dev,
+				"failed to enable the QSPI system clock\n");
+			goto disable_clk;
+		}
+	}
+
 	/* Request the IRQ */
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
-		dev_err(&pdev->dev, "missing IRQ\n");
+		dev_err(dev, "missing IRQ\n");
 		err = irq;
-		goto disable_clk;
+		goto disable_qspick;
 	}
-	err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt,
-			       0, dev_name(&pdev->dev), aq);
+	err = devm_request_irq(dev, irq, atmel_qspi_interrupt, 0,
+			       dev_name(dev), aq);
 	if (err)
-		goto disable_clk;
+		goto disable_qspick;
 
 	err = atmel_qspi_init(aq);
 	if (err)
-		goto disable_clk;
+		goto disable_qspick;
 
 	err = spi_register_controller(ctrl);
 	if (err)
-		goto disable_clk;
+		goto disable_qspick;
 
 	return 0;
 
+disable_qspick:
+	clk_disable_unprepare(aq->qspick);
 disable_clk:
 	clk_disable_unprepare(aq->clk);
 exit:
@@ -484,6 +639,7 @@ static int atmel_qspi_remove(struct platform_device *pdev)
 
 	spi_unregister_controller(ctrl);
 	writel_relaxed(QSPI_CR_QSPIDIS, aq->regs + QSPI_CR);
+	clk_disable_unprepare(aq->qspick);
 	clk_disable_unprepare(aq->clk);
 	return 0;
 }
@@ -492,6 +648,7 @@ static int __maybe_unused atmel_qspi_suspend(struct device *dev)
 {
 	struct atmel_qspi *aq = dev_get_drvdata(dev);
 
+	clk_disable_unprepare(aq->qspick);
 	clk_disable_unprepare(aq->clk);
 
 	return 0;
@@ -502,6 +659,7 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
 	struct atmel_qspi *aq = dev_get_drvdata(dev);
 
 	clk_prepare_enable(aq->clk);
+	clk_prepare_enable(aq->qspick);
 
 	return atmel_qspi_init(aq);
 }
@@ -509,8 +667,32 @@ static int __maybe_unused atmel_qspi_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(atmel_qspi_pm_ops, atmel_qspi_suspend,
 			 atmel_qspi_resume);
 
+static const struct atmel_qspi_ops atmel_sama5d2_qspi_ops = {
+	.set_qspi_cfg = atmel_sama5d2_qspi_set_cfg,
+};
+
+static const struct atmel_qspi_caps atmel_sama5d2_qspi_caps = {
+	.ops = &atmel_sama5d2_qspi_ops,
+};
+
+static const struct atmel_qspi_ops atmel_sam9x60_qspi_ops = {
+	.set_qspi_cfg = atmel_sam9x60_qspi_set_cfg,
+};
+
+static const struct atmel_qspi_caps atmel_sam9x60_qspi_caps = {
+	.ops = &atmel_sam9x60_qspi_ops,
+	.has_qspick = true,
+};
+
 static const struct of_device_id atmel_qspi_dt_ids[] = {
-	{ .compatible = "atmel,sama5d2-qspi" },
+	{
+		.compatible = "atmel,sama5d2-qspi",
+		.data = &atmel_sama5d2_qspi_caps,
+	},
+	{
+		.compatible = "microchip,sam9x60-qspi",
+		.data = &atmel_sam9x60_qspi_caps,
+	},
 	{ /* sentinel */ }
 };
 
-- 
2.9.5

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Boris Brezillon]

On Thu, 31 Jan 2019 16:15:51 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> The sam9x60 qspi controller uses 2 clocks, one for the peripheral register
> access, the other for the qspi core and phy. Both are mandatory. It uses
> dedicated register for Read Instruction Code Register (RICR) and
> Write Instruction Code Register (WICR). ICR/RICR/WICR have identical
> fields.
> 
> Tested with sst26vf064b jedec,spi-nor flash. Backward compatibility test
> done on sama5d2 qspi controller and mx25l25635e jedec,spi-nor flash.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2:
> - rework clock handling
> - reorder setting of register values in set_cfg() calls -> move functions
>   that can fail in the upper part of the function body.
> 
>  drivers/spi/atmel-quadspi.c | 296 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 239 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index d3e76acf8517..80c934f3e479 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -19,6 +19,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/spi/spi-mem.h>
>  
> @@ -35,7 +36,9 @@
>  
>  #define QSPI_IAR     0x0030  /* Instruction Address Register */
>  #define QSPI_ICR     0x0034  /* Instruction Code Register */
> +#define QSPI_WICR    0x0034  /* Write Instruction Code Register */
>  #define QSPI_IFR     0x0038  /* Instruction Frame Register */
> +#define QSPI_RICR    0x003C  /* Read Instruction Code Register */
>  
>  #define QSPI_SMR     0x0040  /* Scrambling Mode Register */
>  #define QSPI_SKR     0x0044  /* Scrambling Key Register */
> @@ -88,7 +91,7 @@
>  #define QSPI_SCR_DLYBS_MASK             GENMASK(23, 16)
>  #define QSPI_SCR_DLYBS(n)               (((n) << 16) & QSPI_SCR_DLYBS_MASK)
>  
> -/* Bitfields in QSPI_ICR (Instruction Code Register) */
> +/* Bitfields in QSPI_ICR (Read/Write Instruction Code Register) */
>  #define QSPI_ICR_INST_MASK              GENMASK(7, 0)
>  #define QSPI_ICR_INST(inst)             (((inst) << 0) & QSPI_ICR_INST_MASK)
>  #define QSPI_ICR_OPT_MASK               GENMASK(23, 16)
> @@ -113,6 +116,8 @@
>  #define QSPI_IFR_OPTL_4BIT              (2 << 8)
>  #define QSPI_IFR_OPTL_8BIT              (3 << 8)
>  #define QSPI_IFR_ADDRL                  BIT(10)
> +#define QSPI_IFR_TFRTYP_TRSFR_MEM	BIT(12)
> +#define QSPI_IFR_TFRTYP_TRSFR_REG	(0 << 12)

You don't need to define TRSFR_REG, just set QSPI_IFR_TFRTYP_TRSFR_MEM
when you do a mem transfer and do nothing when this is a regular
transfer.

>  #define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
>  #define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
>  #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)

Looks like the read/write flag is on bit 13. Can we just add

#define QSPI_IFR_TFRTYP_TRSFR_WRITE	BIT(13)

and drop all others def? This way the implementation is consistent
between sam9x60 and sama5d2.

> @@ -121,6 +126,8 @@
>  #define QSPI_IFR_CRM                    BIT(14)
>  #define QSPI_IFR_NBDUM_MASK             GENMASK(20, 16)
>  #define QSPI_IFR_NBDUM(n)               (((n) << 16) & QSPI_IFR_NBDUM_MASK)
> +#define QSPI_IFR_APBTFRTYP_WRITE	(0 << 24)

As for the other defs, I don't think you need to define _WRITE.

> +#define QSPI_IFR_APBTFRTYP_READ		BIT(24)
>  
>  /* Bitfields in QSPI_SMR (Scrambling Mode Register) */
>  #define QSPI_SMR_SCREN                  BIT(0)
> @@ -137,16 +144,37 @@
>  #define QSPI_WPSR_WPVSRC(src)           (((src) << 8) & QSPI_WPSR_WPVSRC)
>  
>  
> +/* Describes register values. */
> +struct atmel_qspi_cfg {
> +	u32 icr;
> +	u32 iar;
> +	u32 ifr;
> +};
> +
> +struct atmel_qspi_caps;
> +
>  struct atmel_qspi {
>  	void __iomem		*regs;
>  	void __iomem		*mem;
>  	struct clk		*clk;

Can we rename that on pclk?

> +	struct clk		*qspick;
>  	struct platform_device	*pdev;
> +	const struct atmel_qspi_caps *caps;
>  	u32			pending;
>  	u32			mr;
>  	struct completion	cmd_completion;
>  };
>  

...

> +
> +static int atmel_sam9x60_qspi_set_cfg(void __iomem *base,
> +				      const struct spi_mem_op *op,
> +				      struct atmel_qspi_cfg *cfg)
> +{
> +	int ret = atmel_qspi_set_mode(cfg, op);
> +
> +	if (ret)
> +		return ret;
> +
> +	ret = atmel_qspi_set_address_mode(cfg, op);
> +	if (ret)
> +		return ret;
> +
> +	cfg->ifr |= QSPI_IFR_INSTEN;
> +	cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
> +
> +	/* Set data enable */
> +	if (op->data.nbytes)
> +		cfg->ifr |= QSPI_IFR_DATAEN;
> +
> +	if (!op->addr.nbytes) {
> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_REG;
> +		if (op->data.dir == SPI_MEM_DATA_OUT)
> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_WRITE;
> +		else
> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_READ;
> +	} else {
> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_MEM;

Can you try doing only regular transfers and let me know if it still
works. Support for mem transfers can then be added along with dirmap
support.

> +	}
>  
>  	/* Clear pending interrupts */
>  	(void)readl_relaxed(base + QSPI_SR);
>  
>  	/* Set QSPI Instruction Frame registers */
> -	writel_relaxed(iar, base + QSPI_IAR);
> -	writel_relaxed(icr, base + QSPI_ICR);
> -	writel_relaxed(ifr, base + QSPI_IFR);
> +	writel_relaxed(cfg->iar, base + QSPI_IAR);
> +	if (op->data.dir == SPI_MEM_DATA_OUT)
> +		writel_relaxed(cfg->icr, base + QSPI_ICR);
> +	else
> +		writel_relaxed(cfg->icr, base + QSPI_RICR);
> +	writel_relaxed(cfg->ifr, base + QSPI_IFR);
> +
> +	return 0;
> +}
> +

...

> @@ -443,32 +578,52 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>  	/* Enable the peripheral clock */
>  	err = clk_prepare_enable(aq->clk);
>  	if (err) {
> -		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
> +		dev_err(dev, "failed to enable the peripheral clock\n");
>  		goto exit;
>  	}
>  
> +	if (caps->has_qspick) {
> +		/* Get the QSPI system clock */
> +		aq->qspick = devm_clk_get(dev, "qspick");
> +		if (IS_ERR(aq->qspick)) {
> +			dev_err(dev, "missing system clock\n");
> +			err = PTR_ERR(aq->qspick);
> +			goto disable_clk;
> +		}
> +
> +		/* Enable the QSPI system clock */
> +		err = clk_prepare_enable(aq->qspick);
> +		if (err) {
> +			dev_err(dev,
> +				"failed to enable the QSPI system clock\n");
> +			goto disable_clk;
> +		}
> +	}
> +
>  	/* Request the IRQ */
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
> -		dev_err(&pdev->dev, "missing IRQ\n");
> +		dev_err(dev, "missing IRQ\n");
>  		err = irq;
> -		goto disable_clk;
> +		goto disable_qspick;
>  	}
> -	err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt,
> -			       0, dev_name(&pdev->dev), aq);
> +	err = devm_request_irq(dev, irq, atmel_qspi_interrupt, 0,
> +			       dev_name(dev), aq);
>  	if (err)
> -		goto disable_clk;
> +		goto disable_qspick;
>  
>  	err = atmel_qspi_init(aq);
>  	if (err)
> -		goto disable_clk;
> +		goto disable_qspick;
>  
>  	err = spi_register_controller(ctrl);
>  	if (err)
> -		goto disable_clk;
> +		goto disable_qspick;
>  
>  	return 0;
>  
> +disable_qspick:
> +	clk_disable_unprepare(aq->qspick);
>  disable_clk:

We should probably rename this label disable_pclk.

>  	clk_disable_unprepare(aq->clk);
>  exit:

Re: [PATCH v2 01/10] spi: atmel-quadspi: cache MR value to avoid a write access

[Boris Brezillon]

On Thu, 31 Jan 2019 16:15:28 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Cache MR value to avoid write access when setting the controller
> in Serial Memory Mode (SMM). SMM is set in exec_op() and not at
> probe time, to let room for future regular SPI support.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2: cache MR value instead of moving the write access at probe
> 
>  drivers/spi/atmel-quadspi.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index ddc712410812..fe05aee5d845 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -155,6 +155,7 @@ struct atmel_qspi {
>  	struct clk		*clk;
>  	struct platform_device	*pdev;
>  	u32			pending;
> +	u32			mr;
>  	struct completion	cmd_completion;
>  };
>  
> @@ -238,7 +239,9 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  	icr = QSPI_ICR_INST(op->cmd.opcode);
>  	ifr = QSPI_IFR_INSTEN;
>  
> -	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
> +	/* Set the QSPI controller in Serial Memory Mode */
> +	if (!(aq->mr & QSPI_MR_SMM))

	if (aq->mr != QSPI_MR_SMM)

> +		qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);

You need to update ->mr here.

>  
>  	mode = find_mode(op);
>  	if (mode < 0)

Re: [PATCH v2 01/10] spi: atmel-quadspi: cache MR value to avoid a write access

[Tudor.Ambarus]

On 01/31/2019 06:55 PM, Boris Brezillon wrote:
> On Thu, 31 Jan 2019 16:15:28 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Cache MR value to avoid write access when setting the controller
>> in Serial Memory Mode (SMM). SMM is set in exec_op() and not at
>> probe time, to let room for future regular SPI support.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>> v2: cache MR value instead of moving the write access at probe
>>
>>  drivers/spi/atmel-quadspi.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
>> index ddc712410812..fe05aee5d845 100644
>> --- a/drivers/spi/atmel-quadspi.c
>> +++ b/drivers/spi/atmel-quadspi.c
>> @@ -155,6 +155,7 @@ struct atmel_qspi {
>>  	struct clk		*clk;
>>  	struct platform_device	*pdev;
>>  	u32			pending;
>> +	u32			mr;
>>  	struct completion	cmd_completion;
>>  };
>>  
>> @@ -238,7 +239,9 @@ static int atmel_qspi_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
>>  	icr = QSPI_ICR_INST(op->cmd.opcode);
>>  	ifr = QSPI_IFR_INSTEN;
>>  
>> -	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
>> +	/* Set the QSPI controller in Serial Memory Mode */
>> +	if (!(aq->mr & QSPI_MR_SMM))
> 
> 	if (aq->mr != QSPI_MR_SMM)

then I should probably rename mr to smm.

> 
>> +		qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
> 
> You need to update ->mr here.

of course, thanks!

> 
>>  
>>  	mode = find_mode(op);
>>  	if (mode < 0)
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Tudor.Ambarus]

On 01/31/2019 06:32 PM, Boris Brezillon wrote:
> On Thu, 31 Jan 2019 16:15:51 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> The sam9x60 qspi controller uses 2 clocks, one for the peripheral register
>> access, the other for the qspi core and phy. Both are mandatory. It uses
>> dedicated register for Read Instruction Code Register (RICR) and
>> Write Instruction Code Register (WICR). ICR/RICR/WICR have identical
>> fields.
>>
>> Tested with sst26vf064b jedec,spi-nor flash. Backward compatibility test
>> done on sama5d2 qspi controller and mx25l25635e jedec,spi-nor flash.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>> v2:
>> - rework clock handling
>> - reorder setting of register values in set_cfg() calls -> move functions
>>   that can fail in the upper part of the function body.
>>
>>  drivers/spi/atmel-quadspi.c | 296 +++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 239 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
>> index d3e76acf8517..80c934f3e479 100644
>> --- a/drivers/spi/atmel-quadspi.c
>> +++ b/drivers/spi/atmel-quadspi.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>> +#include <linux/of_platform.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/spi/spi-mem.h>
>>  
>> @@ -35,7 +36,9 @@
>>  
>>  #define QSPI_IAR     0x0030  /* Instruction Address Register */
>>  #define QSPI_ICR     0x0034  /* Instruction Code Register */
>> +#define QSPI_WICR    0x0034  /* Write Instruction Code Register */
>>  #define QSPI_IFR     0x0038  /* Instruction Frame Register */
>> +#define QSPI_RICR    0x003C  /* Read Instruction Code Register */
>>  
>>  #define QSPI_SMR     0x0040  /* Scrambling Mode Register */
>>  #define QSPI_SKR     0x0044  /* Scrambling Key Register */
>> @@ -88,7 +91,7 @@
>>  #define QSPI_SCR_DLYBS_MASK             GENMASK(23, 16)
>>  #define QSPI_SCR_DLYBS(n)               (((n) << 16) & QSPI_SCR_DLYBS_MASK)
>>  
>> -/* Bitfields in QSPI_ICR (Instruction Code Register) */
>> +/* Bitfields in QSPI_ICR (Read/Write Instruction Code Register) */
>>  #define QSPI_ICR_INST_MASK              GENMASK(7, 0)
>>  #define QSPI_ICR_INST(inst)             (((inst) << 0) & QSPI_ICR_INST_MASK)
>>  #define QSPI_ICR_OPT_MASK               GENMASK(23, 16)
>> @@ -113,6 +116,8 @@
>>  #define QSPI_IFR_OPTL_4BIT              (2 << 8)
>>  #define QSPI_IFR_OPTL_8BIT              (3 << 8)
>>  #define QSPI_IFR_ADDRL                  BIT(10)
>> +#define QSPI_IFR_TFRTYP_TRSFR_MEM	BIT(12)
>> +#define QSPI_IFR_TFRTYP_TRSFR_REG	(0 << 12)
> 
> You don't need to define TRSFR_REG, just set QSPI_IFR_TFRTYP_TRSFR_MEM
> when you do a mem transfer and do nothing when this is a regular
> transfer.


I chose to introduce macros with zero value for better code readability. I would
expect that the NOP operations to be optimized at compile time. I will remove them
if you prefer, it will result in fewer lines of code.

> 
>>  #define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
>>  #define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
>>  #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)
> 
> Looks like the read/write flag is on bit 13. Can we just add

for sama5d2 only

> 
> #define QSPI_IFR_TFRTYP_TRSFR_WRITE	BIT(13)
> 
> and drop all others def? This way the implementation is consistent
> between sam9x60 and sama5d2.

BIT(13) has no meaning for sam9x60. I can drop the macros with zero value for
sama5d2 in a separate patch.

> 
>> @@ -121,6 +126,8 @@
>>  #define QSPI_IFR_CRM                    BIT(14)
>>  #define QSPI_IFR_NBDUM_MASK             GENMASK(20, 16)
>>  #define QSPI_IFR_NBDUM(n)               (((n) << 16) & QSPI_IFR_NBDUM_MASK)
>> +#define QSPI_IFR_APBTFRTYP_WRITE	(0 << 24)
> 
> As for the other defs, I don't think you need to define _WRITE.

understood

> 
>> +#define QSPI_IFR_APBTFRTYP_READ		BIT(24)
>>  
>>  /* Bitfields in QSPI_SMR (Scrambling Mode Register) */
>>  #define QSPI_SMR_SCREN                  BIT(0)
>> @@ -137,16 +144,37 @@
>>  #define QSPI_WPSR_WPVSRC(src)           (((src) << 8) & QSPI_WPSR_WPVSRC)
>>  
>>  
>> +/* Describes register values. */
>> +struct atmel_qspi_cfg {
>> +	u32 icr;
>> +	u32 iar;
>> +	u32 ifr;
>> +};
>> +
>> +struct atmel_qspi_caps;
>> +
>>  struct atmel_qspi {
>>  	void __iomem		*regs;
>>  	void __iomem		*mem;
>>  	struct clk		*clk;
> 
> Can we rename that on pclk?

will rename it, together with the support for unnamed clock of sama5d2 in a separate
patch. The dt-bindings patch that imposes "pclk" for sama5d2 should be separated too.

> 
>> +	struct clk		*qspick;
>>  	struct platform_device	*pdev;
>> +	const struct atmel_qspi_caps *caps;
>>  	u32			pending;
>>  	u32			mr;
>>  	struct completion	cmd_completion;
>>  };
>>  
> 
> ...
> 
>> +
>> +static int atmel_sam9x60_qspi_set_cfg(void __iomem *base,
>> +				      const struct spi_mem_op *op,
>> +				      struct atmel_qspi_cfg *cfg)
>> +{
>> +	int ret = atmel_qspi_set_mode(cfg, op);
>> +
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = atmel_qspi_set_address_mode(cfg, op);
>> +	if (ret)
>> +		return ret;
>> +
>> +	cfg->ifr |= QSPI_IFR_INSTEN;
>> +	cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
>> +
>> +	/* Set data enable */
>> +	if (op->data.nbytes)
>> +		cfg->ifr |= QSPI_IFR_DATAEN;
>> +
>> +	if (!op->addr.nbytes) {
>> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_REG;
>> +		if (op->data.dir == SPI_MEM_DATA_OUT)
>> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_WRITE;
>> +		else
>> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_READ;
>> +	} else {
>> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_MEM;
> 
> Can you try doing only regular transfers and let me know if it still
> works. Support for mem transfers can then be added along with dirmap
> support.

should work. Will try and let you know.

> 
>> +	}
>>  
>>  	/* Clear pending interrupts */
>>  	(void)readl_relaxed(base + QSPI_SR);
>>  
>>  	/* Set QSPI Instruction Frame registers */
>> -	writel_relaxed(iar, base + QSPI_IAR);
>> -	writel_relaxed(icr, base + QSPI_ICR);
>> -	writel_relaxed(ifr, base + QSPI_IFR);
>> +	writel_relaxed(cfg->iar, base + QSPI_IAR);
>> +	if (op->data.dir == SPI_MEM_DATA_OUT)
>> +		writel_relaxed(cfg->icr, base + QSPI_ICR);
>> +	else
>> +		writel_relaxed(cfg->icr, base + QSPI_RICR);
>> +	writel_relaxed(cfg->ifr, base + QSPI_IFR);
>> +
>> +	return 0;
>> +}
>> +
> 
> ...
> 
>> @@ -443,32 +578,52 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>>  	/* Enable the peripheral clock */
>>  	err = clk_prepare_enable(aq->clk);
>>  	if (err) {
>> -		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
>> +		dev_err(dev, "failed to enable the peripheral clock\n");
>>  		goto exit;
>>  	}
>>  
>> +	if (caps->has_qspick) {
>> +		/* Get the QSPI system clock */
>> +		aq->qspick = devm_clk_get(dev, "qspick");
>> +		if (IS_ERR(aq->qspick)) {
>> +			dev_err(dev, "missing system clock\n");
>> +			err = PTR_ERR(aq->qspick);
>> +			goto disable_clk;
>> +		}
>> +
>> +		/* Enable the QSPI system clock */
>> +		err = clk_prepare_enable(aq->qspick);
>> +		if (err) {
>> +			dev_err(dev,
>> +				"failed to enable the QSPI system clock\n");
>> +			goto disable_clk;
>> +		}
>> +	}
>> +
>>  	/* Request the IRQ */
>>  	irq = platform_get_irq(pdev, 0);
>>  	if (irq < 0) {
>> -		dev_err(&pdev->dev, "missing IRQ\n");
>> +		dev_err(dev, "missing IRQ\n");
>>  		err = irq;
>> -		goto disable_clk;
>> +		goto disable_qspick;
>>  	}
>> -	err = devm_request_irq(&pdev->dev, irq, atmel_qspi_interrupt,
>> -			       0, dev_name(&pdev->dev), aq);
>> +	err = devm_request_irq(dev, irq, atmel_qspi_interrupt, 0,
>> +			       dev_name(dev), aq);
>>  	if (err)
>> -		goto disable_clk;
>> +		goto disable_qspick;
>>  
>>  	err = atmel_qspi_init(aq);
>>  	if (err)
>> -		goto disable_clk;
>> +		goto disable_qspick;
>>  
>>  	err = spi_register_controller(ctrl);
>>  	if (err)
>> -		goto disable_clk;
>> +		goto disable_qspick;
>>  
>>  	return 0;
>>  
>> +disable_qspick:
>> +	clk_disable_unprepare(aq->qspick);
>>  disable_clk:
> 
> We should probably rename this label disable_pclk.

sure. Thanks, Boris!

> 
>>  	clk_disable_unprepare(aq->clk);
>>  exit:
> 

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Boris Brezillon]

On Fri, 1 Feb 2019 07:07:40 +0000
<Tudor.Ambarus@microchip.com> wrote:

> >   
> >>  #define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
> >>  #define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
> >>  #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)  
> > 
> > Looks like the read/write flag is on bit 13. Can we just add  
> 
> for sama5d2 only

Feel free to prefix macros with the SoC name to make it clear:

#define QSPI_IFR_SAMA5D2_WRITE_TRSFR		BIT(13)

> 
> > 
> > #define QSPI_IFR_TFRTYP_TRSFR_WRITE	BIT(13)
> > 
> > and drop all others def? This way the implementation is consistent
> > between sam9x60 and sama5d2.  
> 
> BIT(13) has no meaning for sam9x60. I can drop the macros with zero value for
> sama5d2 in a separate patch.
> >   
> >> +#define QSPI_IFR_APBTFRTYP_READ		BIT(24)

And this one would be

define QSPI_IFR_SAM9X60_READ_TRSFR		BIT(24)

> >>  
> >>  /* Bitfields in QSPI_SMR (Scrambling Mode Register) */
> >>  #define QSPI_SMR_SCREN                  BIT(0)
> >> @@ -137,16 +144,37 @@
> >>  #define QSPI_WPSR_WPVSRC(src)           (((src) << 8) & QSPI_WPSR_WPVSRC)
> >>  
> >>  
> >> +/* Describes register values. */
> >> +struct atmel_qspi_cfg {
> >> +	u32 icr;
> >> +	u32 iar;
> >> +	u32 ifr;
> >> +};
> >> +
> >> +struct atmel_qspi_caps;
> >> +
> >>  struct atmel_qspi {
> >>  	void __iomem		*regs;
> >>  	void __iomem		*mem;
> >>  	struct clk		*clk;  
> > 
> > Can we rename that on pclk?  
> 
> will rename it, together with the support for unnamed clock of sama5d2 in a separate
> patch. The dt-bindings patch that imposes "pclk" for sama5d2 should be separated too.

Sounds good.

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Tudor.Ambarus]

On 02/01/2019 09:07 AM, Tudor.Ambarus@microchip.com wrote:

cut

>>> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c

cut

>>> +static int atmel_sam9x60_qspi_set_cfg(void __iomem *base,
>>> +				      const struct spi_mem_op *op,
>>> +				      struct atmel_qspi_cfg *cfg)
>>> +{
>>> +	int ret = atmel_qspi_set_mode(cfg, op);
>>> +
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = atmel_qspi_set_address_mode(cfg, op);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	cfg->ifr |= QSPI_IFR_INSTEN;
>>> +	cfg->icr |= QSPI_ICR_INST(op->cmd.opcode);
>>> +
>>> +	/* Set data enable */
>>> +	if (op->data.nbytes)
>>> +		cfg->ifr |= QSPI_IFR_DATAEN;
>>> +
>>> +	if (!op->addr.nbytes) {
>>> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_REG;
>>> +		if (op->data.dir == SPI_MEM_DATA_OUT)
>>> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_WRITE;
>>> +		else
>>> +			cfg->ifr |= QSPI_IFR_APBTFRTYP_READ;
>>> +	} else {
>>> +		cfg->ifr |= QSPI_IFR_TFRTYP_TRSFR_MEM;
>>
>> Can you try doing only regular transfers and let me know if it still
>> works. Support for mem transfers can then be added along with dirmap
>> support.
> 
> should work. Will try and let you know.

you were right, it works. I will let mem transfer logic for dirmap support.

Cheers,
ta

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Tudor.Ambarus]

On 02/01/2019 09:57 AM, Boris Brezillon wrote:
> On Fri, 1 Feb 2019 07:07:40 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>>>   
>>>>  #define QSPI_IFR_TFRTYP_MASK            GENMASK(13, 12)
>>>>  #define QSPI_IFR_TFRTYP_TRSFR_READ      (0 << 12)
>>>>  #define QSPI_IFR_TFRTYP_TRSFR_READ_MEM  (1 << 12)  
>>>
>>> Looks like the read/write flag is on bit 13. Can we just add  
>>
>> for sama5d2 only
> 
> Feel free to prefix macros with the SoC name to make it clear:
> 
> #define QSPI_IFR_SAMA5D2_WRITE_TRSFR		BIT(13)

agreed

> 

cut

>>>> +#define QSPI_IFR_APBTFRTYP_READ		BIT(24)
> 
> And this one would be
> 
> define QSPI_IFR_SAM9X60_READ_TRSFR		BIT(24)

I prefer letting this bit named as in the datasheet, QSPI_IFR_APBTFRTYP_READ,
and change it if future versions of the IP will modify its sense. It is a READ
transfer done on APB, it is more generic this way. If you have a strong opinion
on this, please let me know.

ta

Re: [PATCH v2 10/10] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Boris Brezillon]

On Fri, 1 Feb 2019 14:49:27 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 
> >>>> +#define QSPI_IFR_APBTFRTYP_READ		BIT(24)  
> > 
> > And this one would be
> > 
> > define QSPI_IFR_SAM9X60_READ_TRSFR		BIT(24)  
> 
> I prefer letting this bit named as in the datasheet, QSPI_IFR_APBTFRTYP_READ,
> and change it if future versions of the IP will modify its sense. It is a READ
> transfer done on APB, it is more generic this way. If you have a strong opinion
> on this, please let me know.

As you wish.