* [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up
@ 2016-11-25 14:38 Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
` (8 more replies)
0 siblings, 9 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 UTC (permalink / raw)
To: u-boot
This series has fixes, patches to clean the code up, and add support for
specifying the sampling edge.
Changed in v2:
spi: cadence_qspi: Fix baud rate calculation
spi: cadence_qspi: Fix CS timings (was "Fix CQSPI_CAL_DELAY calculation")
spi: cadence_qspi: Support specifying the sample edge used
Added in v2:
spi: cadence_qspi: Better debug information on the SPI clock rate
Phil Edworthy (8):
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: Remove returns from end of void functions
spi: cadence_qspi: Fix CS timings
spi: cadence_qspi: Support specifying the sample edge used
doc/device-tree-bindings/spi/spi-cadence.txt | 2 +
drivers/spi/cadence_qspi.c | 13 +-
drivers/spi/cadence_qspi.h | 3 +-
drivers/spi/cadence_qspi_apb.c | 191 +++++++++++++--------------
4 files changed, 104 insertions(+), 105 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 14:56 ` Marek Vasut
2016-11-25 15:29 ` Jagan Teki
2016-11-25 14:38 ` [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
` (7 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 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.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 14:57 ` Marek Vasut
2016-11-25 15:41 ` Jagan Teki
2016-11-25 14:38 ` [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate Phil Edworthy
` (6 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 UTC (permalink / raw)
To: u-boot
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>
---
v2:
- Use the DIV_ROUND_UP macro
---
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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
+ div = DIV_ROUND_UP(div, 2) - 1;
debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
ref_clk_hz, sclk_hz, div);
--
2.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 14:58 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
` (5 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 UTC (permalink / raw)
To: u-boot
Show what the output clock rate actually is.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
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 b9e0df7..3ae4b5a 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -281,13 +281,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
div = DIV_ROUND_UP(div, 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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (2 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 14:59 ` Marek Vasut
2016-11-25 16:06 ` Jagan Teki
2016-11-25 14:38 ` [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names Phil Edworthy
` (4 subsequent siblings)
8 siblings, 2 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 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 3ae4b5a..cd46a15 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.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (3 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 15:01 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
` (3 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 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 unnecessary 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 cd46a15..e7d8320 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.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (4 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 15:01 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings Phil Edworthy
` (2 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 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 e7d8320..1cd636a 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.7.4
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (5 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 15:04 ` Marek Vasut
2016-11-28 12:48 ` See, Chin Liang
2016-11-25 14:38 ` [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
2016-11-28 8:07 ` [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Jagan Teki
8 siblings, 2 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 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 1cd636a..56ad952 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] 33+ messages in thread
* [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (6 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings Phil Edworthy
@ 2016-11-25 14:38 ` Phil Edworthy
2016-11-25 15:05 ` Marek Vasut
2016-11-28 13:37 ` See, Chin Liang
2016-11-28 8:07 ` [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Jagan Teki
8 siblings, 2 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 14:38 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
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 | 13 +++++++++----
drivers/spi/cadence_qspi.h | 3 ++-
drivers/spi/cadence_qspi_apb.c | 8 +++++++-
4 files changed, 20 insertions(+), 6 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 1051afb..16fb18e 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, 1, 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, 1, i);
+ cadence_qspi_apb_readdata_capture(base, true, 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, 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);
@@ -298,6 +301,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);
@@ -317,7 +321,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->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
+ 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 a849f7b..ad1dbae 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 {
@@ -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);
+ 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 56ad952..e43973c 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,
- unsigned int 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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
@ 2016-11-25 14:56 ` Marek Vasut
2016-11-25 15:29 ` Jagan Teki
1 sibling, 0 replies; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 14:56 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> 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);
Oh, nice.
Acked-by: Marek Vasut <marek.vasut@gmail.com>
> reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 14:38 ` [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
@ 2016-11-25 14:57 ` Marek Vasut
2016-11-25 15:19 ` Phil Edworthy
2016-11-25 15:41 ` Jagan Teki
1 sibling, 1 reply; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 14:57 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy 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>
> ---
> v2:
> - Use the DIV_ROUND_UP macro
> ---
> 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> + div = DIV_ROUND_UP(div, 2) - 1;
Same as you did for u-boot, right ?
Acked-by: Marek Vasut <marek.vasut@gmail.com>
> 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] 33+ messages in thread
* [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate
2016-11-25 14:38 ` [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate Phil Edworthy
@ 2016-11-25 14:58 ` Marek Vasut
0 siblings, 0 replies; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 14:58 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> Show what the output clock rate actually is.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> 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 b9e0df7..3ae4b5a 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -281,13 +281,13 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
> div = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> div = DIV_ROUND_UP(div, 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)));
Fine by me:
Acked-by: Marek Vasut <marek.vasut@gmail.com>
> reg |= (div << CQSPI_REG_CONFIG_BAUD_LSB);
> writel(reg, reg_base + CQSPI_REG_CONFIG);
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts
2016-11-25 14:38 ` [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
@ 2016-11-25 14:59 ` Marek Vasut
2016-11-25 15:22 ` Phil Edworthy
2016-11-25 16:06 ` Jagan Teki
1 sibling, 1 reply; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 14:59 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy 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>
> ---
> 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 3ae4b5a..cd46a15 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);
You can also drop the () around the whole expression here.
>
> - 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;
Except the minor nit above,
Acked-by: Marek Vasut <marek.vasut@gmail.com>
> writel(reg, reg_base + CQSPI_REG_CONFIG);
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names
2016-11-25 14:38 ` [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names Phil Edworthy
@ 2016-11-25 15:01 ` Marek Vasut
0 siblings, 0 replies; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 15:01 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy 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>
I guess this is done in automated way ?
Anyway, thanks !
Acked-by: Marek Vasut <marek.vasut@gmail.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 cd46a15..e7d8320 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 */
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions
2016-11-25 14:38 ` [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
@ 2016-11-25 15:01 ` Marek Vasut
0 siblings, 0 replies; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 15:01 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Nice find :)
Acked-by: Marek Vasut <marek.vasut@gmail.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 e7d8320..1cd636a 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,
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings
2016-11-25 14:38 ` [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings Phil Edworthy
@ 2016-11-25 15:04 ` Marek Vasut
2016-11-28 12:48 ` See, Chin Liang
1 sibling, 0 replies; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 15:04 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> 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>
Looks fine to me, but Dinh/Chin, can you check whether this doesn't
break SoCFPGA ?
> ---
> 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 1cd636a..56ad952 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);
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used
2016-11-25 14:38 ` [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
@ 2016-11-25 15:05 ` Marek Vasut
2016-11-25 15:24 ` Phil Edworthy
2016-11-28 13:37 ` See, Chin Liang
1 sibling, 1 reply; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 15:05 UTC (permalink / raw)
To: u-boot
On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> 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.
The commit message does not match the subject, but I think this needs
splitting into two patches.
> Also change the code to use a bool for the bypass arg.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>
> ---
> 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 | 13 +++++++++----
> drivers/spi/cadence_qspi.h | 3 ++-
> drivers/spi/cadence_qspi_apb.c | 8 +++++++-
> 4 files changed, 20 insertions(+), 6 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 1051afb..16fb18e 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, 1, 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, 1, i);
> + cadence_qspi_apb_readdata_capture(base, true, 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, 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);
>
> @@ -298,6 +301,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);
> @@ -317,7 +321,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->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> + 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 a849f7b..ad1dbae 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 {
> @@ -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);
> + 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 56ad952..e43973c 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,
> - unsigned int 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);
>
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 14:57 ` Marek Vasut
@ 2016-11-25 15:19 ` Phil Edworthy
2016-11-25 15:53 ` Marek Vasut
0 siblings, 1 reply; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 15:19 UTC (permalink / raw)
To: u-boot
Hi Marek,
On 25 November 2016 14:58 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy 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>
> > ---
> > v2:
> > - Use the DIV_ROUND_UP macro
> > ---
> > 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> > + div = DIV_ROUND_UP(div, 2) - 1;
>
> Same as you did for u-boot, right ?
Eh? This is u-boot :)
> Acked-by: Marek Vasut <marek.vasut@gmail.com>
>
> > 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] 33+ messages in thread
* [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts
2016-11-25 14:59 ` Marek Vasut
@ 2016-11-25 15:22 ` Phil Edworthy
0 siblings, 0 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 15:22 UTC (permalink / raw)
To: u-boot
Hi Marek,
On 25 November 2016 15:00 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy 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>
> > ---
> > 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 3ae4b5a..cd46a15 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);
>
> You can also drop the () around the whole expression here.
Ok, will do.
> >
> > - 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;
>
> Except the minor nit above,
>
> Acked-by: Marek Vasut <marek.vasut@gmail.com>
>
> > writel(reg, reg_base + CQSPI_REG_CONFIG);
> >
> >
>
>
> --
> Best regards,
> Marek Vasut
Thanks
Phil
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used
2016-11-25 15:05 ` Marek Vasut
@ 2016-11-25 15:24 ` Phil Edworthy
0 siblings, 0 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 15:24 UTC (permalink / raw)
To: u-boot
Hi Marek,
On 25 November 2016 15:06 Marek Vasut wrote:
> On 11/25/2016 03:38 PM, Phil Edworthy wrote:
> > 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.
>
> The commit message does not match the subject, but I think this needs
> splitting into two patches.
Ah, right the subject was covering it.
I'll split the patch in two.
> > Also change the code to use a bool for the bypass arg.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >
> > ---
> > 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 | 13 +++++++++----
> > drivers/spi/cadence_qspi.h | 3 ++-
> > drivers/spi/cadence_qspi_apb.c | 8 +++++++-
> > 4 files changed, 20 insertions(+), 6 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 1051afb..16fb18e 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, 1, 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, 1, i);
> > + cadence_qspi_apb_readdata_capture(base, true, 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, 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);
> >
> > @@ -298,6 +301,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);
> > @@ -317,7 +321,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->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> > + 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 a849f7b..ad1dbae 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 {
> > @@ -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);
> > + 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 56ad952..e43973c 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,
> > - unsigned int 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);
> >
> >
>
>
> --
> Best regards,
> Marek Vasut
Thanks
Phil
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
2016-11-25 14:56 ` Marek Vasut
@ 2016-11-25 15:29 ` Jagan Teki
2016-11-25 15:34 ` Phil Edworthy
1 sibling, 1 reply; 33+ messages in thread
From: Jagan Teki @ 2016-11-25 15:29 UTC (permalink / raw)
To: u-boot
On Fri, Nov 25, 2016 at 8:08 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>
> ---
> 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);
OK, but see below
>
> reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
This existing code look not easy to understand for me with doing & and
<< operations with numeric 0x1. what about this
Say for example POL and PHA on cadence has with bit 1 and 2
CQSPI_REG_CONFIG_CLK_POL_LSB BIT(1)
CQSPI_REG_CONFIG_CLK_PHA_LSB BIT(2)
reg &= ~(CQSPI_REG_CONFIG_CLK_PHA_LSB | CQSPI_REG_CONFIG_CLK_POL_LSB);
if (mode & SPI_CPHA)
reg |= CQSPI_REG_CONFIG_CLK_PHA_LSB;
if (mode & SPI_CPOL)
reg |= CQSPI_REG_CONFIG_CLK_POL_LSB;
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits
2016-11-25 15:29 ` Jagan Teki
@ 2016-11-25 15:34 ` Phil Edworthy
0 siblings, 0 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 15:34 UTC (permalink / raw)
To: u-boot
Hi Jagan,
On 25 November 2016 15:29 Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 8:08 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>
> > ---
> > 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);
>
> OK, but see below
>
> >
> > reg |= ((clk_pol & 0x1) << CQSPI_REG_CONFIG_CLK_POL_LSB);
> > reg |= ((clk_pha & 0x1) << CQSPI_REG_CONFIG_CLK_PHA_LSB);
>
> This existing code look not easy to understand for me with doing & and
> << operations with numeric 0x1. what about this
>
> Say for example POL and PHA on cadence has with bit 1 and 2
> CQSPI_REG_CONFIG_CLK_POL_LSB BIT(1)
> CQSPI_REG_CONFIG_CLK_PHA_LSB BIT(2)
>
> reg &= ~(CQSPI_REG_CONFIG_CLK_PHA_LSB |
> CQSPI_REG_CONFIG_CLK_POL_LSB);
>
> if (mode & SPI_CPHA)
> reg |= CQSPI_REG_CONFIG_CLK_PHA_LSB;
> if (mode & SPI_CPOL)
> reg |= CQSPI_REG_CONFIG_CLK_POL_LSB;
I completely agree. This is addressed in patch 4 along with the other
code that defines shifts.
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
Thanks
Phil
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 14:38 ` [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
2016-11-25 14:57 ` Marek Vasut
@ 2016-11-25 15:41 ` Jagan Teki
2016-11-25 16:05 ` Phil Edworthy
1 sibling, 1 reply; 33+ messages in thread
From: Jagan Teki @ 2016-11-25 15:41 UTC (permalink / raw)
To: u-boot
On Fri, Nov 25, 2016 at 8:08 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>
> ---
> v2:
> - Use the DIV_ROUND_UP macro
> ---
> 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> + div = DIV_ROUND_UP(div, 2) - 1;
What about div = (ref / (sclk * 2)) -1 that means
div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) -1;
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 15:19 ` Phil Edworthy
@ 2016-11-25 15:53 ` Marek Vasut
2016-11-25 16:07 ` Jagan Teki
0 siblings, 1 reply; 33+ messages in thread
From: Marek Vasut @ 2016-11-25 15:53 UTC (permalink / raw)
To: u-boot
On 11/25/2016 04:19 PM, Phil Edworthy wrote:
> Hi Marek,
>
> On 25 November 2016 14:58 Marek Vasut wrote:
>> On 11/25/2016 03:38 PM, Phil Edworthy 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>
>>> ---
>>> v2:
>>> - Use the DIV_ROUND_UP macro
>>> ---
>>> 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
>>> + div = DIV_ROUND_UP(div, 2) - 1;
>>
>> Same as you did for u-boot, right ?
> Eh? This is u-boot :)
Yeah, I realized that too when I saw the later patches. I thought this
was for the CQSPI in Linux at right.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 15:41 ` Jagan Teki
@ 2016-11-25 16:05 ` Phil Edworthy
0 siblings, 0 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-25 16:05 UTC (permalink / raw)
To: u-boot
Hi Jagan,
On 25 November 2016 15:42 Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 8:08 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>
> > ---
> > v2:
> > - Use the DIV_ROUND_UP macro
> > ---
> > 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
> > + div = DIV_ROUND_UP(div, 2) - 1;
>
> What about div = (ref / (sclk * 2)) -1 that means
> div = DIV_ROUND_UP(ref_clk_hz, sclk_hz * 2) -1;
Yes, that is also the same result. I'll change the code.
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
Thanks
Phil
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts
2016-11-25 14:38 ` [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
2016-11-25 14:59 ` Marek Vasut
@ 2016-11-25 16:06 ` Jagan Teki
1 sibling, 0 replies; 33+ messages in thread
From: Jagan Teki @ 2016-11-25 16:06 UTC (permalink / raw)
To: u-boot
On Fri, Nov 25, 2016 at 8:08 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>
> ---
> 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 3ae4b5a..cd46a15 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;
Take the mode directly from cadence_spi_set_mode and do mode checking
here, this defiantly reduce one func arg and local variables from
cadence_spi_set_mode
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation
2016-11-25 15:53 ` Marek Vasut
@ 2016-11-25 16:07 ` Jagan Teki
0 siblings, 0 replies; 33+ messages in thread
From: Jagan Teki @ 2016-11-25 16:07 UTC (permalink / raw)
To: u-boot
On Fri, Nov 25, 2016 at 9:23 PM, Marek Vasut <marex@denx.de> wrote:
> On 11/25/2016 04:19 PM, Phil Edworthy wrote:
>> Hi Marek,
>>
>> On 25 November 2016 14:58 Marek Vasut wrote:
>>> On 11/25/2016 03:38 PM, Phil Edworthy 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>
>>>> ---
>>>> v2:
>>>> - Use the DIV_ROUND_UP macro
>>>> ---
>>>> 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..b9e0df7 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 = DIV_ROUND_UP(ref_clk_hz, sclk_hz);
>>>> + div = DIV_ROUND_UP(div, 2) - 1;
>>>
>>> Same as you did for u-boot, right ?
>> Eh? This is u-boot :)
>
> Yeah, I realized that too when I saw the later patches. I thought this
> was for the CQSPI in Linux at right.
Yes, and look good as well.
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
` (7 preceding siblings ...)
2016-11-25 14:38 ` [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
@ 2016-11-28 8:07 ` Jagan Teki
2016-11-28 12:50 ` Marek Vasut
8 siblings, 1 reply; 33+ messages in thread
From: Jagan Teki @ 2016-11-28 8:07 UTC (permalink / raw)
To: u-boot
On Fri, Nov 25, 2016 at 8:08 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.
>
> Changed in v2:
> spi: cadence_qspi: Fix baud rate calculation
> spi: cadence_qspi: Fix CS timings (was "Fix CQSPI_CAL_DELAY calculation")
> spi: cadence_qspi: Support specifying the sample edge used
>
> Added in v2:
> spi: cadence_qspi: Better debug information on the SPI clock rate
>
> Phil Edworthy (8):
> spi: cadence_qspi: Fix clearing of pol/pha bits
> spi: cadence_qspi: Fix baud rate calculation
Please fix the comment for this patch.
> spi: cadence_qspi: Better debug information on the SPI clock rate
> spi: cadence_qspi: Use #define for bits instead of bit shifts
And this one,
> spi: cadence_qspi: Clean up the #define names
> spi: cadence_qspi: Remove returns from end of void functions
> spi: cadence_qspi: Fix CS timings
> spi: cadence_qspi: Support specifying the sample edge used
All look OK, expect above two.
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings
2016-11-25 14:38 ` [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings Phil Edworthy
2016-11-25 15:04 ` Marek Vasut
@ 2016-11-28 12:48 ` See, Chin Liang
2016-11-29 10:13 ` Phil Edworthy
1 sibling, 1 reply; 33+ messages in thread
From: See, Chin Liang @ 2016-11-28 12:48 UTC (permalink / raw)
To: u-boot
Hi Phil,
On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote:
>
> 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 1cd636a..56ad952 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;
Any reason we need this?
The DIV_ROUND_UP or previous + 1 in algo will ensure its more than a
SCLK period.
Thanks
Chin Liang
>
> +???????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
>
>
> ________________________________
>
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up
2016-11-28 8:07 ` [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Jagan Teki
@ 2016-11-28 12:50 ` Marek Vasut
2016-11-28 13:32 ` Jagan Teki
0 siblings, 1 reply; 33+ messages in thread
From: Marek Vasut @ 2016-11-28 12:50 UTC (permalink / raw)
To: u-boot
On 11/28/2016 09:07 AM, Jagan Teki wrote:
> On Fri, Nov 25, 2016 at 8:08 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.
>>
>> Changed in v2:
>> spi: cadence_qspi: Fix baud rate calculation
>> spi: cadence_qspi: Fix CS timings (was "Fix CQSPI_CAL_DELAY calculation")
>> spi: cadence_qspi: Support specifying the sample edge used
>>
>> Added in v2:
>> spi: cadence_qspi: Better debug information on the SPI clock rate
>>
>> Phil Edworthy (8):
>> spi: cadence_qspi: Fix clearing of pol/pha bits
>> spi: cadence_qspi: Fix baud rate calculation
>
> Please fix the comment for this patch.
Fix how ? It seems perfectly fine to me, so explain.
>> spi: cadence_qspi: Better debug information on the SPI clock rate
>> spi: cadence_qspi: Use #define for bits instead of bit shifts
>
> And this one,
DTTO, seems perfectly fine.
>> spi: cadence_qspi: Clean up the #define names
>> spi: cadence_qspi: Remove returns from end of void functions
>> spi: cadence_qspi: Fix CS timings
>> spi: cadence_qspi: Support specifying the sample edge used
>
> All look OK, expect above two.
>
> thanks!
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up
2016-11-28 12:50 ` Marek Vasut
@ 2016-11-28 13:32 ` Jagan Teki
0 siblings, 0 replies; 33+ messages in thread
From: Jagan Teki @ 2016-11-28 13:32 UTC (permalink / raw)
To: u-boot
On Mon, Nov 28, 2016 at 6:20 PM, Marek Vasut <marex@denx.de> wrote:
> On 11/28/2016 09:07 AM, Jagan Teki wrote:
>> On Fri, Nov 25, 2016 at 8:08 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.
>>>
>>> Changed in v2:
>>> spi: cadence_qspi: Fix baud rate calculation
>>> spi: cadence_qspi: Fix CS timings (was "Fix CQSPI_CAL_DELAY calculation")
>>> spi: cadence_qspi: Support specifying the sample edge used
>>>
>>> Added in v2:
>>> spi: cadence_qspi: Better debug information on the SPI clock rate
>>>
>>> Phil Edworthy (8):
>>> spi: cadence_qspi: Fix clearing of pol/pha bits
>>> spi: cadence_qspi: Fix baud rate calculation
>>
>> Please fix the comment for this patch.
>
> Fix how ? It seems perfectly fine to me, so explain.
>
>>> spi: cadence_qspi: Better debug information on the SPI clock rate
>>> spi: cadence_qspi: Use #define for bits instead of bit shifts
>>
>> And this one,
>
> DTTO, seems perfectly fine.
See my comments on these patch threads.
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used
2016-11-25 14:38 ` [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
2016-11-25 15:05 ` Marek Vasut
@ 2016-11-28 13:37 ` See, Chin Liang
1 sibling, 0 replies; 33+ messages in thread
From: See, Chin Liang @ 2016-11-28 13:37 UTC (permalink / raw)
To: u-boot
Hi Phil,
On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote:
> 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.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>
> ---
> 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???????????????????| 13 +++++++++----
> ?drivers/spi/cadence_qspi.h???????????????????|??3 ++-
> ?drivers/spi/cadence_qspi_apb.c???????????????|??8 +++++++-
> ?4 files changed, 20 insertions(+), 6 deletions(-)
>
[..]
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c
> index 56ad952..e43973c 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)
Actually we don't have this edge bit at SOCFPGA.
But no harm as its unused bit at SOCFPGA today
Thanks
Chin Liang
^ permalink raw reply [flat|nested] 33+ messages in thread
* [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings
2016-11-28 12:48 ` See, Chin Liang
@ 2016-11-29 10:13 ` Phil Edworthy
0 siblings, 0 replies; 33+ messages in thread
From: Phil Edworthy @ 2016-11-29 10:13 UTC (permalink / raw)
To: u-boot
Hi Chin Liang,
On 28 November 2016 12:49 See, Chin Liang wrote:
> On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote:
> >
> > 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 1cd636a..56ad952 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;
> Any reason we need this?
> The DIV_ROUND_UP or previous + 1 in algo will ensure its more than a
> SCLK period.
In general, all of these CS timing should be optimal. I measured differences
in throughput with the sub-optimal setting. Admittedly, the difference is
small but still we should set it correctly.
> Thanks
> Chin Liang
>
> >
> > +???????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
> >
Thanks
Phil
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-11-29 10:13 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25 14:38 [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 1/8] spi: cadence_qspi: Fix clearing of pol/pha bits Phil Edworthy
2016-11-25 14:56 ` Marek Vasut
2016-11-25 15:29 ` Jagan Teki
2016-11-25 15:34 ` Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 2/8] spi: cadence_qspi: Fix baud rate calculation Phil Edworthy
2016-11-25 14:57 ` Marek Vasut
2016-11-25 15:19 ` Phil Edworthy
2016-11-25 15:53 ` Marek Vasut
2016-11-25 16:07 ` Jagan Teki
2016-11-25 15:41 ` Jagan Teki
2016-11-25 16:05 ` Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 3/8] spi: cadence_qspi: Better debug information on the SPI clock rate Phil Edworthy
2016-11-25 14:58 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 4/8] spi: cadence_qspi: Use #define for bits instead of bit shifts Phil Edworthy
2016-11-25 14:59 ` Marek Vasut
2016-11-25 15:22 ` Phil Edworthy
2016-11-25 16:06 ` Jagan Teki
2016-11-25 14:38 ` [U-Boot] [PATCH v2 5/8] spi: cadence_qspi: Clean up the #define names Phil Edworthy
2016-11-25 15:01 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 6/8] spi: cadence_qspi: Remove returns from end of void functions Phil Edworthy
2016-11-25 15:01 ` Marek Vasut
2016-11-25 14:38 ` [U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings Phil Edworthy
2016-11-25 15:04 ` Marek Vasut
2016-11-28 12:48 ` See, Chin Liang
2016-11-29 10:13 ` Phil Edworthy
2016-11-25 14:38 ` [U-Boot] [PATCH v2 8/8] spi: cadence_qspi: Support specifying the sample edge used Phil Edworthy
2016-11-25 15:05 ` Marek Vasut
2016-11-25 15:24 ` Phil Edworthy
2016-11-28 13:37 ` See, Chin Liang
2016-11-28 8:07 ` [U-Boot] [PATCH v2 0/8] SF: Cadence QSPI driver fixes and clean up Jagan Teki
2016-11-28 12:50 ` Marek Vasut
2016-11-28 13:32 ` 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.