[PATCH 0/9] spi: atmel-quadspi: introduce sam9x60 qspi contoller

[PATCH 0/9] spi: atmel-quadspi: introduce sam9x60 qspi contoller

[Tudor.Ambarus]

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

Patches from 1 to 7 are minor fixes or cosmetics.
Patches 8 and 9 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.

Tudor Ambarus (9):
  spi: atmel-quadspi: optimize qspi init
  spi: atmel-quadspi: order header files inclusion alphabetically
  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      |  30 +-
 drivers/spi/atmel-quadspi.c                        | 406 ++++++++++++++++-----
 2 files changed, 336 insertions(+), 100 deletions(-)

-- 
2.9.5

[PATCH 1/9] spi: atmel-quadspi: optimize qspi init

[Tudor.Ambarus]

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

Set the QSPI controller in Serial Memory Mode at init and not
at each exec_op() call.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/spi/atmel-quadspi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index ddc712410812..f79b17792a11 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -238,8 +238,6 @@ 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);
-
 	mode = find_mode(op);
 	if (mode < 0)
 		return -ENOTSUPP;
@@ -381,6 +379,9 @@ static int atmel_qspi_init(struct atmel_qspi *aq)
 	/* Reset the QSPI controller */
 	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
 
+	/* Set the QSPI controller in Serial Memory Mode */
+	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
+
 	/* Enable the QSPI controller */
 	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
 
-- 
2.9.5

Re: [PATCH 1/9] spi: atmel-quadspi: optimize qspi init

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:29 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Set the QSPI controller in Serial Memory Mode at init and not
> at each exec_op() call.

If you ever want to support regular SPI you'll have to put it back to
atmel_qspi_exec_op(), so I'm not sure this is a good move. Another
approach would be to cache the MR value to avoid doing a write access
on the bus when the value hasn't changed.

> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/spi/atmel-quadspi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index ddc712410812..f79b17792a11 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -238,8 +238,6 @@ 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);
> -
>  	mode = find_mode(op);
>  	if (mode < 0)
>  		return -ENOTSUPP;
> @@ -381,6 +379,9 @@ static int atmel_qspi_init(struct atmel_qspi *aq)
>  	/* Reset the QSPI controller */
>  	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
>  
> +	/* Set the QSPI controller in Serial Memory Mode */
> +	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
> +
>  	/* Enable the QSPI controller */
>  	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
>  

Re: [PATCH 1/9] spi: atmel-quadspi: optimize qspi init

[Tudor.Ambarus]

On 01/30/2019 07:15 PM, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 15:08:29 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Set the QSPI controller in Serial Memory Mode at init and not
>> at each exec_op() call.
> 
> If you ever want to support regular SPI you'll have to put it back to
> atmel_qspi_exec_op(), so I'm not sure this is a good move. Another
> approach would be to cache the MR value to avoid doing a write access
> on the bus when the value hasn't changed.

I'll cache MR value, thanks!

ta

[PATCH 2/9] 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>
---
 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 f79b17792a11..64475ad16c83 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

Re: [PATCH 2/9] spi: atmel-quadspi: order header files inclusion alphabetically

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:31 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 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.com>

> ---
>  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 f79b17792a11..64475ad16c83 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 */

[PATCH 3/9] 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>
---
 drivers/spi/atmel-quadspi.c | 52 ++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 64475ad16c83..e156c345705b 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -157,14 +157,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,18 +175,18 @@ static const struct qspi_mode sama5d2_qspi_modes[] = {
 };
 
 /* Register access functions */
-static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
+static inline u32 atmel_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)
+static inline void atmel_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)
+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;
@@ -200,12 +200,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;
@@ -214,7 +214,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 */
@@ -237,7 +237,7 @@ 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;
 
-	mode = find_mode(op);
+	mode = atmel_qspi_find_mode(op);
 	if (mode < 0)
 		return -ENOTSUPP;
 
@@ -293,17 +293,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)atmel_qspi_readl(aq, QSPI_SR);
 
 	/* Set QSPI Instruction Frame registers */
-	qspi_writel(aq, QSPI_IAR, iar);
-	qspi_writel(aq, QSPI_ICR, icr);
-	qspi_writel(aq, QSPI_IFR, ifr);
+	atmel_qspi_writel(aq, QSPI_IAR, iar);
+	atmel_qspi_writel(aq, QSPI_ICR, icr);
+	atmel_qspi_writel(aq, QSPI_IFR, 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)atmel_qspi_readl(aq, QSPI_IFR);
 
 		/* Send/Receive data */
 		if (op->data.dir == SPI_MEM_DATA_IN)
@@ -314,22 +314,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);
+		atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
 	}
 
 	/* Poll INSTRuction End status */
