[U-Boot] [PATCH v3 00/11] SF: Cadence QSPI driver fixes and clean up

[U-Boot] [PATCH v3 00/11] SF: Cadence QSPI driver fixes and clean up

[Phil Edworthy]

This series has fixes, patches to clean the code up, and add support for
specifying the sampling edge.

Main changes in v3:
  1. Added "spi: cadence_qspi: Use spi mode at the point it is needed" to
     address comments for SPI pol/phase code.
  2. "spi: cadence_qspi: Support specifying the sample edge used" has been
     split into 3 separate patches.


Phil Edworthy (11):
  spi: cadence_qspi: Fix clearing of pol/pha bits
  spi: cadence_qspi: Fix baud rate calculation
  spi: cadence_qspi: Better debug information on the SPI clock rate
  spi: cadence_qspi: Use #define for bits instead of bit shifts
  spi: cadence_qspi: Clean up the #define names
  spi: cadence_qspi: Use spi mode at the point it is needed
  spi: cadence_qspi: Remove returns from end of void functions
  spi: cadence_qspi: Fix CS timings
  spi: cadence_qspi: Move DT prop code to match layout
  spi: cadence_qspi: Change readdata_capture() arg to bool
  spi: cadence_qspi: Support specifying the sample edge used

 doc/device-tree-bindings/spi/spi-cadence.txt |   2 +
 drivers/spi/cadence_qspi.c                   |  17 ++-
 drivers/spi/cadence_qspi.h                   |   6 +-
 drivers/spi/cadence_qspi_apb.c               | 193 +++++++++++++--------------
 4 files changed, 106 insertions(+), 112 deletions(-)

-- 
2.7.4

[U-Boot] [PATCH v3 01/11] spi: cadence_qspi: Fix clearing of pol/pha bits

[Phil Edworthy]

Or'ing together bit positions is clearly wrong.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Marek Vasut <marek.vasut@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e285d3c..2403e71 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -311,8 +311,8 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
 	cadence_qspi_apb_controller_disable(reg_base);
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
-	reg &= ~(1 <<
-		(CQSPI_REG_CONFIG_CLK_POL_LSB | CQSPI_REG_CONFIG_CLK_PHA_LSB));
+	reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
+	reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
 
 	reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
 	reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
-- 
2.7.4

[U-Boot] [PATCH v3 02/11] spi: cadence_qspi: Fix baud rate calculation

[Phil Edworthy]

With the existing code, when the requested SPI clock rate is near
to the lowest that can be achieved by the hardware (max divider
of the ref clock is 32), the generated clock rate is wrong.
For example, with a 50MHz ref clock, when asked for anything less
than a 1.5MHz SPI clock, the code sets up the divider to generate
25MHz.

This change fixes the calculation.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v3:
 - Use single line DIV_ROUND_UP instead of two
v2:
 - Use the DIV_ROUND_UP macro
---
 drivers/spi/cadence_qspi_apb.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2403e71..b5c664f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -273,22 +273,12 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	reg &= ~(CQSPI_REG_CONFIG_BAUD_MASK << CQSPI_REG_CONFIG_BAUD_LSB);
 
-	div = ref_clk_hz / sclk_hz;
-
-	if (div > 32)
-		div = 32;
-
-	/* Check if even number. */
-	if ((div & 1)) {
-		div = (div / 2);
-	} else {
-		if (ref_clk_hz % sclk_hz)
-			/* ensure generated SCLK doesn't exceed user
-			specified sclk_hz */
-			div = (div / 2);
-		else
-			div = (div / 2) - 1;
-	}
+	/*
+	 * The baud_div field in the config reg is 4 bits, and the ref clock is
+	 * divided by 2 * (baud_div + 1). Round up the divider to ensure the
+	 * SPI clock rate is less than or equal to the requested clock rate.
+	 */
+	div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) - 1;
 
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
-- 
2.7.4

[U-Boot] [PATCH v3 03/11] spi: cadence_qspi: Better debug information on the SPI clock rate

[Phil Edworthy]

Show what the output clock rate actually is.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Marek Vasut <marek.vasut@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b5c664f..0a2963d 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -280,13 +280,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	 */
 	div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) - 1;
 
-	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
-	      ref_clk_hz, sclk_hz, div);
-
 	/* ensure the baud rate doesn't exceed the max value */
 	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
 		div = CQSPI_REG_CONFIG_BAUD_MASK;
 
+	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x, actual %dHz\n", __func__,
+	      ref_clk_hz, sclk_hz, div, ref_clk_hz / (2 * (div + 1)));
+
 	reg |= (div << CQSPI_REG_CONFIG_BAUD_LSB);
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

[U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Most of the code already uses #defines for the bit value, rather
than the shift required to get the value. This changes the remaining
code over.

Whislt at it, fix the names of the "Rd Data Capture" register defs.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Marek Vasut <marek.vasut@gmail.com>
---
 v3:
 - Remove brackets that aren't needed.
 v2:
 - No change.
---
 drivers/spi/cadence_qspi_apb.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 0a2963d..b41f36b 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -57,9 +57,9 @@
  * Controller's configuration and status register (offset from QSPI_BASE)
  ****************************************************************************/
 #define	CQSPI_REG_CONFIG			0x00
-#define	CQSPI_REG_CONFIG_CLK_POL_LSB		1
-#define	CQSPI_REG_CONFIG_CLK_PHA_LSB		2
 #define	CQSPI_REG_CONFIG_ENABLE_MASK		BIT(0)
+#define	CQSPI_REG_CONFIG_CLK_POL		BIT(1)
+#define	CQSPI_REG_CONFIG_CLK_PHA		BIT(2)
 #define	CQSPI_REG_CONFIG_DIRECT_MASK		BIT(7)
 #define	CQSPI_REG_CONFIG_DECODE_MASK		BIT(9)
 #define	CQSPI_REG_CONFIG_XIP_IMM_MASK		BIT(18)
@@ -94,10 +94,10 @@
 #define	CQSPI_REG_DELAY_TSD2D_MASK		0xFF
 #define	CQSPI_REG_DELAY_TSHSL_MASK		0xFF
 
-#define	CQSPI_READLCAPTURE			0x10
-#define	CQSPI_READLCAPTURE_BYPASS_LSB		0
-#define	CQSPI_READLCAPTURE_DELAY_LSB		1
-#define	CQSPI_READLCAPTURE_DELAY_MASK		0xF
+#define	CQSPI_REG_RD_DATA_CAPTURE		0x10
+#define	CQSPI_REG_RD_DATA_CAPTURE_BYPASS	BIT(0)
+#define	CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB	1
+#define	CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK	0xF
 
 #define	CQSPI_REG_SIZE				0x14
 #define	CQSPI_REG_SIZE_ADDRESS_LSB		0
@@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
 	unsigned int reg;
 	cadence_qspi_apb_controller_disable(reg_base);
 
-	reg = readl(reg_base + CQSPI_READLCAPTURE);
+	reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
 	if (bypass)
-		reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+		reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
 	else
-		reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
+		reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
 
-	reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
-		<< CQSPI_READLCAPTURE_DELAY_LSB);
+	reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
+		<< CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
 
-	reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
-		<< CQSPI_READLCAPTURE_DELAY_LSB);
+	reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
+		<< CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
 
-	writel(reg, reg_base + CQSPI_READLCAPTURE);
+	writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
 	cadence_qspi_apb_controller_enable(reg_base);
 	return;
@@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 
 	cadence_qspi_apb_controller_disable(reg_base);
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
-	reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
-	reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+	reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
 
-	reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
-	reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
+	if (clk_pol)
+		reg |= CQSPI_REG_CONFIG_CLK_POL;
+	if (clk_pha)
+		reg |= CQSPI_REG_CONFIG_CLK_PHA;
 
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
-- 
2.7.4

[U-Boot] [PATCH v3 05/11] spi: cadence_qspi: Clean up the #define names

[Phil Edworthy]

A lot of the #defines are for single bits in a register, where the
name has _MASK on the end. Since this can be used for both a mask
and the value, remove _MASK from them.

Whilst doing so, also remove the unnecessary brackets around the
constants.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Marek Vasut <marek.vasut@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 86 +++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b41f36b..634a857 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -32,37 +32,37 @@
 #include <spi.h>
 #include "cadence_qspi.h"
 
-#define CQSPI_REG_POLL_US			(1) /* 1us */
-#define CQSPI_REG_RETRY				(10000)
-#define CQSPI_POLL_IDLE_RETRY			(3)
+#define CQSPI_REG_POLL_US			1 /* 1us */
+#define CQSPI_REG_RETRY				10000
+#define CQSPI_POLL_IDLE_RETRY			3
 
-#define CQSPI_FIFO_WIDTH			(4)
+#define CQSPI_FIFO_WIDTH			4
 
-#define CQSPI_REG_SRAM_THRESHOLD_WORDS		(50)
+#define CQSPI_REG_SRAM_THRESHOLD_WORDS		50
 
 /* Transfer mode */
-#define CQSPI_INST_TYPE_SINGLE			(0)
-#define CQSPI_INST_TYPE_DUAL			(1)
-#define CQSPI_INST_TYPE_QUAD			(2)
+#define CQSPI_INST_TYPE_SINGLE			0
+#define CQSPI_INST_TYPE_DUAL			1
+#define CQSPI_INST_TYPE_QUAD			2
 
-#define CQSPI_STIG_DATA_LEN_MAX			(8)
-
-#define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
-#define CQSPI_DUMMY_BYTES_MAX			(4)
+#define CQSPI_STIG_DATA_LEN_MAX			8
 
