All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up
@ 2016-11-02 15:15 Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 1/7] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

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

Phil Edworthy (7):
  spi: cadence_qspi: Fix clearing of pol/pha bits
  spi: cadence_qspi: Fix baud rate calculation
  spi: cadence_qspi: Use #define for bits instead of bit shifts
  spi: cadence_qspi: Clean up the #define names
  spi: cadence_qspi: Remove returns from end of void functions
  spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation
  spi: cadence_qspi: Support specifying the sample edge used

 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               | 164 +++++++++++++--------------
 4 files changed, 88 insertions(+), 91 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/7] spi: cadence_qspi: Fix clearing of pol/pha bits
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

Or'ing together bit positions is clearly wrong.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 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.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 1/7] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-02 19:39   ` Marek Vasut
  2016-11-02 15:15 ` [U-Boot] [PATCH 3/7] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

With the existing code, when the requested SPI clock rate is near
to the lowerest 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>
---
 drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2403e71..71bde1f 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -273,22 +273,13 @@ 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 = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
+	div = ((div + 1) / 2) - 1;
 
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/7] spi: cadence_qspi: Use #define for bits instead of bit shifts
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 1/7] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 4/7] spi: cadence_qspi: Clean up the #define names Phil Edworthy
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

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>
---
 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 71bde1f..d1e1b49 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;
@@ -302,11 +302,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.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 4/7] spi: cadence_qspi: Clean up the #define names
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
                   ` (2 preceding siblings ...)
  2016-11-02 15:15 ` [U-Boot] [PATCH 3/7] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 5/7] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

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 unneccesary brackets around the
constants.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 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 d1e1b49..639780c 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;
 }
@@ -328,9 +328,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
@@ -424,12 +424,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);
 	}
@@ -647,7 +647,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) {
@@ -676,21 +676,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;
 }
@@ -738,7 +738,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) {
@@ -763,20 +763,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;
 }
@@ -787,9 +787,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.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 5/7] spi: cadence_qspi: Remove returns from end of void functions
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
                   ` (3 preceding siblings ...)
  2016-11-02 15:15 ` [U-Boot] [PATCH 4/7] spi: cadence_qspi: Clean up the #define names Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 6/7] spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation Phil Edworthy
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 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 639780c..a0edeb8 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,
@@ -292,7 +289,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,
@@ -312,7 +308,6 @@ void cadence_qspi_apb_set_clk_mode(void *reg_base,
 	writel(reg, reg_base + CQSPI_REG_CONFIG);
 
 	cadence_qspi_apb_controller_enable(reg_base);
-	return;
 }
 
 void cadence_qspi_apb_chipselect(void *reg_base,
@@ -347,7 +342,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,
@@ -385,7 +379,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)
@@ -413,7 +406,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.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 6/7] spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
                   ` (4 preceding siblings ...)
  2016-11-02 15:15 ` [U-Boot] [PATCH 5/7] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-17 15:49   ` [U-Boot] [PATCH v2 6/7] spi: cadence_qspi: Fix CS timings Phil Edworthy
  2016-11-02 15:15 ` [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
  2016-11-25 12:32 ` [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
  7 siblings, 1 reply; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

This change copes with the delay being less than a SCLK period.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/spi/cadence_qspi_apb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index a0edeb8..533274d 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -170,7 +170,7 @@
 		CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
 
 #define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)		\
-	((((tdelay_ns) - (tsclk_ns)) / (tref_ns)))
+	((tdelay_ns) > (tsclk_ns)) ? ((((tdelay_ns) - (tsclk_ns)) / (tref_ns))) : 0
 
 #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)			\
 	(((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>	\
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
                   ` (5 preceding siblings ...)
  2016-11-02 15:15 ` [U-Boot] [PATCH 6/7] spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation Phil Edworthy
@ 2016-11-02 15:15 ` Phil Edworthy
  2016-11-04  5:57   ` Vignesh R
  2016-11-25 12:32 ` [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
  7 siblings, 1 reply; 15+ messages in thread
From: Phil Edworthy @ 2016-11-02 15:15 UTC (permalink / raw)
  To: u-boot

The HW manual does not give details about what the register
value for this bit actually does, other than "Choose edge on
which data outputs from flash memory will be sampled".

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

---
Our HW engineers tell me that it needs to be set for our hardware.
---
 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..71aa06a 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		: The edge on which data outputs from flash memory will
+			  be sampled.
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 1051afb..afe7cb2 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;
+	unsigned int edge = plat->sample_edge;
 	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, 1, 0);
+	cadence_qspi_apb_readdata_capture(base, 1, 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, 1, i);
+		cadence_qspi_apb_readdata_capture(base, 1, edge, i);
 
 		/* Enable back QSPI */
 		cadence_qspi_apb_controller_enable(base);
@@ -105,7 +107,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, 1, 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);
 
@@ -318,6 +321,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	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);
+	plat->sample_edge = fdtdec_get_int(blob, subnode, "sample-edge", 0);
 
 	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 a849f7b..3cd3b00 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;
+	unsigned int	sample_edge;
 };
 
 struct cadence_spi_priv {
@@ -73,6 +74,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);
+	unsigned int bypass, unsigned int 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 533274d..c43fde6 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
@@ -237,7 +238,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)
+	unsigned int bypass, unsigned int edge, unsigned int delay)
 {
 	unsigned int reg;
 	cadence_qspi_apb_controller_disable(reg_base);
@@ -249,6 +250,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.5.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation
  2016-11-02 15:15 ` [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
@ 2016-11-02 19:39   ` Marek Vasut
  2016-11-03  7:36     ` Phil Edworthy
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2016-11-02 19:39 UTC (permalink / raw)
  To: u-boot

On 11/02/2016 04:15 PM, Phil Edworthy wrote:
> With the existing code, when the requested SPI clock rate is near
> to the lowerest 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>
> ---
>  drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 2403e71..71bde1f 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -273,22 +273,13 @@ 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 = (ref_clk_hz + sclk_hz - 1) / sclk_hz;

DIV_ROUND_UP()

> +	div = ((div + 1) / 2) - 1;

Is this roundup() ?

>  	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
>  	      ref_clk_hz, sclk_hz, div);
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation
  2016-11-02 19:39   ` Marek Vasut
@ 2016-11-03  7:36     ` Phil Edworthy
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-03  7:36 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On 02 November 2016 19:39, Marek Vasut wrote:
> On 11/02/2016 04:15 PM, Phil Edworthy wrote:
> > With the existing code, when the requested SPI clock rate is near
> > to the lowerest 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>
> > ---
> >  drivers/spi/cadence_qspi_apb.c | 23 +++++++----------------
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 2403e71..71bde1f 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -273,22 +273,13 @@ 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 = (ref_clk_hz + sclk_hz - 1) / sclk_hz;
> 
> DIV_ROUND_UP()
Ah yes, that would be clearer!

> > +	div = ((div + 1) / 2) - 1;
> Is this roundup() ?
It's DIV_ROUND_UP() - 1

> 
> >  	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
> >  	      ref_clk_hz, sclk_hz, div);
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Thanks
Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used
  2016-11-02 15:15 ` [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
@ 2016-11-04  5:57   ` Vignesh R
  2016-11-04  9:07     ` Phil Edworthy
  0 siblings, 1 reply; 15+ messages in thread
From: Vignesh R @ 2016-11-04  5:57 UTC (permalink / raw)
  To: u-boot

Hi,

On Wednesday 02 November 2016 08:45 PM, Phil Edworthy wrote:
> The HW manual does not give details about what the register
> value for this bit actually does, other than "Choose edge on
> which data outputs from flash memory will be sampled".
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> ---
> Our HW engineers tell me that it needs to be set for our hardware.
> ---
>  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..71aa06a 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		: The edge on which data outputs from flash memory will
> +			  be sampled.

Could this be changed to bool property say sample-edge-rising?

sample-edge-rising(optional): data outputs from flash memory will
		  be sampled at the rising edge. Default is falling edge


-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used
  2016-11-04  5:57   ` Vignesh R
@ 2016-11-04  9:07     ` Phil Edworthy
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-04  9:07 UTC (permalink / raw)
  To: u-boot

Hi Vignesh,

On 04 November 2016 05:57, Vignesh R wrote:
> On Wednesday 02 November 2016 08:45 PM, Phil Edworthy wrote:
> > The HW manual does not give details about what the register
> > value for this bit actually does, other than "Choose edge on
> > which data outputs from flash memory will be sampled".
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >
> > ---
> > Our HW engineers tell me that it needs to be set for our hardware.
> > ---
> >  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..71aa06a 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		: The edge on which data outputs from flash
> memory will
> > +			  be sampled.
> 
> Could this be changed to bool property say sample-edge-rising?
> 
> sample-edge-rising(optional): data outputs from flash memory will
> 		  be sampled at the rising edge. Default is falling edge
Whilst this makes sense, the trouble I have is that I don't know if the
register setting  is making it use rising or falling edge sampling. I'll try to
find out again...
 
> --
> Regards
> Vignesh

Thanks
Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v2 6/7] spi: cadence_qspi: Fix CS timings
  2016-11-02 15:15 ` [U-Boot] [PATCH 6/7] spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation Phil Edworthy
@ 2016-11-17 15:49   ` Phil Edworthy
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Edworthy @ 2016-11-17 15:49 UTC (permalink / raw)
  To: u-boot

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>
---
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 72c7c79..4d53aad 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)
@@ -357,16 +354,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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up
  2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
                   ` (6 preceding siblings ...)
  2016-11-02 15:15 ` [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
@ 2016-11-25 12:32 ` Phil Edworthy
  2016-11-25 14:15   ` Jagan Teki
  7 siblings, 1 reply; 15+ messages in thread
From: Phil Edworthy @ 2016-11-25 12:32 UTC (permalink / raw)
  To: u-boot

Hi Jagan,

On 02 November 2016 15:16, Phil wrote:
> This series has fixes, patches to clean the code up, and add support for
> specifying the sampling edge.
> 
> Phil Edworthy (7):
>   spi: cadence_qspi: Fix clearing of pol/pha bits
>   spi: cadence_qspi: Fix baud rate calculation
>   spi: cadence_qspi: Use #define for bits instead of bit shifts
>   spi: cadence_qspi: Clean up the #define names
>   spi: cadence_qspi: Remove returns from end of void functions
>   spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation
>   spi: cadence_qspi: Support specifying the sample edge used
> 
>  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               | 164 +++++++++++++--------------
>  4 files changed, 88 insertions(+), 91 deletions(-)

I've sent a couple of new versions of some of these patches based on feedback
and something I noticed, but most have not had any comments.
Would you like me to send the whole series again with the latest versions?

Thanks
Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up
  2016-11-25 12:32 ` [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
@ 2016-11-25 14:15   ` Jagan Teki
  0 siblings, 0 replies; 15+ messages in thread
From: Jagan Teki @ 2016-11-25 14:15 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 25, 2016 at 6:02 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> Hi Jagan,
>
> On 02 November 2016 15:16, Phil wrote:
>> This series has fixes, patches to clean the code up, and add support for
>> specifying the sampling edge.
>>
>> Phil Edworthy (7):
>>   spi: cadence_qspi: Fix clearing of pol/pha bits
>>   spi: cadence_qspi: Fix baud rate calculation
>>   spi: cadence_qspi: Use #define for bits instead of bit shifts
>>   spi: cadence_qspi: Clean up the #define names
>>   spi: cadence_qspi: Remove returns from end of void functions
>>   spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation
>>   spi: cadence_qspi: Support specifying the sample edge used
>>
>>  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               | 164 +++++++++++++--------------
>>  4 files changed, 88 insertions(+), 91 deletions(-)
>
> I've sent a couple of new versions of some of these patches based on feedback
> and something I noticed, but most have not had any comments.
> Would you like me to send the whole series again with the latest versions?

Yes, please do that.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-11-25 14:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-02 15:15 [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 1/7] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 2/7] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
2016-11-02 19:39   ` Marek Vasut
2016-11-03  7:36     ` Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 3/7] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 4/7] spi: cadence_qspi: Clean up the #define names Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 5/7] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 6/7] spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation Phil Edworthy
2016-11-17 15:49   ` [U-Boot] [PATCH v2 6/7] spi: cadence_qspi: Fix CS timings Phil Edworthy
2016-11-02 15:15 ` [U-Boot] [PATCH 7/7] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
2016-11-04  5:57   ` Vignesh R
2016-11-04  9:07     ` Phil Edworthy
2016-11-25 12:32 ` [U-Boot] [PATCH 0/7] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
2016-11-25 14:15   ` Jagan Teki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.