-	sr = qspi_readl(aq, QSPI_SR);
+	sr = atmel_qspi_readl(aq, 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);
+	atmel_qspi_writel(aq, QSPI_IER, QSPI_SR_CMD_COMPLETED);
 	if (!wait_for_completion_timeout(&aq->cmd_completion,
 					 msecs_to_jiffies(1000)))
 		err = -ETIMEDOUT;
-	qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
+	atmel_qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
 
 	return err;
 }
@@ -368,7 +368,7 @@ static int atmel_qspi_setup(struct spi_device *spi)
 		scbr--;
 
 	scr = QSPI_SCR_SCBR(scbr);
-	qspi_writel(aq, QSPI_SCR, scr);
+	atmel_qspi_writel(aq, QSPI_SCR, scr);
 
 	return 0;
 }
@@ -376,13 +376,13 @@ static int atmel_qspi_setup(struct spi_device *spi)
 static int atmel_qspi_init(struct atmel_qspi *aq)
 {
 	/* Reset the QSPI controller */
-	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
+	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
 
 	/* Set the QSPI controller in Serial Memory Mode */
-	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
+	atmel_qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
 
 	/* Enable the QSPI controller */
-	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
+	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
 
 	return 0;
 }
@@ -392,8 +392,8 @@ static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
 	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
 	u32 status, mask, pending;
 
-	status = qspi_readl(aq, QSPI_SR);
-	mask = qspi_readl(aq, QSPI_IMR);
+	status = atmel_qspi_readl(aq, QSPI_SR);
+	mask = atmel_qspi_readl(aq, QSPI_IMR);
 	pending = status & mask;
 
 	if (!pending)
@@ -499,7 +499,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);
+	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
 	clk_disable_unprepare(aq->clk);
 	return 0;
 }
-- 
2.9.5

Re: [PATCH 3/9] spi: atmel-quadspi: fix naming scheme

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:33 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> Let general names to core drivers.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/spi/atmel-quadspi.c | 52 ++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index 64475ad16c83..e156c345705b 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -157,14 +157,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,18 +175,18 @@ static const struct qspi_mode sama5d2_qspi_modes[] = {
>  };
>  
>  /* Register access functions */
> -static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
> +static inline u32 atmel_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)
> +static inline void atmel_qspi_writel(struct atmel_qspi *aq, u32 reg, u32 value)
>  {
>  	writel_relaxed(value, aq->regs + reg);
>  }

Can we get rid of these wrappers? I'm not a big fan of wrappers that
hide the fact that accesses are relaxed.

>  
> -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;
> @@ -200,12 +200,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;
> @@ -214,7 +214,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 */
> @@ -237,7 +237,7 @@ 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;
>  
> -	mode = find_mode(op);
> +	mode = atmel_qspi_find_mode(op);
>  	if (mode < 0)
>  		return -ENOTSUPP;
>  
> @@ -293,17 +293,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)atmel_qspi_readl(aq, QSPI_SR);
>  
>  	/* Set QSPI Instruction Frame registers */
> -	qspi_writel(aq, QSPI_IAR, iar);
> -	qspi_writel(aq, QSPI_ICR, icr);
> -	qspi_writel(aq, QSPI_IFR, ifr);
> +	atmel_qspi_writel(aq, QSPI_IAR, iar);
> +	atmel_qspi_writel(aq, QSPI_ICR, icr);
> +	atmel_qspi_writel(aq, QSPI_IFR, 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)atmel_qspi_readl(aq, QSPI_IFR);
>  
>  		/* Send/Receive data */
>  		if (op->data.dir == SPI_MEM_DATA_IN)
> @@ -314,22 +314,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);
> +		atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
>  	}
>  
>  	/* Poll INSTRuction End status */
> -	sr = qspi_readl(aq, QSPI_SR);
> +	sr = atmel_qspi_readl(aq, 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);
> +	atmel_qspi_writel(aq, QSPI_IER, QSPI_SR_CMD_COMPLETED);
>  	if (!wait_for_completion_timeout(&aq->cmd_completion,
>  					 msecs_to_jiffies(1000)))
>  		err = -ETIMEDOUT;
> -	qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
> +	atmel_qspi_writel(aq, QSPI_IDR, QSPI_SR_CMD_COMPLETED);
>  
>  	return err;
>  }
> @@ -368,7 +368,7 @@ static int atmel_qspi_setup(struct spi_device *spi)
>  		scbr--;
>  
>  	scr = QSPI_SCR_SCBR(scbr);
> -	qspi_writel(aq, QSPI_SCR, scr);
> +	atmel_qspi_writel(aq, QSPI_SCR, scr);
>  
>  	return 0;
>  }
> @@ -376,13 +376,13 @@ static int atmel_qspi_setup(struct spi_device *spi)
>  static int atmel_qspi_init(struct atmel_qspi *aq)
>  {
>  	/* Reset the QSPI controller */
> -	qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
> +	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_SWRST);
>  
>  	/* Set the QSPI controller in Serial Memory Mode */
> -	qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
> +	atmel_qspi_writel(aq, QSPI_MR, QSPI_MR_SMM);
>  
>  	/* Enable the QSPI controller */
> -	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
> +	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIEN);
>  
>  	return 0;
>  }
> @@ -392,8 +392,8 @@ static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
>  	struct atmel_qspi *aq = (struct atmel_qspi *)dev_id;
>  	u32 status, mask, pending;
>  
> -	status = qspi_readl(aq, QSPI_SR);
> -	mask = qspi_readl(aq, QSPI_IMR);
> +	status = atmel_qspi_readl(aq, QSPI_SR);
> +	mask = atmel_qspi_readl(aq, QSPI_IMR);
>  	pending = status & mask;
>  
>  	if (!pending)
> @@ -499,7 +499,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);
> +	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
>  	clk_disable_unprepare(aq->clk);
>  	return 0;
>  }