+#define CQSPI_DUMMY_CLKS_PER_BYTE		8
+#define CQSPI_DUMMY_BYTES_MAX			4
 
 #define CQSPI_REG_SRAM_FILL_THRESHOLD	\
 	((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
+
 /****************************************************************************
  * Controller's configuration and status register (offset from QSPI_BASE)
  ****************************************************************************/
 #define	CQSPI_REG_CONFIG			0x00
-#define	CQSPI_REG_CONFIG_ENABLE_MASK		BIT(0)
+#define	CQSPI_REG_CONFIG_ENABLE			BIT(0)
 #define	CQSPI_REG_CONFIG_CLK_POL		BIT(1)
 #define	CQSPI_REG_CONFIG_CLK_PHA		BIT(2)
-#define	CQSPI_REG_CONFIG_DIRECT_MASK		BIT(7)
-#define	CQSPI_REG_CONFIG_DECODE_MASK		BIT(9)
-#define	CQSPI_REG_CONFIG_XIP_IMM_MASK		BIT(18)
+#define	CQSPI_REG_CONFIG_DIRECT			BIT(7)
+#define	CQSPI_REG_CONFIG_DECODE			BIT(9)
+#define	CQSPI_REG_CONFIG_XIP_IMM		BIT(18)
 #define	CQSPI_REG_CONFIG_CHIPSELECT_LSB		10
 #define	CQSPI_REG_CONFIG_BAUD_LSB		19
 #define	CQSPI_REG_CONFIG_IDLE_LSB		31
@@ -123,18 +123,18 @@
 #define	CQSPI_REG_IRQMASK			0x44
 
 #define	CQSPI_REG_INDIRECTRD			0x60
-#define	CQSPI_REG_INDIRECTRD_START_MASK		BIT(0)
-#define	CQSPI_REG_INDIRECTRD_CANCEL_MASK	BIT(1)
-#define	CQSPI_REG_INDIRECTRD_INPROGRESS_MASK	BIT(2)
-#define	CQSPI_REG_INDIRECTRD_DONE_MASK		BIT(5)
+#define	CQSPI_REG_INDIRECTRD_START		BIT(0)
+#define	CQSPI_REG_INDIRECTRD_CANCEL		BIT(1)
+#define	CQSPI_REG_INDIRECTRD_INPROGRESS		BIT(2)
+#define	CQSPI_REG_INDIRECTRD_DONE		BIT(5)
 
 #define	CQSPI_REG_INDIRECTRDWATERMARK		0x64
 #define	CQSPI_REG_INDIRECTRDSTARTADDR		0x68
 #define	CQSPI_REG_INDIRECTRDBYTES		0x6C
 
 #define	CQSPI_REG_CMDCTRL			0x90
-#define	CQSPI_REG_CMDCTRL_EXECUTE_MASK		BIT(0)
-#define	CQSPI_REG_CMDCTRL_INPROGRESS_MASK	BIT(1)
+#define	CQSPI_REG_CMDCTRL_EXECUTE		BIT(0)
+#define	CQSPI_REG_CMDCTRL_INPROGRESS		BIT(1)
 #define	CQSPI_REG_CMDCTRL_DUMMY_LSB		7
 #define	CQSPI_REG_CMDCTRL_WR_BYTES_LSB		12
 #define	CQSPI_REG_CMDCTRL_WR_EN_LSB		15
@@ -150,10 +150,10 @@
 #define	CQSPI_REG_CMDCTRL_OPCODE_MASK		0xFF
 
 #define	CQSPI_REG_INDIRECTWR			0x70
-#define	CQSPI_REG_INDIRECTWR_START_MASK		BIT(0)
-#define	CQSPI_REG_INDIRECTWR_CANCEL_MASK	BIT(1)
-#define	CQSPI_REG_INDIRECTWR_INPROGRESS_MASK	BIT(2)
-#define	CQSPI_REG_INDIRECTWR_DONE_MASK		BIT(5)
+#define	CQSPI_REG_INDIRECTWR_START		BIT(0)
+#define	CQSPI_REG_INDIRECTWR_CANCEL		BIT(1)
+#define	CQSPI_REG_INDIRECTWR_INPROGRESS		BIT(2)
+#define	CQSPI_REG_INDIRECTWR_DONE		BIT(5)
 
 #define	CQSPI_REG_INDIRECTWRWATERMARK		0x74
 #define	CQSPI_REG_INDIRECTWRSTARTADDR		0x78
@@ -197,7 +197,7 @@ void cadence_qspi_apb_controller_enable(void *reg_base)
 {
 	unsigned int reg;
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
-	reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
+	reg |= CQSPI_REG_CONFIG_ENABLE;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 	return;
 }
@@ -206,7 +206,7 @@ void cadence_qspi_apb_controller_disable(void *reg_base)
 {
 	unsigned int reg;
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
-	reg &= ~CQSPI_REG_CONFIG_ENABLE_MASK;
+	reg &= ~CQSPI_REG_CONFIG_ENABLE;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 	return;
 }
@@ -327,9 +327,9 @@ void cadence_qspi_apb_chipselect(void *reg_base,
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	/* docoder */
 	if (decoder_enable) {
-		reg |= CQSPI_REG_CONFIG_DECODE_MASK;
+		reg |= CQSPI_REG_CONFIG_DECODE;
 	} else {
-		reg &= ~CQSPI_REG_CONFIG_DECODE_MASK;
+		reg &= ~CQSPI_REG_CONFIG_DECODE;
 		/* Convert CS if without decoder.
 		 * CS0 to 4b'1110
 		 * CS1 to 4b'1101
@@ -423,12 +423,12 @@ static int cadence_qspi_apb_exec_flash_cmd(void *reg_base,
 	/* Write the CMDCTRL without start execution. */
 	writel(reg, reg_base + CQSPI_REG_CMDCTRL);
 	/* Start execute */
-	reg |= CQSPI_REG_CMDCTRL_EXECUTE_MASK;
+	reg |= CQSPI_REG_CMDCTRL_EXECUTE;
 	writel(reg, reg_base + CQSPI_REG_CMDCTRL);
 
 	while (retry--) {
 		reg = readl(reg_base + CQSPI_REG_CMDCTRL);
-		if ((reg & CQSPI_REG_CMDCTRL_INPROGRESS_MASK) == 0)
+		if ((reg & CQSPI_REG_CMDCTRL_INPROGRESS) == 0)
 			break;
 		udelay(1);
 	}
@@ -646,7 +646,7 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 	writel(n_rx, plat->regbase + CQSPI_REG_INDIRECTRDBYTES);
 
 	/* Start the indirect read transfer */
-	writel(CQSPI_REG_INDIRECTRD_START_MASK,
+	writel(CQSPI_REG_INDIRECTRD_START,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 
 	while (remaining > 0) {
@@ -675,21 +675,21 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 
 	/* Check indirect done status */
 	ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_INDIRECTRD,
-			   CQSPI_REG_INDIRECTRD_DONE_MASK, 1, 10, 0);
+			   CQSPI_REG_INDIRECTRD_DONE, 1, 10, 0);
 	if (ret) {
 		printf("Indirect read completion error (%i)\n", ret);
 		goto failrd;
 	}
 
 	/* Clear indirect completion status */
-	writel(CQSPI_REG_INDIRECTRD_DONE_MASK,
+	writel(CQSPI_REG_INDIRECTRD_DONE,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 
 	return 0;
 
 failrd:
 	/* Cancel the indirect read */
-	writel(CQSPI_REG_INDIRECTRD_CANCEL_MASK,
+	writel(CQSPI_REG_INDIRECTRD_CANCEL,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 	return ret;
 }
@@ -737,7 +737,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 	writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
 
 	/* Start the indirect write transfer */
-	writel(CQSPI_REG_INDIRECTWR_START_MASK,
+	writel(CQSPI_REG_INDIRECTWR_START,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
 
 	while (remaining > 0) {
@@ -762,20 +762,20 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 
 	/* Check indirect done status */
 	ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_INDIRECTWR,
-			   CQSPI_REG_INDIRECTWR_DONE_MASK, 1, 10, 0);
+			   CQSPI_REG_INDIRECTWR_DONE, 1, 10, 0);
 	if (ret) {
 		printf("Indirect write completion error (%i)\n", ret);
 		goto failwr;
 	}
 
 	/* Clear indirect completion status */
-	writel(CQSPI_REG_INDIRECTWR_DONE_MASK,
+	writel(CQSPI_REG_INDIRECTWR_DONE,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
 	return 0;
 
 failwr:
 	/* Cancel the indirect write */
-	writel(CQSPI_REG_INDIRECTWR_CANCEL_MASK,
+	writel(CQSPI_REG_INDIRECTWR_CANCEL,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
 	return ret;
 }
@@ -786,9 +786,9 @@ void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy)
 
 	/* enter XiP mode immediately and enable direct mode */
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
-	reg |= CQSPI_REG_CONFIG_ENABLE_MASK;
-	reg |= CQSPI_REG_CONFIG_DIRECT_MASK;
-	reg |= CQSPI_REG_CONFIG_XIP_IMM_MASK;
+	reg |= CQSPI_REG_CONFIG_ENABLE;
+	reg |= CQSPI_REG_CONFIG_DIRECT;
+	reg |= CQSPI_REG_CONFIG_XIP_IMM;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
 	/* keep the XiP mode */
-- 
2.7.4

[U-Boot] [PATCH v3 06/11] spi: cadence_qspi: Use spi mode at the point it is needed

[Phil Edworthy]

Instead of extracting mode settings and passing them as separate
args to another function, just pass the SPI mode as an arg.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 v3:
  - New patch introduced to address comments.
---
 drivers/spi/cadence_qspi.c     | 4 +---
 drivers/spi/cadence_qspi.h     | 3 +--
 drivers/spi/cadence_qspi_apb.c | 7 +++----
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1051afb..55192d6 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -170,14 +170,12 @@ static int cadence_spi_probe(struct udevice *bus)
 static int cadence_spi_set_mode(struct udevice *bus, uint mode)
 {
 	struct cadence_spi_priv *priv = dev_get_priv(bus);
-	unsigned int clk_pol = (mode & SPI_CPOL) ? 1 : 0;
-	unsigned int clk_pha = (mode & SPI_CPHA) ? 1 : 0;
 
 	/* Disable QSPI */
 	cadence_qspi_apb_controller_disable(priv->regbase);
 
 	/* Set SPI mode */
-	cadence_qspi_apb_set_clk_mode(priv->regbase, clk_pol, clk_pha);
+	cadence_qspi_apb_set_clk_mode(priv->regbase, mode);
 
 	/* Enable QSPI */
 	cadence_qspi_apb_controller_enable(priv->regbase);
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index a849f7b..d1927a4 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -63,8 +63,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 
 void cadence_qspi_apb_chipselect(void *reg_base,
 	unsigned int chip_select, unsigned int decoder_enable);
-void cadence_qspi_apb_set_clk_mode(void *reg_base_addr,
-	unsigned int clk_pol, unsigned int clk_pha);
+void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode);
 void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	unsigned int ref_clk_hz, unsigned int sclk_hz);
 void cadence_qspi_apb_delay(void *reg_base,
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 634a857..e81d678 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -294,8 +294,7 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	return;
 }
 
-void cadence_qspi_apb_set_clk_mode(void *reg_base,
-	unsigned int clk_pol, unsigned int clk_pha)
+void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode)
 {
 	unsigned int reg;
 
@@ -303,9 +302,9 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
 
-	if (clk_pol)
+	if (mode & SPI_CPOL)
 		reg |= CQSPI_REG_CONFIG_CLK_POL;
-	if (clk_pha)
+	if (mode & SPI_CPHA)
 		reg |= CQSPI_REG_CONFIG_CLK_PHA;
 
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
-- 
2.7.4

[U-Boot] [PATCH v3 07/11] spi: cadence_qspi: Remove returns from end of void functions

[Phil Edworthy]

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Acked-by: Marek Vasut <marek.vasut@gmail.com>
---
 v3:
  - No change.
 v2:
  - No change.
---
 drivers/spi/cadence_qspi_apb.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index e81d678..39e31f6 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -199,7 +199,6 @@ void cadence_qspi_apb_controller_enable(void *reg_base)
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	reg |= CQSPI_REG_CONFIG_ENABLE;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
-	return;
 }
 
 void cadence_qspi_apb_controller_disable(void *reg_base)
@@ -208,7 +207,6 @@ void cadence_qspi_apb_controller_disable(void *reg_base)
 	reg = readl(reg_base + CQSPI_REG_CONFIG);
 	reg &= ~CQSPI_REG_CONFIG_ENABLE;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
-	return;
 }
 
 /* Return 1 if idle, otherwise return 0 (busy). */
@@ -260,7 +258,6 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
 	writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_config_baudrate_div(void *reg_base,
@@ -291,7 +288,6 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode)
@@ -310,7 +306,6 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base, uint mode)
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_chipselect(void *reg_base,
@@ -345,7 +340,6 @@ void cadence_qspi_apb_chipselect(void *reg_base,
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_delay(void *reg_base,
@@ -383,7 +377,6 @@ void cadence_qspi_apb_delay(void *reg_base,
 	writel(reg, reg_base + CQSPI_REG_DELAY);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
@@ -411,7 +404,6 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
 	writel(0, plat->regbase + CQSPI_REG_IRQMASK);
 
 	cadence_qspi_apb_controller_enable(plat->regbase);
-	return;
 }
 
 static int cadence_qspi_apb_exec_flash_cmd(void *reg_base,
-- 
2.7.4

[U-Boot] [PATCH v3 08/11] spi: cadence_qspi: Fix CS timings

[Phil Edworthy]

The Cadence QSPI controller has specified overheads for the various CS
times that are in addition to those programmed in to the Device Delay
register. The overheads are different for the delays.

In addition, the existing code does not handle the case when the delay
is less than a SCLK period.

This change accurately calculates the additional delays in Ref clocks.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 v3:
  - No change.
 v2:
 Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
 Note only did the existing code not cope with the delay less than
 an SCLK period, but the calculation didn't round properly, and
 didn't take into account the delays that the QSPI Controller adds
 to those programmed into the Device Delay reg.
---
 drivers/spi/cadence_qspi_apb.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 39e31f6..df6a91f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -169,9 +169,6 @@
 	((readl(base + CQSPI_REG_CONFIG) >>		\
 		CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
 
-#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)		\
-	((((tdelay_ns) - (tsclk_ns)) / (tref_ns)))
-
 #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)			\
 	(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>	\
 	CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
@@ -355,16 +352,20 @@ void cadence_qspi_apb_delay(void *reg_base,
 	cadence_qspi_apb_controller_disable(reg_base);
 
 	/* Convert to ns. */
-	ref_clk_ns = (1000000000) / ref_clk;
+	ref_clk_ns = DIV_ROUND_UP(1000000000, ref_clk);
 
 	/* Convert to ns. */
-	sclk_ns = (1000000000) / sclk_hz;
-
-	/* Plus 1 to round up 1 clock cycle. */
-	tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
-	tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
-	tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
-	tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
+	sclk_ns = DIV_ROUND_UP(1000000000, sclk_hz);
+
+	/* The controller adds additional delay to that programmed in the reg */
+	if (tshsl_ns >= sclk_ns + ref_clk_ns)
+		tshsl_ns -= sclk_ns + ref_clk_ns;
+	if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
+		tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
+	tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
+	tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
+	tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
+	tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
 
 	reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
 			<< CQSPI_REG_DELAY_TSHSL_LSB);
-- 
2.7.4

[U-Boot] [PATCH v3 09/11] spi: cadence_qspi: Move DT prop code to match layout

[Phil Edworthy]

Move the code to read the "sram-size" property into the other code
that reads properties from the node, rather than the SF subnode.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 v3:
  - New patch to split changes.
---
 drivers/spi/cadence_qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 55192d6..f16f90d 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -296,6 +296,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 
 	plat->regbase = (void *)data[0];
 	plat->ahbbase = (void *)data[2];
+	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
 	/* All other paramters are embedded in the child node */
 	subnode = fdt_first_subnode(blob, node);
@@ -315,7 +316,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
 	plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
 	plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
-	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
 	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
 	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
-- 
2.7.4

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

This is in preparation for adding another arg.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 v3:
  - New patch to split changes.
---
 drivers/spi/cadence_qspi.c     | 7 ++++---
 drivers/spi/cadence_qspi.h     | 2 +-
 drivers/spi/cadence_qspi_apb.c | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index f16f90d..1c4ed33 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -49,7 +49,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
 	cadence_spi_write_speed(bus, 1000000);
 
 	/* configure the read data capture delay register to 0 */
-	cadence_qspi_apb_readdata_capture(base, 1, 0);
+	cadence_qspi_apb_readdata_capture(base, true, 0);
 
 	/* Enable QSPI */
 	cadence_qspi_apb_controller_enable(base);
@@ -69,7 +69,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
 		cadence_qspi_apb_controller_disable(base);
 
 		/* reconfigure the read data capture delay register */
-		cadence_qspi_apb_readdata_capture(base, 1, i);
+		cadence_qspi_apb_readdata_capture(base, true, i);
 
 		/* Enable back QSPI */
 		cadence_qspi_apb_controller_enable(base);
@@ -105,7 +105,8 @@ static int spi_calibration(struct udevice *bus, uint hz)
 	cadence_qspi_apb_controller_disable(base);
 
 	/* configure the final value for read data capture delay register */
-	cadence_qspi_apb_readdata_capture(base, 1, (range_hi + range_lo) / 2);
+	cadence_qspi_apb_readdata_capture(base, true,
+		(range_hi + range_lo) / 2);
 	debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
 	      (range_hi + range_lo) / 2, range_lo, range_hi);
 
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index d1927a4..50c6e7c 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -72,6 +72,6 @@ void cadence_qspi_apb_delay(void *reg_base,
 	unsigned int tchsh_ns, unsigned int tslch_ns);
 void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy);
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-	unsigned int bypass, unsigned int delay);
+	bool bypass, unsigned int delay);
 
 #endif /* __CADENCE_QSPI_H__ */
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index df6a91f..f2cd852 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -234,7 +234,7 @@ static unsigned int cadence_qspi_wait_idle(void *reg_base)
 }
 
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-				unsigned int bypass, unsigned int delay)
+	bool bypass, unsigned int delay)
 {
 	unsigned int reg;
 	cadence_qspi_apb_controller_disable(reg_base);
-- 
2.7.4

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Introduce a new DT property to specify whether the QSPI Controller
samples the data on a rising edge. The default is falling edge.
Some versions of the QSPI Controller do not implement this bit, in
which case the property should be omitted.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 v3:
  - Patch split so this one only has code related to the subject.
  - Commit message updated.
 v2:
  - Change name of DT prop and provide details of what it does.
    Whilst at it, move the code to read the "sram-size" property
    into the other code that reads properties from the node, rather
    than the SF subnode.

    Also change the code to use a bool for the bypass arg.
---
 doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
 drivers/spi/cadence_qspi.c                   | 10 +++++++---
 drivers/spi/cadence_qspi.h                   |  3 ++-
 drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-bindings/spi/spi-cadence.txt
index c1e2233..94c800b 100644
--- a/doc/device-tree-bindings/spi/spi-cadence.txt
+++ b/doc/device-tree-bindings/spi/spi-cadence.txt
@@ -26,3 +26,5 @@ connected flash properties
 			  select (n_ss_out).
 - tslch-ns		: Delay in master reference clocks between setting
 			  n_ss_out low and first bit transfer
+- sample-edge-rising	: Data outputs from flash memory will be sampled on the
+			  rising edge. Default is falling edge.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1c4ed33..6fa917d 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -39,8 +39,10 @@ static int cadence_spi_write_speed(struct udevice *bus, uint hz)
 /* Calibration sequence to determine the read data capture delay register */
 static int spi_calibration(struct udevice *bus, uint hz)
 {
+	struct cadence_spi_platdata *plat = bus->platdata;
 	struct cadence_spi_priv *priv = dev_get_priv(bus);
 	void *base = priv->regbase;
+	bool edge = plat->sample_edge_rising;
 	u8 opcode_rdid = 0x9F;
 	unsigned int idcode = 0, temp = 0;
 	int err = 0, i, range_lo = -1, range_hi = -1;
@@ -49,7 +51,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
 	cadence_spi_write_speed(bus, 1000000);
 
 	/* configure the read data capture delay register to 0 */
-	cadence_qspi_apb_readdata_capture(base, true, 0);
+	cadence_qspi_apb_readdata_capture(base, true, edge, 0);
 
 	/* Enable QSPI */
 	cadence_qspi_apb_controller_enable(base);
@@ -69,7 +71,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
 		cadence_qspi_apb_controller_disable(base);
 
 		/* reconfigure the read data capture delay register */
-		cadence_qspi_apb_readdata_capture(base, true, i);
+		cadence_qspi_apb_readdata_capture(base, true, edge, i);
 
 		/* Enable back QSPI */
 		cadence_qspi_apb_controller_enable(base);
@@ -105,7 +107,7 @@ static int spi_calibration(struct udevice *bus, uint hz)
 	cadence_qspi_apb_controller_disable(base);
 
 	/* configure the final value for read data capture delay register */
-	cadence_qspi_apb_readdata_capture(base, true,
+	cadence_qspi_apb_readdata_capture(base, true, edge,
 		(range_hi + range_lo) / 2);
 	debug("SF: Read data capture delay calibrated to %i (%i - %i)\n",
 	      (range_hi + range_lo) / 2, range_lo, range_hi);
@@ -317,6 +319,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	plat->tsd2d_ns = fdtdec_get_int(blob, subnode, "tsd2d-ns", 255);
 	plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
 	plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
+	plat->sample_edge_rising = fdtdec_get_bool(blob, subnode,
+		"sample-edge-rising");
 
 	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
 	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index 50c6e7c..897e5f0 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -26,6 +26,7 @@ struct cadence_spi_platdata {
 	u32		tchsh_ns;
 	u32		tslch_ns;
 	u32		sram_size;
+	bool		sample_edge_rising;
 };
 
 struct cadence_spi_priv {
@@ -72,6 +73,6 @@ void cadence_qspi_apb_delay(void *reg_base,
 	unsigned int tchsh_ns, unsigned int tslch_ns);
 void cadence_qspi_apb_enter_xip(void *reg_base, char xip_dummy);
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-	bool bypass, unsigned int delay);
+	bool bypass, bool edge, unsigned int delay);
 
 #endif /* __CADENCE_QSPI_H__ */
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index f2cd852..4d1ee65 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -98,6 +98,7 @@
 #define	CQSPI_REG_RD_DATA_CAPTURE_BYPASS	BIT(0)
 #define	CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB	1
 #define	CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK	0xF
+#define	CQSPI_REG_RD_DATA_CAPTURE_EDGE		BIT(5)
 
 #define	CQSPI_REG_SIZE				0x14
 #define	CQSPI_REG_SIZE_ADDRESS_LSB		0
@@ -234,7 +235,7 @@ static unsigned int cadence_qspi_wait_idle(void *reg_base)
 }
 
 void cadence_qspi_apb_readdata_capture(void *reg_base,
-	bool bypass, unsigned int delay)
+	bool bypass, bool edge, unsigned int delay)
 {
 	unsigned int reg;
 	cadence_qspi_apb_controller_disable(reg_base);
@@ -246,6 +247,11 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
 	else
 		reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
 
+	if (edge)
+		reg |= CQSPI_REG_RD_DATA_CAPTURE_EDGE;
+	else
+		reg &= ~CQSPI_REG_RD_DATA_CAPTURE_EDGE;
+
 	reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
 		<< CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
 
-- 
2.7.4

[U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Most of the code already uses #defines for the bit value, rather
> than the shift required to get the value. This changes the remaining
> code over.
>
> Whislt at it, fix the names of the "Rd Data Capture" register defs.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  v3:
>  - Remove brackets that aren't needed.
>  v2:
>  - No change.
> ---
>  drivers/spi/cadence_qspi_apb.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 0a2963d..b41f36b 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -57,9 +57,9 @@
>   * Controller's configuration and status register (offset from QSPI_BASE)
>   ****************************************************************************/
>  #define        CQSPI_REG_CONFIG                        0x00
> -#define        CQSPI_REG_CONFIG_CLK_POL_LSB            1
> -#define        CQSPI_REG_CONFIG_CLK_PHA_LSB            2
>  #define        CQSPI_REG_CONFIG_ENABLE_MASK            BIT(0)
> +#define        CQSPI_REG_CONFIG_CLK_POL                BIT(1)
> +#define        CQSPI_REG_CONFIG_CLK_PHA                BIT(2)
>  #define        CQSPI_REG_CONFIG_DIRECT_MASK            BIT(7)
>  #define        CQSPI_REG_CONFIG_DECODE_MASK            BIT(9)
>  #define        CQSPI_REG_CONFIG_XIP_IMM_MASK           BIT(18)
> @@ -94,10 +94,10 @@
>  #define        CQSPI_REG_DELAY_TSD2D_MASK              0xFF
>  #define        CQSPI_REG_DELAY_TSHSL_MASK              0xFF
>
> -#define        CQSPI_READLCAPTURE                      0x10
> -#define        CQSPI_READLCAPTURE_BYPASS_LSB           0
> -#define        CQSPI_READLCAPTURE_DELAY_LSB            1
> -#define        CQSPI_READLCAPTURE_DELAY_MASK           0xF
> +#define        CQSPI_REG_RD_DATA_CAPTURE               0x10
> +#define        CQSPI_REG_RD_DATA_CAPTURE_BYPASS        BIT(0)
> +#define        CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB     1
> +#define        CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK    0xF
>
>  #define        CQSPI_REG_SIZE                          0x14
>  #define        CQSPI_REG_SIZE_ADDRESS_LSB              0
> @@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void *reg_base,
>         unsigned int reg;
>         cadence_qspi_apb_controller_disable(reg_base);
>
> -       reg = readl(reg_base + CQSPI_READLCAPTURE);
> +       reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
>
>         if (bypass)
> -               reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> +               reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
>         else
> -               reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> +               reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
>
> -       reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
> -               << CQSPI_READLCAPTURE_DELAY_LSB);
> +       reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
> +               << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
>
> -       reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
> -               << CQSPI_READLCAPTURE_DELAY_LSB);
> +       reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
> +               << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
>
> -       writel(reg, reg_base + CQSPI_READLCAPTURE);
> +       writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
>
>         cadence_qspi_apb_controller_enable(reg_base);
>         return;
> @@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
>
>         cadence_qspi_apb_controller_disable(reg_base);
>         reg = readl(reg_base + CQSPI_REG_CONFIG);
> -       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> -       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> +       reg &= ~(CQSPI_REG_CONFIG_CLK_POL | CQSPI_REG_CONFIG_CLK_PHA);
>
> -       reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> -       reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> +       if (clk_pol)
> +               reg |= CQSPI_REG_CONFIG_CLK_POL;
> +       if (clk_pha)
> +               reg |= CQSPI_REG_CONFIG_CLK_PHA;

I've commented about this change [1]?

[1] https://www.mail-archive.com/u-boot at lists.denx.de/msg232120.html

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

[U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Phil Edworthy]

Hi Jagan,

On 30 November 2016 04:59, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > Most of the code already uses #defines for the bit value, rather
> > than the shift required to get the value. This changes the remaining
> > code over.
> >
> > Whislt at it, fix the names of the "Rd Data Capture" register defs.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > Acked-by: Marek Vasut <marek.vasut@gmail.com>
> > ---
> >  v3:
> >  - Remove brackets that aren't needed.
> >  v2:
> >  - No change.
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 37 +++++++++++++++++++------------------
> >  1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 0a2963d..b41f36b 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -57,9 +57,9 @@
> >   * Controller's configuration and status register (offset from QSPI_BASE)
> >
> **************************************************************
> **************/
> >  #define        CQSPI_REG_CONFIG                        0x00
> > -#define        CQSPI_REG_CONFIG_CLK_POL_LSB            1
> > -#define        CQSPI_REG_CONFIG_CLK_PHA_LSB            2
> >  #define        CQSPI_REG_CONFIG_ENABLE_MASK            BIT(0)
> > +#define        CQSPI_REG_CONFIG_CLK_POL                BIT(1)
> > +#define        CQSPI_REG_CONFIG_CLK_PHA                BIT(2)
> >  #define        CQSPI_REG_CONFIG_DIRECT_MASK            BIT(7)
> >  #define        CQSPI_REG_CONFIG_DECODE_MASK            BIT(9)
> >  #define        CQSPI_REG_CONFIG_XIP_IMM_MASK           BIT(18)
> > @@ -94,10 +94,10 @@
> >  #define        CQSPI_REG_DELAY_TSD2D_MASK              0xFF
> >  #define        CQSPI_REG_DELAY_TSHSL_MASK              0xFF
> >
> > -#define        CQSPI_READLCAPTURE                      0x10
> > -#define        CQSPI_READLCAPTURE_BYPASS_LSB           0
> > -#define        CQSPI_READLCAPTURE_DELAY_LSB            1
> > -#define        CQSPI_READLCAPTURE_DELAY_MASK           0xF
> > +#define        CQSPI_REG_RD_DATA_CAPTURE               0x10
> > +#define        CQSPI_REG_RD_DATA_CAPTURE_BYPASS        BIT(0)
> > +#define        CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB     1
> > +#define        CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK    0xF
> >
> >  #define        CQSPI_REG_SIZE                          0x14
> >  #define        CQSPI_REG_SIZE_ADDRESS_LSB              0
> > @@ -244,20 +244,20 @@ void cadence_qspi_apb_readdata_capture(void
> *reg_base,
> >         unsigned int reg;
> >         cadence_qspi_apb_controller_disable(reg_base);
> >
> > -       reg = readl(reg_base + CQSPI_READLCAPTURE);
> > +       reg = readl(reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> >         if (bypass)
> > -               reg |= (1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +               reg |= CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> >         else
> > -               reg &= ~(1 << CQSPI_READLCAPTURE_BYPASS_LSB);
> > +               reg &= ~CQSPI_REG_RD_DATA_CAPTURE_BYPASS;
> >
> > -       reg &= ~(CQSPI_READLCAPTURE_DELAY_MASK
> > -               << CQSPI_READLCAPTURE_DELAY_LSB);
> > +       reg &= ~(CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK
> > +               << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB);
> >
> > -       reg |= ((delay & CQSPI_READLCAPTURE_DELAY_MASK)
> > -               << CQSPI_READLCAPTURE_DELAY_LSB);
> > +       reg |= (delay & CQSPI_REG_RD_DATA_CAPTURE_DELAY_MASK)
> > +               << CQSPI_REG_RD_DATA_CAPTURE_DELAY_LSB;
> >
> > -       writel(reg, reg_base + CQSPI_READLCAPTURE);
> > +       writel(reg, reg_base + CQSPI_REG_RD_DATA_CAPTURE);
> >
> >         cadence_qspi_apb_controller_enable(reg_base);
> >         return;
> > @@ -301,11 +301,12 @@ void cadence_qspi_apb_set_clk_mode(void
> *reg_base,
> >
> >         cadence_qspi_apb_controller_disable(reg_base);
> >         reg = readl(reg_base + CQSPI_REG_CONFIG);
> > -       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -       reg &= ~(1 << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +       reg &= ~(CQSPI_REG_CONFIG_CLK_POL |
> CQSPI_REG_CONFIG_CLK_PHA);
> >
> > -       reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > -       reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
> > +       if (clk_pol)
> > +               reg |= CQSPI_REG_CONFIG_CLK_POL;
> > +       if (clk_pha)
> > +               reg |= CQSPI_REG_CONFIG_CLK_PHA;
> 
> I've commented about this change [1]?
> 
> [1] https://www.mail-archive.com/u-boot at lists.denx.de/msg232120.html
Yes, I fixed that in a separate patch, see
https://www.mail-archive.com/u-boot at lists.denx.de/msg232489.html

I did it in a separate patch because this patch is purely about replacing the
definitions of bit shifts with BIT(x).

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

Thanks
Phil

[U-Boot] [PATCH v3 01/11] spi: cadence_qspi: Fix clearing of pol/pha bits

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Or'ing together bit positions is clearly wrong.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>


Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 02/11] spi: cadence_qspi: Fix baud rate calculation

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> With the existing code, when the requested SPI clock rate is near
> to the lowest that can be achieved by the hardware (max divider
> of the ref clock is 32), the generated clock rate is wrong.
> For example, with a 50MHz ref clock, when asked for anything less
> than a 1.5MHz SPI clock, the code sets up the divider to generate
> 25MHz.
>
> This change fixes the calculation.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Perhaps you missed, Marek Acked-by tag on previous version, don't
worry will add while applying if any.

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 03/11] spi: cadence_qspi: Better debug information on the SPI clock rate

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Show what the output clock rate actually is.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 04/11] spi: cadence_qspi: Use #define for bits instead of bit shifts

[Jagan Teki]

On Wed, Nov 30, 2016 at 1:02 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Hi Jagan,
>
> On 30 November 2016 04:59, Jagan Teki wrote:
>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > Most of the code already uses #defines for the bit value, rather
>> > than the shift required to get the value. This changes the remaining
>> > code over.
>> >
>> > Whislt at it, fix the names of the "Rd Data Capture" register defs.
>> >
>> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>> > Acked-by: Marek Vasut <marek.vasut@gmail.com>

<snip>

>>
>> I've commented about this change [1]?
>>
>> [1] https://www.mail-archive.com/u-boot at lists.denx.de/msg232120.html
> Yes, I fixed that in a separate patch, see
> https://www.mail-archive.com/u-boot at lists.denx.de/msg232489.html
>
> I did it in a separate patch because this patch is purely about replacing the
> definitions of bit shifts with BIT(x).

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 05/11] spi: cadence_qspi: Clean up the #define names

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> A lot of the #defines are for single bits in a register, where the
> name has _MASK on the end. Since this can be used for both a mask
> and the value, remove _MASK from them.
>
> Whilst doing so, also remove the unnecessary brackets around the
> constants.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 06/11] spi: cadence_qspi: Use spi mode at the point it is needed

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Instead of extracting mode settings and passing them as separate
> args to another function, just pass the SPI mode as an arg.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 07/11] spi: cadence_qspi: Remove returns from end of void functions

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 09/11] spi: cadence_qspi: Move DT prop code to match layout

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Move the code to read the "sram-size" property into the other code
> that reads properties from the node, rather than the SF subnode.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Reviewed-by: Jagan Teki <jagan@openedev.com>

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

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> This is in preparation for adding another arg.

?? proper reason for changing arg to bool.

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

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Jagan Teki]

On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Introduce a new DT property to specify whether the QSPI Controller
> samples the data on a rising edge. The default is falling edge.
> Some versions of the QSPI Controller do not implement this bit, in
> which case the property should be omitted.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
>  v3:
>   - Patch split so this one only has code related to the subject.
>   - Commit message updated.
>  v2:
>   - Change name of DT prop and provide details of what it does.
>     Whilst at it, move the code to read the "sram-size" property
>     into the other code that reads properties from the node, rather
>     than the SF subnode.
>
>     Also change the code to use a bool for the bypass arg.
> ---
>  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
>  drivers/spi/cadence_qspi.c                   | 10 +++++++---
>  drivers/spi/cadence_qspi.h                   |  3 ++-
>  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
>  4 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-bindings/spi/spi-cadence.txt
> index c1e2233..94c800b 100644
> --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> @@ -26,3 +26,5 @@ connected flash properties
>                           select (n_ss_out).
>  - tslch-ns             : Delay in master reference clocks between setting
>                           n_ss_out low and first bit transfer
> +- sample-edge-rising   : Data outputs from flash memory will be sampled on the
> +                         rising edge. Default is falling edge.

Code look reasonable, but how Linux handling this with the same dt
property, any idea? I couldn't find it either.

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

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

Hi Jagan,

On 02 December 2016 14:20, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > This is in preparation for adding another arg.
> 
> ?? proper reason for changing arg to bool.
Purely because the patch 11 adds another arg that is a bool (which is the natural
type as it is read from a dtb). Then having this bypass arg as something other
than a bool look a bit odd.

Thanks
Phil

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 02 December 2016 14:23, Jagan Teki wrote:
> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > Introduce a new DT property to specify whether the QSPI Controller
> > samples the data on a rising edge. The default is falling edge.
> > Some versions of the QSPI Controller do not implement this bit, in
> > which case the property should be omitted.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> >  v3:
> >   - Patch split so this one only has code related to the subject.
> >   - Commit message updated.
> >  v2:
> >   - Change name of DT prop and provide details of what it does.
> >     Whilst at it, move the code to read the "sram-size" property
> >     into the other code that reads properties from the node, rather
> >     than the SF subnode.
> >
> >     Also change the code to use a bool for the bypass arg.
> > ---
> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >  drivers/spi/cadence_qspi.c                   | 10 +++++++---
> >  drivers/spi/cadence_qspi.h                   |  3 ++-
> >  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
> bindings/spi/spi-cadence.txt
> > index c1e2233..94c800b 100644
> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> > @@ -26,3 +26,5 @@ connected flash properties
> >                           select (n_ss_out).
> >  - tslch-ns             : Delay in master reference clocks between setting
> >                           n_ss_out low and first bit transfer
> > +- sample-edge-rising   : Data outputs from flash memory will be sampled on
> the
> > +                         rising edge. Default is falling edge.
> 
> Code look reasonable, but how Linux handling this with the same dt
> property, any idea? I couldn't find it either.
The Linux driver does not yet have this property. Is there a policy to add new
props to Linux first?

Thanks
Phil

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Jagan Teki]

On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Hi Jagan,
>
> On 02 December 2016 14:23, Jagan Teki wrote:
>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > Introduce a new DT property to specify whether the QSPI Controller
>> > samples the data on a rising edge. The default is falling edge.
>> > Some versions of the QSPI Controller do not implement this bit, in
>> > which case the property should be omitted.
>> >
>> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>> > ---
>> >  v3:
>> >   - Patch split so this one only has code related to the subject.
>> >   - Commit message updated.
>> >  v2:
>> >   - Change name of DT prop and provide details of what it does.
>> >     Whilst at it, move the code to read the "sram-size" property
>> >     into the other code that reads properties from the node, rather
>> >     than the SF subnode.
>> >
>> >     Also change the code to use a bool for the bypass arg.
>> > ---
>> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
>> >  drivers/spi/cadence_qspi.c                   | 10 +++++++---
>> >  drivers/spi/cadence_qspi.h                   |  3 ++-
>> >  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
>> >  4 files changed, 18 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
>> bindings/spi/spi-cadence.txt
>> > index c1e2233..94c800b 100644
>> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
>> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
>> > @@ -26,3 +26,5 @@ connected flash properties
>> >                           select (n_ss_out).
>> >  - tslch-ns             : Delay in master reference clocks between setting
>> >                           n_ss_out low and first bit transfer
>> > +- sample-edge-rising   : Data outputs from flash memory will be sampled on
>> the
>> > +                         rising edge. Default is falling edge.
>>
>> Code look reasonable, but how Linux handling this with the same dt
>> property, any idea? I couldn't find it either.
> The Linux driver does not yet have this property. Is there a policy to add new
> props to Linux first?

If the same/equal code used in Linux better to have the same property
instead of another name used in U-boot?

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

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Jagan Teki]

On Mon, Dec 5, 2016 at 11:07 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Hi Jagan,
>
> On 02 December 2016 14:20, Jagan Teki wrote:
>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > This is in preparation for adding another arg.
>>
>> ?? proper reason for changing arg to bool.
> Purely because the patch 11 adds another arg that is a bool (which is the natural
> type as it is read from a dtb). Then having this bypass arg as something other
> than a bool look a bit odd.

Can't we make this as 11 and saying the reason for bool which is
used/compatible with previous dt patch (I mean 11th patch in the
current case)?

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

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

HI Jagan,

On 05 December 2016 10:26, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > Hi Jagan,
> >
> > On 02 December 2016 14:23, Jagan Teki wrote:
> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> <phil.edworthy@renesas.com> wrote:
> >> > Introduce a new DT property to specify whether the QSPI Controller
> >> > samples the data on a rising edge. The default is falling edge.
> >> > Some versions of the QSPI Controller do not implement this bit, in
> >> > which case the property should be omitted.
> >> >
> >> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >> > ---
> >> >  v3:
> >> >   - Patch split so this one only has code related to the subject.
> >> >   - Commit message updated.
> >> >  v2:
> >> >   - Change name of DT prop and provide details of what it does.
> >> >     Whilst at it, move the code to read the "sram-size" property
> >> >     into the other code that reads properties from the node, rather
> >> >     than the SF subnode.
> >> >
> >> >     Also change the code to use a bool for the bypass arg.
> >> > ---
> >> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >> >  drivers/spi/cadence_qspi.c                   | 10 +++++++---
> >> >  drivers/spi/cadence_qspi.h                   |  3 ++-
> >> >  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
> >> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
> >> bindings/spi/spi-cadence.txt
> >> > index c1e2233..94c800b 100644
> >> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >> > @@ -26,3 +26,5 @@ connected flash properties
> >> >                           select (n_ss_out).
> >> >  - tslch-ns             : Delay in master reference clocks between setting
> >> >                           n_ss_out low and first bit transfer
> >> > +- sample-edge-rising   : Data outputs from flash memory will be sampled on
> >> the
> >> > +                         rising edge. Default is falling edge.
> >>
> >> Code look reasonable, but how Linux handling this with the same dt
> >> property, any idea? I couldn't find it either.
> > The Linux driver does not yet have this property. Is there a policy to add new
> > props to Linux first?
> 
> If the same/equal code used in Linux better to have the same property
> instead of another name used in U-boot?
Of course, but I cannot see this in Linux:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt

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

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Phil Edworthy]

Hi Jagan,

On 05 December 2016 10:29, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:07 AM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > Hi Jagan,
> >
> > On 02 December 2016 14:20, Jagan Teki wrote:
> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> <phil.edworthy@renesas.com> wrote:
> >> > This is in preparation for adding another arg.
> >>
> >> ?? proper reason for changing arg to bool.
> > Purely because the patch 11 adds another arg that is a bool (which is the natural
> > type as it is read from a dtb). Then having this bypass arg as something other
> > than a bool look a bit odd.
> 
> Can't we make this as 11 and saying the reason for bool which is
> used/compatible with previous dt patch (I mean 11th patch in the
> current case)?
Do you mean swap patches 10 and 11? Then this commit msg is basically
to say it is changed to bool to match the other arg?
If so, then sure, no problem.

Thanks
Phil

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

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Jagan Teki]