Re: [PATCH 3/9] spi: atmel-quadspi: fix naming scheme

[Tudor.Ambarus]

On 01/30/2019 07:19 PM, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 15:08:33 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> Let general names to core drivers.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/spi/atmel-quadspi.c | 52 ++++++++++++++++++++++-----------------------
>>  1 file changed, 26 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
>> index 64475ad16c83..e156c345705b 100644
>> --- a/drivers/spi/atmel-quadspi.c
>> +++ b/drivers/spi/atmel-quadspi.c
>> @@ -157,14 +157,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,18 +175,18 @@ static const struct qspi_mode sama5d2_qspi_modes[] = {
>>  };
>>  
>>  /* Register access functions */
>> -static inline u32 qspi_readl(struct atmel_qspi *aq, u32 reg)
>> +static inline u32 atmel_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)
>> +static inline void atmel_qspi_writel(struct atmel_qspi *aq, u32 reg, u32 value)
>>  {
>>  	writel_relaxed(value, aq->regs + reg);
>>  }
> Can we get rid of these wrappers? I'm not a big fan of wrappers that
> hide the fact that accesses are relaxed.
> 

Will do, thanks!

ta

[PATCH 4/9] 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>
---
 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 e156c345705b..3643f0c851b0 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -389,7 +389,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;
 	u32 status, mask, pending;
 
 	status = atmel_qspi_readl(aq, QSPI_SR);
-- 
2.9.5

Re: [PATCH 4/9] spi: atmel-quadspi: remove unnecessary cast

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:35 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 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>

> ---
>  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 e156c345705b..3643f0c851b0 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -389,7 +389,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;
>  	u32 status, mask, pending;
>  
>  	status = atmel_qspi_readl(aq, QSPI_SR);

[PATCH 5/9] 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>
---
 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 3643f0c851b0..e6de6e3d1345 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -208,7 +208,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,
@@ -239,7 +239,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

Re: [PATCH 5/9] spi: atmel-quadspi: return appropriate error code

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:38 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 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>

> ---
>  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 3643f0c851b0..e6de6e3d1345 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -208,7 +208,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,
> @@ -239,7 +239,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;
>  

[PATCH 6/9] 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>
---
 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 e6de6e3d1345..1d21db7851e9 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

Re: [PATCH 6/9] spi: atmel-quadspi: switch to SPDX license identifiers

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:40 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 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>

> ---
>  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 e6de6e3d1345..1d21db7851e9 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.
>   */
>  

[PATCH 7/9] 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>
---
 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

Re: [PATCH 7/9] dt-bindings: spi: atmel-quadspi: update example to new clock binding

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:43 +0000
<Tudor.Ambarus@microchip.com> wrote:

> 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>

> ---
>  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";

[PATCH 8/9] 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>
---
 .../devicetree/bindings/spi/atmel-quadspi.txt      | 28 ++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
index e9dae6264d89..e7b7f297c5d7 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:    Only for sam9x60 - should contain two strigs:
+		  - "pclk" for the peripheral clock
+		  - "qspick" for the system clock
 - #address-cells: Should be <1>.
 - #size-cells:    Should be <0>.
 
@@ -29,3 +37,19 @@ spi@f0020000 {
 		...
 	};
 };
+
+qspi@f0014000 {
+	compatible = "microchip,sam9x60-qspi";
+	reg = <0xf0014000 0x100>, <0x70000000 0x08000000>;
+	reg-names = "qspi_base", "qspi_mmap";
+	interrupts = <35 IRQ_TYPE_LEVEL_HIGH 7>;
+	clocks =  <&pmc PMC_TYPE_PERIPHERAL 35>, <&pmc PMC_TYPE_SYSTEM 19>;
+	clock-names = "pclk", "qspick";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	pinctrl-names = "default";
+
+	flash@0 {
+		...
+	};
+};
-- 
2.9.5