On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> HI Jagan,
>
> On 05 December 2016 10:26, Jagan Teki wrote:
>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>> > Hi Jagan,
>> >
>> > On 02 December 2016 14:23, Jagan Teki wrote:
>> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>> >> <phil.edworthy@renesas.com> wrote:
>> >> > Introduce a new DT property to specify whether the QSPI Controller
>> >> > samples the data on a rising edge. The default is falling edge.
>> >> > Some versions of the QSPI Controller do not implement this bit, in
>> >> > which case the property should be omitted.
>> >> >
>> >> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>> >> > ---
>> >> >  v3:
>> >> >   - Patch split so this one only has code related to the subject.
>> >> >   - Commit message updated.
>> >> >  v2:
>> >> >   - Change name of DT prop and provide details of what it does.
>> >> >     Whilst at it, move the code to read the "sram-size" property
>> >> >     into the other code that reads properties from the node, rather
>> >> >     than the SF subnode.
>> >> >
>> >> >     Also change the code to use a bool for the bypass arg.
>> >> > ---
>> >> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
>> >> >  drivers/spi/cadence_qspi.c                   | 10 +++++++---
>> >> >  drivers/spi/cadence_qspi.h                   |  3 ++-
>> >> >  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
>> >> >  4 files changed, 18 insertions(+), 5 deletions(-)
>> >> >
>> >> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-tree-
>> >> bindings/spi/spi-cadence.txt
>> >> > index c1e2233..94c800b 100644
>> >> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
>> >> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
>> >> > @@ -26,3 +26,5 @@ connected flash properties
>> >> >                           select (n_ss_out).
>> >> >  - tslch-ns             : Delay in master reference clocks between setting
>> >> >                           n_ss_out low and first bit transfer
>> >> > +- sample-edge-rising   : Data outputs from flash memory will be sampled on
>> >> the
>> >> > +                         rising edge. Default is falling edge.
>> >>
>> >> Code look reasonable, but how Linux handling this with the same dt
>> >> property, any idea? I couldn't find it either.
>> > The Linux driver does not yet have this property. Is there a policy to add new
>> > props to Linux first?
>>
>> If the same/equal code used in Linux better to have the same property
>> instead of another name used in U-boot?
> Of course, but I cannot see this in Linux:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt

Yeah, I saw this. Do you have any idea how Linux handling this sample edge?

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

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 05 December 2016 10:42, Jagan Teki wrote:
> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> <phil.edworthy@renesas.com> wrote:
> > HI Jagan,
> >
> > On 05 December 2016 10:26, Jagan Teki wrote:
> >> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >> <phil.edworthy@renesas.com> wrote:
> >> > Hi Jagan,
> >> >
> >> > On 02 December 2016 14:23, Jagan Teki wrote:
> >> >> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> >> <phil.edworthy@renesas.com> wrote:
> >> >> > Introduce a new DT property to specify whether the QSPI Controller
> >> >> > samples the data on a rising edge. The default is falling edge.
> >> >> > Some versions of the QSPI Controller do not implement this bit, in
> >> >> > which case the property should be omitted.
> >> >> >
> >> >> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >> >> > ---
> >> >> >  v3:
> >> >> >   - Patch split so this one only has code related to the subject.
> >> >> >   - Commit message updated.
> >> >> >  v2:
> >> >> >   - Change name of DT prop and provide details of what it does.
> >> >> >     Whilst at it, move the code to read the "sram-size" property
> >> >> >     into the other code that reads properties from the node, rather
> >> >> >     than the SF subnode.
> >> >> >
> >> >> >     Also change the code to use a bool for the bypass arg.
> >> >> > ---
> >> >> >  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >> >> >  drivers/spi/cadence_qspi.c                   | 10 +++++++---
> >> >> >  drivers/spi/cadence_qspi.h                   |  3 ++-
> >> >> >  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
> >> >> >  4 files changed, 18 insertions(+), 5 deletions(-)
> >> >> >
> >> >> > diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-
> tree-
> >> >> bindings/spi/spi-cadence.txt
> >> >> > index c1e2233..94c800b 100644
> >> >> > --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >> >> > +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >> >> > @@ -26,3 +26,5 @@ connected flash properties
> >> >> >                           select (n_ss_out).
> >> >> >  - tslch-ns             : Delay in master reference clocks between setting
> >> >> >                           n_ss_out low and first bit transfer
> >> >> > +- sample-edge-rising   : Data outputs from flash memory will be sampled
> on
> >> >> the
> >> >> > +                         rising edge. Default is falling edge.
> >> >>
> >> >> Code look reasonable, but how Linux handling this with the same dt
> >> >> property, any idea? I couldn't find it either.
> >> > The Linux driver does not yet have this property. Is there a policy to add new
> >> > props to Linux first?
> >>
> >> If the same/equal code used in Linux better to have the same property
> >> instead of another name used in U-boot?
> > Of course, but I cannot see this in Linux:
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> 
> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
The same way U-Boot currently handles it, i.e. it does nothing with this. Intel/Altera
(Chin Liang) said that they do not have this bit in their version of the Cadence QSPI
Controller.