Re: [PATCH 8/9] dt-bindings: spi: atmel-quadspi: QuadSPI driver for Microchip SAM9X60

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:45 +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.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  .../devicetree/bindings/spi/atmel-quadspi.txt      | 28 ++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
> index e9dae6264d89..e7b7f297c5d7 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:    Only for sam9x60 - should contain two strigs:

							   ^strings

And I think naming clocks even for sama5d2 is a good practice, so I'd
suggest making "pclk" mandatory even if you support unnamed clk in the
driver to be backward compatible with old DTs.

> +		  - "pclk" for the peripheral clock
> +		  - "qspick" for the system clock
>  - #address-cells: Should be <1>.
>  - #size-cells:    Should be <0>.
>  
> @@ -29,3 +37,19 @@ spi@f0020000 {
>  		...
>  	};
>  };
> +
> +qspi@f0014000 {
> +	compatible = "microchip,sam9x60-qspi";
> +	reg = <0xf0014000 0x100>, <0x70000000 0x08000000>;
> +	reg-names = "qspi_base", "qspi_mmap";
> +	interrupts = <35 IRQ_TYPE_LEVEL_HIGH 7>;
> +	clocks =  <&pmc PMC_TYPE_PERIPHERAL 35>, <&pmc PMC_TYPE_SYSTEM 19>;
> +	clock-names = "pclk", "qspick";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	pinctrl-names = "default";
> +
> +	flash@0 {
> +		...
> +	};
> +};

No need to add one example per compat, especially when all that
changes is the compat string and an extra clk. 

Re: [PATCH 8/9] dt-bindings: spi: atmel-quadspi: QuadSPI driver for Microchip SAM9X60

[Tudor.Ambarus]

On 01/30/2019 07:30 PM, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 15:08:45 +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.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  .../devicetree/bindings/spi/atmel-quadspi.txt      | 28 ++++++++++++++++++++--
>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/atmel-quadspi.txt b/Documentation/devicetree/bindings/spi/atmel-quadspi.txt
>> index e9dae6264d89..e7b7f297c5d7 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:    Only for sam9x60 - should contain two strigs:
> 
> 							   ^strings
> 
> And I think naming clocks even for sama5d2 is a good practice, so I'd
> suggest making "pclk" mandatory even if you support unnamed clk in the
> driver to be backward compatible with old DTs.

agreed

> 
>> +		  - "pclk" for the peripheral clock
>> +		  - "qspick" for the system clock
>>  - #address-cells: Should be <1>.
>>  - #size-cells:    Should be <0>.
>>  
>> @@ -29,3 +37,19 @@ spi@f0020000 {
>>  		...
>>  	};
>>  };
>> +
>> +qspi@f0014000 {
>> +	compatible = "microchip,sam9x60-qspi";
>> +	reg = <0xf0014000 0x100>, <0x70000000 0x08000000>;
>> +	reg-names = "qspi_base", "qspi_mmap";
>> +	interrupts = <35 IRQ_TYPE_LEVEL_HIGH 7>;
>> +	clocks =  <&pmc PMC_TYPE_PERIPHERAL 35>, <&pmc PMC_TYPE_SYSTEM 19>;
>> +	clock-names = "pclk", "qspick";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	pinctrl-names = "default";
>> +
>> +	flash@0 {
>> +		...
>> +	};
>> +};
> 
> No need to add one example per compat, especially when all that
> changes is the compat string and an extra clk. 

ok, thanks!
ta

[PATCH 9/9] 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>
---
 drivers/spi/atmel-quadspi.c | 331 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 277 insertions(+), 54 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 1d21db7851e9..bfdc8488f23b 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,15 +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;
 	struct completion	cmd_completion;
 };
 
+struct atmel_qspi_ops {
+	int (*clk_prepare_enable)(struct atmel_qspi *aq);
+	void (*clk_disable_unprepare)(struct atmel_qspi *aq);
+	int (*set_qspi_cfg)(struct atmel_qspi *aq, const struct spi_mem_op *op,
+			    struct atmel_qspi_cfg *cfg);
+};
+
+struct atmel_qspi_caps {
+	const struct atmel_qspi_ops *ops;
+};
+
 struct atmel_qspi_mode {
 	u8 cmd_buswidth;
 	u8 addr_buswidth;
@@ -214,23 +243,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);
-	int mode;
-	u32 dummy_cycles = 0;
-	u32 iar, icr, ifr, sr;
-	int err = 0;
-
-	iar = 0;
-	icr = QSPI_ICR_INST(op->cmd.opcode);
-	ifr = QSPI_IFR_INSTEN;
+	int mode = atmel_qspi_find_mode(op);
 
-	mode = atmel_qspi_find_mode(op);
 	if (mode < 0)
 		return mode;