We are using a later version that has had this bit added.

BR
Phil

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

[U-Boot] [PATCH v3 10/11] spi: cadence_qspi: Change readdata_capture() arg to bool

[Marek Vasut]

On 12/05/2016 11:33 AM, Phil Edworthy wrote:
> Hi Jagan,
> 
> On 05 December 2016 10:29, Jagan Teki wrote:
>> On Mon, Dec 5, 2016 at 11:07 AM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>>> Hi Jagan,
>>>
>>> On 02 December 2016 14:20, Jagan Teki wrote:
>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>>>> <phil.edworthy@renesas.com> wrote:
>>>>> This is in preparation for adding another arg.
>>>>
>>>> ?? proper reason for changing arg to bool.
>>> Purely because the patch 11 adds another arg that is a bool (which is the natural
>>> type as it is read from a dtb). Then having this bypass arg as something other
>>> than a bool look a bit odd.
>>
>> Can't we make this as 11 and saying the reason for bool which is
>> used/compatible with previous dt patch (I mean 11th patch in the
>> current case)?
> Do you mean swap patches 10 and 11? Then this commit msg is basically
> to say it is changed to bool to match the other arg?
> If so, then sure, no problem.

I don't think it makes any sense to swap patches 10 and 11, they seem
orthogonal to me.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Marek Vasut]

On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> Hi Jagan,
> 
> On 05 December 2016 10:42, Jagan Teki wrote:
>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
>> <phil.edworthy@renesas.com> wrote:
>>> HI Jagan,
>>>
>>> On 05 December 2016 10:26, Jagan Teki wrote:
>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
>>>> <phil.edworthy@renesas.com> wrote:
>>>>> Hi Jagan,
>>>>>
>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>>>>>> <phil.edworthy@renesas.com> wrote:
>>>>>>> Introduce a new DT property to specify whether the QSPI Controller
>>>>>>> samples the data on a rising edge. The default is falling edge.
>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
>>>>>>> which case the property should be omitted.
>>>>>>>
>>>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>>>>>>> ---
>>>>>>>  v3:
>>>>>>>   - Patch split so this one only has code related to the subject.
>>>>>>>   - Commit message updated.
>>>>>>>  v2:
>>>>>>>   - Change name of DT prop and provide details of what it does.
>>>>>>>     Whilst at it, move the code to read the "sram-size" property
>>>>>>>     into the other code that reads properties from the node, rather
>>>>>>>     than the SF subnode.
>>>>>>>
>>>>>>>     Also change the code to use a bool for the bypass arg.
>>>>>>> ---
>>>>>>>  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
>>>>>>>  drivers/spi/cadence_qspi.c                   | 10 +++++++---
>>>>>>>  drivers/spi/cadence_qspi.h                   |  3 ++-
>>>>>>>  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
>>>>>>>  4 files changed, 18 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-
>> tree-
>>>>>> bindings/spi/spi-cadence.txt
>>>>>>> index c1e2233..94c800b 100644
>>>>>>> --- a/doc/device-tree-bindings/spi/spi-cadence.txt
>>>>>>> +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
>>>>>>> @@ -26,3 +26,5 @@ connected flash properties
>>>>>>>                           select (n_ss_out).
>>>>>>>  - tslch-ns             : Delay in master reference clocks between setting
>>>>>>>                           n_ss_out low and first bit transfer
>>>>>>> +- sample-edge-rising   : Data outputs from flash memory will be sampled
>> on
>>>>>> the
>>>>>>> +                         rising edge. Default is falling edge.
>>>>>>
>>>>>> Code look reasonable, but how Linux handling this with the same dt
>>>>>> property, any idea? I couldn't find it either.
>>>>> The Linux driver does not yet have this property. Is there a policy to add new
>>>>> props to Linux first?
>>>>
>>>> If the same/equal code used in Linux better to have the same property
>>>> instead of another name used in U-boot?
>>> Of course, but I cannot see this in Linux:
>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
>>
>> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
> The same way U-Boot currently handles it, i.e. it does nothing with this. Intel/Altera
> (Chin Liang) said that they do not have this bit in their version of the Cadence QSPI
> Controller.
> 
> We are using a later version that has had this bit added.