+	cfg->ifr = QSPI_IFR_INSTEN | 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;
@@ -240,28 +282,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;
@@ -270,24 +312,99 @@ 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);
 
-	/* Set data enable */
-	if (op->data.nbytes)
-		ifr |= QSPI_IFR_DATAEN;
+	return 0;
+}
+
+static int atmel_sama5d2_qspi_set_cfg(struct atmel_qspi *aq,
+				      const struct spi_mem_op *op,
+				      struct atmel_qspi_cfg *cfg)
+{
+	int ret = atmel_qspi_set_mode(cfg, op);
+
+	if (ret)
+		return ret;
+
+	cfg->icr = QSPI_ICR_INST(op->cmd.opcode);
 
 	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;
+
+	/* Set data enable */
+	if (op->data.nbytes)
+		cfg->ifr |= QSPI_IFR_DATAEN;
+
+	ret = atmel_qspi_set_address_mode(cfg, op);
+	if (ret)
+		return ret;
+
+	/* Clear pending interrupts */
+	(void)atmel_qspi_readl(aq, QSPI_SR);
+
+	/* Set QSPI Instruction Frame registers */
+	atmel_qspi_writel(aq, QSPI_IAR, cfg->iar);
+	atmel_qspi_writel(aq, QSPI_ICR, cfg->icr);
+	atmel_qspi_writel(aq, QSPI_IFR, cfg->ifr);
+
+	return 0;
+}
+
+static int atmel_sam9x60_qspi_set_cfg(struct atmel_qspi *aq,
+				      const struct spi_mem_op *op,
+				      struct atmel_qspi_cfg *cfg)
+{
+	int ret = atmel_qspi_set_mode(cfg, op);
+
+	if (ret)
+		return ret;
+
+	cfg->icr = QSPI_ICR_INST(op->cmd.opcode);
+
+	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;
+	}
+
+	/* Set data enable */
+	if (op->data.nbytes)
+		cfg->ifr |= QSPI_IFR_DATAEN;
+
+	ret = atmel_qspi_set_address_mode(cfg, op);
+	if (ret)
+		return ret;
 
 	/* Clear pending interrupts */
 	(void)atmel_qspi_readl(aq, QSPI_SR);
 
 	/* Set QSPI Instruction Frame registers */
-	atmel_qspi_writel(aq, QSPI_IAR, iar);
-	atmel_qspi_writel(aq, QSPI_ICR, icr);
-	atmel_qspi_writel(aq, QSPI_IFR, ifr);
+	atmel_qspi_writel(aq, QSPI_IAR, cfg->iar);
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		atmel_qspi_writel(aq, QSPI_ICR, cfg->icr);
+	else
+		atmel_qspi_writel(aq, QSPI_RICR, cfg->icr);
+	atmel_qspi_writel(aq, QSPI_IFR, cfg->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);
+	struct atmel_qspi_cfg cfg = {0};
+	u32 sr;
+	int err;
+
+	err = aq->caps->ops->set_qspi_cfg(aq, op, &cfg);
+	if (err)
+		return err;
 
 	/* Skip to the final steps if there is no data */
 	if (op->data.nbytes) {
@@ -296,11 +413,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 */
 		atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_LASTXFER);
@@ -395,10 +512,84 @@ static irqreturn_t atmel_qspi_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int atmel_sama5d2_qspi_clk_prepare_enable(struct atmel_qspi *aq)
+{
+	int ret;
+
+	if (!aq->clk) {
+		/* Get the peripheral clock */
+		aq->clk = devm_clk_get(&aq->pdev->dev, NULL);
+		if (IS_ERR(aq->clk)) {
+			dev_err(&aq->pdev->dev, "missing peripheral clock\n");
+			return PTR_ERR(aq->clk);
+		}
+	}
+
+	/* Enable the peripheral clock */
+	ret = clk_prepare_enable(aq->clk);
+	if (ret)
+		dev_err(&aq->pdev->dev,
+			"failed to enable the peripheral clock\n");
+
+	return ret;
+}
+
+static void atmel_sama5d2_qspi_clk_disable_unprepare(struct atmel_qspi *aq)
+{
+	clk_disable_unprepare(aq->clk);
+}
+
+static int atmel_sam9x60_qspi_clk_prepare_enable(struct atmel_qspi *aq)
+{
+	struct device *dev = &aq->pdev->dev;
+	int ret;
+
+	if (!aq->clk) {
+		/* Get the peripheral clock */
+		aq->clk = devm_clk_get(dev, "pclk");
+		if (IS_ERR(aq->clk)) {
+			dev_err(dev, "missing peripheral clock\n");
+			return PTR_ERR(aq->clk);
+		}
+	}
+
+	if (!aq->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");
+			return PTR_ERR(aq->qspick);
+		}
+	}
+
+	/* Enable the peripheral clock */
+	ret = clk_prepare_enable(aq->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable the peripheral clock\n");
+		return ret;
+	}
+
+	/* Enable the QSPI system clock */
+	ret = clk_prepare_enable(aq->qspick);
+	if (ret) {
+		dev_err(dev, "failed to enable the QSPI system clock\n");
+		clk_disable_unprepare(aq->clk);
+	}
+
+	return ret;
+}
+
+static void atmel_sam9x60_qspi_clk_disable_unprepare(struct atmel_qspi *aq)
+{
+	clk_disable_unprepare(aq->qspick);
+	clk_disable_unprepare(aq->clk);
+}
+
 static int atmel_qspi_probe(struct platform_device *pdev)
 {
 	struct spi_controller *ctrl;
 	struct atmel_qspi *aq;
+	const struct atmel_qspi_caps *caps;
 	struct resource *res;
 	int irq, err = 0;
 
@@ -437,20 +628,22 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 		goto exit;
 	}
 