You were looking at the wrong bindings:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt

but yes, Linux does not do support the data edge toggling. I think there
was another QSPI patch in Linux which tried adding such property, so
check linux-mtd for it. Generic one would be great.

And no, there is no policy for pushing new props to linux first. New DT
props should ideally get approved via devicetree at vger though, but that's
about it. Also, while I tried backporting the Linux CQSPI driver to
U-Boot, but unfortunately, it turned out to be extremely difficult due
significant differences between the Linux and U-Boot SPI NOR  framework.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Marek,

On 05 December 2016 13:31, Marek Vasut wrote:
> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> > On 05 December 2016 10:42, Jagan Teki wrote:
> >> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >> <phil.edworthy@renesas.com> wrote:
> >>> On 05 December 2016 10:26, Jagan Teki wrote:
> >>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >>>> <phil.edworthy@renesas.com> wrote:
> >>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >>>>>> <phil.edworthy@renesas.com> wrote:
> >>>>>>> Introduce a new DT property to specify whether the QSPI Controller
> >>>>>>> samples the data on a rising edge. The default is falling edge.
> >>>>>>> Some versions of the QSPI Controller do not implement this bit, in
> >>>>>>> which case the property should be omitted.
> >>>>>>>
> >>>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >>>>>>> ---
> >>>>>>>  v3:
> >>>>>>>   - Patch split so this one only has code related to the subject.
> >>>>>>>   - Commit message updated.
> >>>>>>>  v2:
> >>>>>>>   - Change name of DT prop and provide details of what it does.
> >>>>>>>     Whilst at it, move the code to read the "sram-size" property
> >>>>>>>     into the other code that reads properties from the node, rather
> >>>>>>>     than the SF subnode.
> >>>>>>>
> >>>>>>>     Also change the code to use a bool for the bypass arg.
> >>>>>>> ---
> >>>>>>>  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
> >>>>>>>  drivers/spi/cadence_qspi.c                   | 10 +++++++---
> >>>>>>>  drivers/spi/cadence_qspi.h                   |  3 ++-
> >>>>>>>  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
> >>>>>>>  4 files changed, 18 insertions(+), 5 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-
> >> tree-
> >>>>>> bindings/spi/spi-cadence.txt
> >>>>>>> index c1e2233..94c800b 100644
> >>>>>>> --- a/doc/device-tree-bindings/spi/spi-cadence.txt
> >>>>>>> +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
> >>>>>>> @@ -26,3 +26,5 @@ connected flash properties
> >>>>>>>                           select (n_ss_out).
> >>>>>>>  - tslch-ns             : Delay in master reference clocks between setting
> >>>>>>>                           n_ss_out low and first bit transfer
> >>>>>>> +- sample-edge-rising   : Data outputs from flash memory will be
> sampled
> >> on
> >>>>>> the
> >>>>>>> +                         rising edge. Default is falling edge.
> >>>>>>
> >>>>>> Code look reasonable, but how Linux handling this with the same dt
> >>>>>> property, any idea? I couldn't find it either.
> >>>>> The Linux driver does not yet have this property. Is there a policy to add
> new
> >>>>> props to Linux first?
> >>>>
> >>>> If the same/equal code used in Linux better to have the same property
> >>>> instead of another name used in U-boot?
> >>> Of course, but I cannot see this in Linux:
> >>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >>
> >> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
> > The same way U-Boot currently handles it, i.e. it does nothing with this.
> Intel/Altera
> > (Chin Liang) said that they do not have this bit in their version of the Cadence
> QSPI
> > Controller.
> >
> > We are using a later version that has had this bit added.
> 
> You were looking at the wrong bindings:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
Thanks for pointing that out!

> but yes, Linux does not do support the data edge toggling. I think there
> was another QSPI patch in Linux which tried adding such property, so
> check linux-mtd for it. Generic one would be great.
I had a search around, but couldn't find anything.

> And no, there is no policy for pushing new props to linux first. New DT
> props should ideally get approved via devicetree at vger though, but that's
> about it. Also, while I tried backporting the Linux CQSPI driver to
> U-Boot, but unfortunately, it turned out to be extremely difficult due
> significant differences between the Linux and U-Boot SPI NOR  framework.
OK, thanks for the information.

Thanks
Phil

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Marek Vasut]

On 12/06/2016 11:25 AM, Phil Edworthy wrote:
> Hi Marek,
> 
> On 05 December 2016 13:31, Marek Vasut wrote:
>> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
>>> On 05 December 2016 10:42, Jagan Teki wrote:
>>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
>>>> <phil.edworthy@renesas.com> wrote:
>>>>> On 05 December 2016 10:26, Jagan Teki wrote:
>>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
>>>>>> <phil.edworthy@renesas.com> wrote:
>>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
>>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>>>>>>>> <phil.edworthy@renesas.com> wrote:
>>>>>>>>> Introduce a new DT property to specify whether the QSPI Controller
>>>>>>>>> samples the data on a rising edge. The default is falling edge.
>>>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
>>>>>>>>> which case the property should be omitted.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>>>>>>>>> ---
>>>>>>>>>  v3:
>>>>>>>>>   - Patch split so this one only has code related to the subject.
>>>>>>>>>   - Commit message updated.
>>>>>>>>>  v2:
>>>>>>>>>   - Change name of DT prop and provide details of what it does.
>>>>>>>>>     Whilst at it, move the code to read the "sram-size" property
>>>>>>>>>     into the other code that reads properties from the node, rather
>>>>>>>>>     than the SF subnode.
>>>>>>>>>
>>>>>>>>>     Also change the code to use a bool for the bypass arg.
>>>>>>>>> ---
>>>>>>>>>  doc/device-tree-bindings/spi/spi-cadence.txt |  2 ++
>>>>>>>>>  drivers/spi/cadence_qspi.c                   | 10 +++++++---
>>>>>>>>>  drivers/spi/cadence_qspi.h                   |  3 ++-
>>>>>>>>>  drivers/spi/cadence_qspi_apb.c               |  8 +++++++-
>>>>>>>>>  4 files changed, 18 insertions(+), 5 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/doc/device-tree-bindings/spi/spi-cadence.txt b/doc/device-
>>>> tree-
>>>>>>>> bindings/spi/spi-cadence.txt
>>>>>>>>> index c1e2233..94c800b 100644
>>>>>>>>> --- a/doc/device-tree-bindings/spi/spi-cadence.txt
>>>>>>>>> +++ b/doc/device-tree-bindings/spi/spi-cadence.txt
>>>>>>>>> @@ -26,3 +26,5 @@ connected flash properties
>>>>>>>>>                           select (n_ss_out).
>>>>>>>>>  - tslch-ns             : Delay in master reference clocks between setting
>>>>>>>>>                           n_ss_out low and first bit transfer
>>>>>>>>> +- sample-edge-rising   : Data outputs from flash memory will be
>> sampled
>>>> on
>>>>>>>> the
>>>>>>>>> +                         rising edge. Default is falling edge.
>>>>>>>>
>>>>>>>> Code look reasonable, but how Linux handling this with the same dt
>>>>>>>> property, any idea? I couldn't find it either.
>>>>>>> The Linux driver does not yet have this property. Is there a policy to add
>> new
>>>>>>> props to Linux first?
>>>>>>
>>>>>> If the same/equal code used in Linux better to have the same property
>>>>>> instead of another name used in U-boot?
>>>>> Of course, but I cannot see this in Linux:
>>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
>>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
>>>>
>>>> Yeah, I saw this. Do you have any idea how Linux handling this sample edge?
>>> The same way U-Boot currently handles it, i.e. it does nothing with this.
>> Intel/Altera
>>> (Chin Liang) said that they do not have this bit in their version of the Cadence
>> QSPI
>>> Controller.
>>>
>>> We are using a later version that has had this bit added.
>>
>> You were looking at the wrong bindings:
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
>> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> Thanks for pointing that out!
> 
>> but yes, Linux does not do support the data edge toggling. I think there
>> was another QSPI patch in Linux which tried adding such property, so
>> check linux-mtd for it. Generic one would be great.
> I had a search around, but couldn't find anything.

Look for negative_edge here:
http://www.spinics.net/lists/devicetree/msg153582.html

>> And no, there is no policy for pushing new props to linux first. New DT
>> props should ideally get approved via devicetree at vger though, but that's
>> about it. Also, while I tried backporting the Linux CQSPI driver to
>> U-Boot, but unfortunately, it turned out to be extremely difficult due
>> significant differences between the Linux and U-Boot SPI NOR  framework.
> OK, thanks for the information.
> 
> Thanks
> Phil
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan, Marek,

On 06 December 2016 12:39 Marek Vasut wrote:
> On 12/06/2016 11:25 AM, Phil Edworthy wrote:
> > On 05 December 2016 13:31, Marek Vasut wrote:
> >> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> >>> On 05 December 2016 10:42, Jagan Teki wrote:
> >>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >>>> <phil.edworthy@renesas.com> wrote:
> >>>>> On 05 December 2016 10:26, Jagan Teki wrote:
> >>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >>>>>> <phil.edworthy@renesas.com> wrote:
> >>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >>>>>>>> <phil.edworthy@renesas.com> wrote:
> >>>>>>>>> Introduce a new DT property to specify whether the QSPI Controller
> >>>>>>>>> samples the data on a rising edge. The default is falling edge.
> >>>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
> >>>>>>>>> which case the property should be omitted.
<snip>

> >>>>>>>> Code look reasonable, but how Linux handling this with the same dt
> >>>>>>>> property, any idea? I couldn't find it either.
> >>>>>>> The Linux driver does not yet have this property. Is there a policy to add
> >> new
> >>>>>>> props to Linux first?
> >>>>>>
> >>>>>> If the same/equal code used in Linux better to have the same property
> >>>>>> instead of another name used in U-boot?
> >>>>> Of course, but I cannot see this in Linux:
> >>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >>>>
> >>>> Yeah, I saw this. Do you have any idea how Linux handling this sample
> edge?
> >>> The same way U-Boot currently handles it, i.e. it does nothing with this.
> >> Intel/Altera
> >>> (Chin Liang) said that they do not have this bit in their version of the Cadence
> >> QSPI
> >>> Controller.
> >>>
> >>> We are using a later version that has had this bit added.
> >>
> >> You were looking at the wrong bindings:
> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > Thanks for pointing that out!
> >
> >> but yes, Linux does not do support the data edge toggling. I think there
> >> was another QSPI patch in Linux which tried adding such property, so
> >> check linux-mtd for it. Generic one would be great.
> > I had a search around, but couldn't find anything.
> 
> Look for negative_edge here:
> http://www.spinics.net/lists/devicetree/msg153582.html
> 
> >> And no, there is no policy for pushing new props to linux first. New DT
> >> props should ideally get approved via devicetree at vger though, but that's
> >> about it. Also, while I tried backporting the Linux CQSPI driver to
> >> U-Boot, but unfortunately, it turned out to be extremely difficult due
> >> significant differences between the Linux and U-Boot SPI NOR  framework.
> > OK, thanks for the information.

Since it will take a bit more time to get a generic prop for the sample edge to
be ack'd by devicetree at vger, would it make sense to drop it from this series,
so we can get the rest in?

Thanks
Phil

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Jagan Teki]

On Tue, Dec 6, 2016 at 6:00 PM, Phil Edworthy <phil.edworthy@renesas.com> wrote:
> Hi Jagan, Marek,
>
> On 06 December 2016 12:39 Marek Vasut wrote:
>> On 12/06/2016 11:25 AM, Phil Edworthy wrote:
>> > On 05 December 2016 13:31, Marek Vasut wrote:
>> >> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
>> >>> On 05 December 2016 10:42, Jagan Teki wrote:
>> >>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
>> >>>> <phil.edworthy@renesas.com> wrote:
>> >>>>> On 05 December 2016 10:26, Jagan Teki wrote:
>> >>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
>> >>>>>> <phil.edworthy@renesas.com> wrote:
>> >>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
>> >>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
>> >>>>>>>> <phil.edworthy@renesas.com> wrote:
>> >>>>>>>>> Introduce a new DT property to specify whether the QSPI Controller
>> >>>>>>>>> samples the data on a rising edge. The default is falling edge.
>> >>>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
>> >>>>>>>>> which case the property should be omitted.
> <snip>
>
>> >>>>>>>> Code look reasonable, but how Linux handling this with the same dt
>> >>>>>>>> property, any idea? I couldn't find it either.
>> >>>>>>> The Linux driver does not yet have this property. Is there a policy to add
>> >> new
>> >>>>>>> props to Linux first?
>> >>>>>>
>> >>>>>> If the same/equal code used in Linux better to have the same property
>> >>>>>> instead of another name used in U-boot?
>> >>>>> Of course, but I cannot see this in Linux:
>> >>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
>> >>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
>> >>>>
>> >>>> Yeah, I saw this. Do you have any idea how Linux handling this sample
>> edge?
>> >>> The same way U-Boot currently handles it, i.e. it does nothing with this.
>> >> Intel/Altera
>> >>> (Chin Liang) said that they do not have this bit in their version of the Cadence
>> >> QSPI
>> >>> Controller.
>> >>>
>> >>> We are using a later version that has had this bit added.
>> >>
>> >> You were looking at the wrong bindings:
>> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
>> >> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>> > Thanks for pointing that out!
>> >
>> >> but yes, Linux does not do support the data edge toggling. I think there
>> >> was another QSPI patch in Linux which tried adding such property, so
>> >> check linux-mtd for it. Generic one would be great.
>> > I had a search around, but couldn't find anything.
>>
>> Look for negative_edge here:
>> http://www.spinics.net/lists/devicetree/msg153582.html
>>
>> >> And no, there is no policy for pushing new props to linux first. New DT
>> >> props should ideally get approved via devicetree at vger though, but that's
>> >> about it. Also, while I tried backporting the Linux CQSPI driver to
>> >> U-Boot, but unfortunately, it turned out to be extremely difficult due
>> >> significant differences between the Linux and U-Boot SPI NOR  framework.
>> > OK, thanks for the information.
>
> Since it will take a bit more time to get a generic prop for the sample edge to
> be ack'd by devicetree at vger, would it make sense to drop it from this series,
> so we can get the rest in?

I can drop 10 and 11 from the series, is that OK?

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

[U-Boot] [PATCH v3 11/11] spi: cadence_qspi: Support specifying the sample edge used

[Phil Edworthy]

Hi Jagan,

On 06 December 2016 17:24 Jagan Teki wrote:
> On Tue, Dec 6, 2016 at 6:00 PM, Phil Edworthy <phil.edworthy@renesas.com>
> wrote:
> > Hi Jagan, Marek,
> >
> > On 06 December 2016 12:39 Marek Vasut wrote:
> >> On 12/06/2016 11:25 AM, Phil Edworthy wrote:
> >> > On 05 December 2016 13:31, Marek Vasut wrote:
> >> >> On 12/05/2016 11:46 AM, Phil Edworthy wrote:
> >> >>> On 05 December 2016 10:42, Jagan Teki wrote:
> >> >>>> On Mon, Dec 5, 2016 at 11:31 AM, Phil Edworthy
> >> >>>> <phil.edworthy@renesas.com> wrote:
> >> >>>>> On 05 December 2016 10:26, Jagan Teki wrote:
> >> >>>>>> On Mon, Dec 5, 2016 at 11:09 AM, Phil Edworthy
> >> >>>>>> <phil.edworthy@renesas.com> wrote:
> >> >>>>>>> On 02 December 2016 14:23, Jagan Teki wrote:
> >> >>>>>>>> On Tue, Nov 29, 2016 at 6:28 PM, Phil Edworthy
> >> >>>>>>>> <phil.edworthy@renesas.com> wrote:
> >> >>>>>>>>> Introduce a new DT property to specify whether the QSPI
> Controller
> >> >>>>>>>>> samples the data on a rising edge. The default is falling edge.
> >> >>>>>>>>> Some versions of the QSPI Controller do not implement this bit, in
> >> >>>>>>>>> which case the property should be omitted.
> > <snip>
> >
> >> >>>>>>>> Code look reasonable, but how Linux handling this with the same
> dt
> >> >>>>>>>> property, any idea? I couldn't find it either.
> >> >>>>>>> The Linux driver does not yet have this property. Is there a policy to
> add
> >> >> new
> >> >>>>>>> props to Linux first?
> >> >>>>>>
> >> >>>>>> If the same/equal code used in Linux better to have the same
> property
> >> >>>>>> instead of another name used in U-boot?
> >> >>>>> Of course, but I cannot see this in Linux:
> >> >>>>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> >>>> next.git/tree/Documentation/devicetree/bindings/spi/spi-cadence.txt
> >> >>>>
> >> >>>> Yeah, I saw this. Do you have any idea how Linux handling this sample
> >> edge?
> >> >>> The same way U-Boot currently handles it, i.e. it does nothing with this.
> >> >> Intel/Altera
> >> >>> (Chin Liang) said that they do not have this bit in their version of the
> Cadence
> >> >> QSPI
> >> >>> Controller.
> >> >>>
> >> >>> We are using a later version that has had this bit added.
> >> >>
> >> >> You were looking at the wrong bindings:
> >> >> https://git.kernel.org/cgit/linux/kernel/git/next/linux-
> >> >> next.git/tree/Documentation/devicetree/bindings/mtd/cadence-
> quadspi.txt
> >> > Thanks for pointing that out!
> >> >
> >> >> but yes, Linux does not do support the data edge toggling. I think there
> >> >> was another QSPI patch in Linux which tried adding such property, so
> >> >> check linux-mtd for it. Generic one would be great.
> >> > I had a search around, but couldn't find anything.
> >>
> >> Look for negative_edge here:
> >> http://www.spinics.net/lists/devicetree/msg153582.html
> >>
> >> >> And no, there is no policy for pushing new props to linux first. New DT
> >> >> props should ideally get approved via devicetree at vger though, but that's
> >> >> about it. Also, while I tried backporting the Linux CQSPI driver to
> >> >> U-Boot, but unfortunately, it turned out to be extremely difficult due
> >> >> significant differences between the Linux and U-Boot SPI NOR  framework.
> >> > OK, thanks for the information.
> >
> > Since it will take a bit more time to get a generic prop for the sample edge to
> > be ack'd by devicetree at vger, would it make sense to drop it from this series,
> > so we can get the rest in?
> 
> I can drop 10 and 11 from the series, is that OK?
Yes please!

Thanks
Phil

[U-Boot] [PATCH v3 00/11] SF: Cadence QSPI driver fixes and clean up

[Jagan Teki]

On Tue, Nov 29, 2016 at 1:58 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> This series has fixes, patches to clean the code up, and add support for
> specifying the sampling edge.
>
> Main changes in v3:
>   1. Added "spi: cadence_qspi: Use spi mode at the point it is needed" to
>      address comments for SPI pol/phase code.
>   2. "spi: cadence_qspi: Support specifying the sample edge used" has been
>      split into 3 separate patches.
>
>
> Phil Edworthy (11):
>   spi: cadence_qspi: Fix clearing of pol/pha bits
>   spi: cadence_qspi: Fix baud rate calculation
>   spi: cadence_qspi: Better debug information on the SPI clock rate
>   spi: cadence_qspi: Use #define for bits instead of bit shifts
>   spi: cadence_qspi: Clean up the #define names
>   spi: cadence_qspi: Use spi mode at the point it is needed
>   spi: cadence_qspi: Remove returns from end of void functions
>   spi: cadence_qspi: Fix CS timings
>   spi: cadence_qspi: Move DT prop code to match layout

Applied to u-boot-spi/master

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