-	/* Get the peripheral clock */
-	aq->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(aq->clk)) {
-		dev_err(&pdev->dev, "missing peripheral clock\n");
-		err = PTR_ERR(aq->clk);
-		goto exit;
+	caps = of_device_get_match_data(&pdev->dev);
+	if (!caps) {
+		dev_err(&pdev->dev, "Could not retrieve QSPI caps\n");
+		return -EINVAL;
 	}
 
-	/* Enable the peripheral clock */
-	err = clk_prepare_enable(aq->clk);
-	if (err) {
-		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
-		goto exit;
+	if (!caps->ops->clk_prepare_enable ||
+	    !caps->ops->clk_disable_unprepare || !caps->ops->set_qspi_cfg) {
+		dev_err(&pdev->dev, "Could not retrieve QSPI ops\n");
+		return -EINVAL;
 	}
+	aq->caps = caps;
+
+	err = caps->ops->clk_prepare_enable(aq);
+	if (err)
+		return err;
 
 	/* Request the IRQ */
 	irq = platform_get_irq(pdev, 0);
@@ -475,7 +668,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	return 0;
 
 disable_clk:
-	clk_disable_unprepare(aq->clk);
+	caps->ops->clk_disable_unprepare(aq);
 exit:
 	spi_controller_put(ctrl);
 
@@ -486,18 +679,20 @@ static int atmel_qspi_remove(struct platform_device *pdev)
 {
 	struct spi_controller *ctrl = platform_get_drvdata(pdev);
 	struct atmel_qspi *aq = spi_controller_get_devdata(ctrl);
+	const struct atmel_qspi_caps *caps = aq->caps;
 
 	spi_unregister_controller(ctrl);
 	atmel_qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
-	clk_disable_unprepare(aq->clk);
+	caps->ops->clk_disable_unprepare(aq);
 	return 0;
 }
 
 static int __maybe_unused atmel_qspi_suspend(struct device *dev)
 {
 	struct atmel_qspi *aq = dev_get_drvdata(dev);
+	const struct atmel_qspi_caps *caps = aq->caps;
 
-	clk_disable_unprepare(aq->clk);
+	caps->ops->clk_disable_unprepare(aq);
 
 	return 0;
 }
@@ -505,8 +700,9 @@ static int __maybe_unused atmel_qspi_suspend(struct device *dev)
 static int __maybe_unused atmel_qspi_resume(struct device *dev)
 {
 	struct atmel_qspi *aq = dev_get_drvdata(dev);
+	const struct atmel_qspi_caps *caps = aq->caps;
 
-	clk_prepare_enable(aq->clk);
+	caps->ops->clk_prepare_enable(aq);
 
 	return atmel_qspi_init(aq);
 }
@@ -514,8 +710,35 @@ 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 = {
+	.clk_prepare_enable = atmel_sama5d2_qspi_clk_prepare_enable,
+	.clk_disable_unprepare = atmel_sama5d2_qspi_clk_disable_unprepare,
+	.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 = {
+	.clk_prepare_enable = atmel_sam9x60_qspi_clk_prepare_enable,
+	.clk_disable_unprepare = atmel_sam9x60_qspi_clk_disable_unprepare,
+	.set_qspi_cfg = atmel_sam9x60_qspi_set_cfg,
+};
+
+static const struct atmel_qspi_caps atmel_sam9x60_qspi_caps = {
+	.ops = &atmel_sam9x60_qspi_ops,
+};
+
 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 9/9] spi: atmel-quadspi: add support for sam9x60 qspi controller

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:47 +0000
<Tudor.Ambarus@microchip.com> wrote:

> +static int atmel_sam9x60_qspi_clk_prepare_enable(struct atmel_qspi *aq)
> +{
> +	struct device *dev = &aq->pdev->dev;
> +	int ret;
> +
> +	if (!aq->clk) {
> +		/* Get the peripheral clock */
> +		aq->clk = devm_clk_get(dev, "pclk");
> +		if (IS_ERR(aq->clk)) {
> +			dev_err(dev, "missing peripheral clock\n");
> +			return PTR_ERR(aq->clk);
> +		}
> +	}
> +
> +	if (!aq->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");
> +			return PTR_ERR(aq->qspick);
> +		}
> +	}

Move the devm_clk_get() calls to the probe path instead of doing it at
prepare time, and you can make it generic for both compats with
something like:

	aq->clk = devm_clk_get(dev, "pclk");
	if (IS_ERR(aq->clk))
		aq->clk = devm_clk_get(dev, NULL);

	if (IS_ERR(aq->clk))
		return PTR_ERR(aq->clk);

	if (aq->caps->qspick_required) {
		aq->qspick = devm_clk_get(dev, "qspick");
		if (IS_ERR(aq->qspick)) {
			return PTR_ERR(aq->qspick);
	}

> +
> +	/* Enable the peripheral clock */
> +	ret = clk_prepare_enable(aq->clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable the peripheral clock\n");
> +		return ret;
> +	}
> +
> +	/* Enable the QSPI system clock */
> +	ret = clk_prepare_enable(aq->qspick);
> +	if (ret) {
> +		dev_err(dev, "failed to enable the QSPI system clock\n");
> +		clk_disable_unprepare(aq->clk);
> +	}

Again, you can make the enable function generic since
clk_prepare_enable(NULL) is a NO-OP that returns 0 and aq->qspick will
be NULL when it's not required.

> +
> +	return ret;
> +}
> +
> +static void atmel_sam9x60_qspi_clk_disable_unprepare(struct atmel_qspi *aq)
> +{
> +	clk_disable_unprepare(aq->qspick);
> +	clk_disable_unprepare(aq->clk);

Ditto.

> +}

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

[Tudor.Ambarus]

On 01/30/2019 07:43 PM, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 15:08:47 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> +static int atmel_sam9x60_qspi_clk_prepare_enable(struct atmel_qspi *aq)
>> +{
>> +	struct device *dev = &aq->pdev->dev;
>> +	int ret;
>> +
>> +	if (!aq->clk) {
>> +		/* Get the peripheral clock */
>> +		aq->clk = devm_clk_get(dev, "pclk");
>> +		if (IS_ERR(aq->clk)) {
>> +			dev_err(dev, "missing peripheral clock\n");
>> +			return PTR_ERR(aq->clk);
>> +		}
>> +	}
>> +
>> +	if (!aq->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");
>> +			return PTR_ERR(aq->qspick);
>> +		}
>> +	}
> 
> Move the devm_clk_get() calls to the probe path instead of doing it at
> prepare time, and you can make it generic for both compats with
> something like:
> 
> 	aq->clk = devm_clk_get(dev, "pclk");
> 	if (IS_ERR(aq->clk))
> 		aq->clk = devm_clk_get(dev, NULL);
> 
> 	if (IS_ERR(aq->clk))
> 		return PTR_ERR(aq->clk);
> 
> 	if (aq->caps->qspick_required) {
> 		aq->qspick = devm_clk_get(dev, "qspick");
> 		if (IS_ERR(aq->qspick)) {
> 			return PTR_ERR(aq->qspick);
> 	}

good tip, will do, thanks!

ta

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

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:47 +0000
<Tudor.Ambarus@microchip.com> wrote:

> +
> +static int atmel_sam9x60_qspi_set_cfg(struct atmel_qspi *aq,
> +				      const struct spi_mem_op *op,
> +				      struct atmel_qspi_cfg *cfg)
> +{
> +	int ret = atmel_qspi_set_mode(cfg, op);
> +
> +	if (ret)
> +		return ret;
> +
> +	cfg->icr = QSPI_ICR_INST(op->cmd.opcode);
> +
> +	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;

Why do you use a MEM transfer here? What's the difference with a
regular transfer?

> +	}
> +
> +	/* Set data enable */
> +	if (op->data.nbytes)
> +		cfg->ifr |= QSPI_IFR_DATAEN;
> +
> +	ret = atmel_qspi_set_address_mode(cfg, op);
> +	if (ret)
> +		return ret;
>  
>  	/* Clear pending interrupts */
>  	(void)atmel_qspi_readl(aq, QSPI_SR);
>  
>  	/* Set QSPI Instruction Frame registers */
> -	atmel_qspi_writel(aq, QSPI_IAR, iar);
> -	atmel_qspi_writel(aq, QSPI_ICR, icr);
> -	atmel_qspi_writel(aq, QSPI_IFR, ifr);
> +	atmel_qspi_writel(aq, QSPI_IAR, cfg->iar);
> +	if (op->data.dir == SPI_MEM_DATA_OUT)
> +		atmel_qspi_writel(aq, QSPI_ICR, cfg->icr);
> +	else
> +		atmel_qspi_writel(aq, QSPI_RICR, cfg->icr);
> +	atmel_qspi_writel(aq, QSPI_IFR, cfg->ifr);
> +
> +	return 0;
> +}

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

[Tudor.Ambarus]

On 01/31/2019 01:55 PM, Boris Brezillon wrote:
> On Wed, 30 Jan 2019 15:08:47 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> +
>> +static int atmel_sam9x60_qspi_set_cfg(struct atmel_qspi *aq,
>> +				      const struct spi_mem_op *op,
>> +				      struct atmel_qspi_cfg *cfg)
>> +{
>> +	int ret = atmel_qspi_set_mode(cfg, op);
>> +
>> +	if (ret)
>> +		return ret;
>> +
>> +	cfg->icr = QSPI_ICR_INST(op->cmd.opcode);
>> +
>> +	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;
> 
> Why do you use a MEM transfer here? What's the difference with a
> regular transfer?

QSPI_IFR_TFRTYP_TRSFR_MEM must be set when one wants to read/write in the serial
memory, and particularly a memory data.

QSPI_IFR_TFRTYP_TRSFR_REG must be set when one wants to read or write to serial
memory, but not a memory data.
Read examples: JEDEC_ID or QSPI_SR
Write examples: writing the configuration or the QSPI_SR.

Does this answers your question?

> 
>> +	}
>> +
>> +	/* Set data enable */
>> +	if (op->data.nbytes)
>> +		cfg->ifr |= QSPI_IFR_DATAEN;
>> +
>> +	ret = atmel_qspi_set_address_mode(cfg, op);
>> +	if (ret)
>> +		return ret;
>>  
>>  	/* Clear pending interrupts */
>>  	(void)atmel_qspi_readl(aq, QSPI_SR);
>>  
>>  	/* Set QSPI Instruction Frame registers */
>> -	atmel_qspi_writel(aq, QSPI_IAR, iar);
>> -	atmel_qspi_writel(aq, QSPI_ICR, icr);
>> -	atmel_qspi_writel(aq, QSPI_IFR, ifr);
>> +	atmel_qspi_writel(aq, QSPI_IAR, cfg->iar);
>> +	if (op->data.dir == SPI_MEM_DATA_OUT)
>> +		atmel_qspi_writel(aq, QSPI_ICR, cfg->icr);
>> +	else
>> +		atmel_qspi_writel(aq, QSPI_RICR, cfg->icr);
>> +	atmel_qspi_writel(aq, QSPI_IFR, cfg->ifr);
>> +
>> +	return 0;
>> +}
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

[Boris Brezillon]

On Thu, 31 Jan 2019 12:40:04 +0000
<Tudor.Ambarus@microchip.com> wrote:

> On 01/31/2019 01:55 PM, Boris Brezillon wrote:
> > On Wed, 30 Jan 2019 15:08:47 +0000
> > <Tudor.Ambarus@microchip.com> wrote:
> >   
> >> +
> >> +static int atmel_sam9x60_qspi_set_cfg(struct atmel_qspi *aq,
> >> +				      const struct spi_mem_op *op,
> >> +				      struct atmel_qspi_cfg *cfg)
> >> +{
> >> +	int ret = atmel_qspi_set_mode(cfg, op);
> >> +
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	cfg->icr = QSPI_ICR_INST(op->cmd.opcode);
> >> +
> >> +	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;  
> > 
> > Why do you use a MEM transfer here? What's the difference with a
> > regular transfer?  
> 
> QSPI_IFR_TFRTYP_TRSFR_MEM must be set when one wants to read/write in the serial
> memory, and particularly a memory data.
> 
> QSPI_IFR_TFRTYP_TRSFR_REG must be set when one wants to read or write to serial
> memory, but not a memory data.
> Read examples: JEDEC_ID or QSPI_SR
> Write examples: writing the configuration or the QSPI_SR.
> 
> Does this answers your question?

Not really :-). From the SPI bus perspective, there's no difference
between a read/write from/to actual memory blocks or a read/write
reg/param-page, so there must be something different on the controller
side. I think regular transfers should work for anything, which is why
I initially suggested to use that in the ->exec_op() implementation.
If memory accesses are optimized somehow and do not work for all
accesses, then we should keep them for the dirmap implementation.

After reading the sama5d2 datasheet, I have the feeling that the only
difference is the fact that the address is directly extracted from the
AHB window offset when using mem accesses (instead of being taken from
the IAR register), plus the possibility to enable the data
scrambler/randomize.

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

[Boris Brezillon]

On Wed, 30 Jan 2019 15:08:47 +0000
<Tudor.Ambarus@microchip.com> wrote:

> +/*
> + * 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.

Too bad they didn't patch the IP to support 1 to 4 address bytes
instead of only 3 or 4 :-(.