[U-Boot] [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
This patchset: - removes sram polling while reading/writing from flash. - fixes trigger base & transfer start address register programming. This fix superseeds the previous patch "spi: cadence_qspi: Fix the indirect ahb trigger address setting" - adds support to get fifo width from device tree
Vikas Manocha (7): spi: cadence_qspi: remove sram polling from flash read spi: cadence_qspi: read can be independent of fifo width spi: cadence_qspi: remove sram polling from flash write spi: cadence_qspi: move trigger base configuration in init spi: cadence_qspi: fix indirect read/write start address spi: cadence_qspi: fix base trigger address & transfer start address spi: cadence_qspi: get fifo width from device tree
arch/arm/dts/socfpga.dtsi | 2 + arch/arm/dts/stv0991.dts | 4 +- drivers/spi/cadence_qspi.c | 14 ++-- drivers/spi/cadence_qspi.h | 6 +- drivers/spi/cadence_qspi_apb.c | 140 ++++++++++------------------------------ 5 files changed, 50 insertions(+), 116 deletions(-)
There is no need to check for sram fill level. If sram is empty, cpu will go in the wait state till the time data is available from flash.
Also Relying on SRAM fill level only for deciding when the data should be fetched from the local SRAM is not most efficient approach, particulary if we are working on high data rates.
It should be noticed that after one SRAM word is collected, the information is forwarded into system domain and then synchronized into register domain (apb). If we are using slow APB and AHB clks, SRAM fill level might not be up-to-dated because of latency cycles needed for synchronization. For example in case we are getting SRAM fill level equal to 10 locations but in reality there were 2 another words completed and actual level is 12 but information may not be synchronized yet because of the synchronization latency on APB domain.
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- drivers/spi/cadence_qspi_apb.c | 45 +++++----------------------------------- 1 file changed, 5 insertions(+), 40 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 00a115f..8eeb423 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -198,7 +198,7 @@ static unsigned int cadence_qspi_apb_cmd2addr(const unsigned char *addr_buf, return addr; }
-static void cadence_qspi_apb_read_fifo_data(void *dest, +static int cadence_qspi_apb_read_fifo_data(void *dest, const void *src_ahb_addr, unsigned int bytes) { unsigned int temp; @@ -219,7 +219,7 @@ static void cadence_qspi_apb_read_fifo_data(void *dest, dest_ptr++; }
- return; + return 0; }
static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr, @@ -246,42 +246,6 @@ static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr, return; }
-/* Read from SRAM FIFO with polling SRAM fill level. */ -static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr, - const void *src_addr, unsigned int num_bytes) -{ - unsigned int remaining = num_bytes; - unsigned int retry; - unsigned int sram_level = 0; - unsigned char *dest = (unsigned char *)dest_addr; - - while (remaining > 0) { - retry = CQSPI_REG_RETRY; - while (retry--) { - sram_level = CQSPI_GET_RD_SRAM_LEVEL(reg_base); - if (sram_level) - break; - udelay(1); - } - - if (!retry) { - printf("QSPI: No receive data after polling for %d times\n", - CQSPI_REG_RETRY); - return -1; - } - - sram_level *= CQSPI_FIFO_WIDTH; - sram_level = sram_level > remaining ? remaining : sram_level; - - /* Read data from FIFO. */ - cadence_qspi_apb_read_fifo_data(dest, src_addr, sram_level); - dest += sram_level; - remaining -= sram_level; - udelay(1); - } - return 0; -} - /* Write to SRAM FIFO with polling SRAM fill level. */ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes) @@ -759,9 +723,10 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, /* Start the indirect read transfer */ writel(CQSPI_REG_INDIRECTRD_START_MASK, plat->regbase + CQSPI_REG_INDIRECTRD); + udelay(1);
- if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf, - (const void *)plat->ahbbase, rxlen)) + if (cadence_qspi_apb_read_fifo_data((void *)rxbuf, + (const void *)plat->ahbbase, rxlen)) goto failrd;
/* Check flash indirect controller */
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- drivers/spi/cadence_qspi_apb.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 8eeb423..794d51d 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -206,18 +206,16 @@ static int cadence_qspi_apb_read_fifo_data(void *dest, unsigned int *dest_ptr = (unsigned int *)dest; unsigned int *src_ptr = (unsigned int *)src_ahb_addr;
- while (remaining > 0) { - if (remaining >= CQSPI_FIFO_WIDTH) { - *dest_ptr = readl(src_ptr); - remaining -= CQSPI_FIFO_WIDTH; - } else { - /* dangling bytes */ - temp = readl(src_ptr); - memcpy(dest_ptr, &temp, remaining); - break; - } + while (remaining >= sizeof(dest_ptr)) { + *dest_ptr = readl(src_ptr); + remaining -= sizeof(dest_ptr); dest_ptr++; } + if (remaining) { + /* dangling bytes */ + temp = readl(src_ptr); + memcpy(dest_ptr, &temp, remaining); + }
return 0; }
There is no need to poll sram level before writing to flash, data going to SRAM till sram is full, after that backpressure will take over.
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- drivers/spi/cadence_qspi_apb.c | 59 ++++++++++------------------------------ 1 file changed, 15 insertions(+), 44 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 794d51d..403230a 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -220,63 +220,34 @@ static int cadence_qspi_apb_read_fifo_data(void *dest, return 0; }
-static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr, - const void *src, unsigned int bytes) -{ - unsigned int temp; - int remaining = bytes; - unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr; - unsigned int *src_ptr = (unsigned int *)src; - - while (remaining > 0) { - if (remaining >= CQSPI_FIFO_WIDTH) { - writel(*src_ptr, dest_ptr); - remaining -= sizeof(unsigned int); - } else { - /* dangling bytes */ - memcpy(&temp, src_ptr, remaining); - writel(temp, dest_ptr); - break; - } - src_ptr++; - } - - return; -} - -/* Write to SRAM FIFO with polling SRAM fill level. */ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes) { - const void *reg_base = plat->regbase; - void *dest_addr = plat->ahbbase; - unsigned int retry = CQSPI_REG_RETRY; - unsigned int sram_level; + int i = 0; + unsigned int *dest_addr = plat->trigger_base; unsigned int wr_bytes; - unsigned char *src = (unsigned char *)src_addr; + unsigned int *src_ptr = (unsigned int *)src_addr; int remaining = num_bytes; unsigned int page_size = plat->page_size; - unsigned int sram_threshold_words = CQSPI_REG_SRAM_THRESHOLD_WORDS;
while (remaining > 0) { - retry = CQSPI_REG_RETRY; - while (retry--) { - sram_level = CQSPI_GET_WR_SRAM_LEVEL(reg_base); - if (sram_level <= sram_threshold_words) - break; - } - if (!retry) { - printf("QSPI: SRAM fill level (0x%08x) not hit lower expected level (0x%08x)", - sram_level, sram_threshold_words); - return -1; - } /* Write a page or remaining bytes. */ wr_bytes = (remaining > page_size) ? page_size : remaining;
- cadence_qspi_apb_write_fifo_data(dest_addr, src, wr_bytes); - src += wr_bytes; remaining -= wr_bytes; + while (wr_bytes >= CQSPI_FIFO_WIDTH) { + for (i = 0; i < CQSPI_FIFO_WIDTH/sizeof(dest_addr); i++) + writel(*(src_ptr+i), dest_addr+i); + src_ptr += CQSPI_FIFO_WIDTH/sizeof(dest_addr); + wr_bytes -= CQSPI_FIFO_WIDTH; + } + if (wr_bytes) { + /* dangling bytes */ + i = wr_bytes/sizeof(dest_addr); + for (i = wr_bytes/sizeof(dest_addr); i >= 0; i--) + writel(*(src_ptr+i), dest_addr+i); + } }
return 0;
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- drivers/spi/cadence_qspi_apb.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 403230a..e1e1315 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -471,6 +471,10 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat) /* Configure the remap address register, no remap */ writel(0, plat->regbase + CQSPI_REG_REMAP);
+ /* Setup the indirect trigger address */ + writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK), + plat->regbase + CQSPI_REG_INDIRECTTRIGGER); + /* Disable all interrupts */ writel(0, plat->regbase + CQSPI_REG_IRQMASK);
@@ -629,10 +633,6 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, /* for normal read (only ramtron as of now) */ addr_bytes = cmdlen - 1;
- /* Setup the indirect trigger address */ - writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK), - plat->regbase + CQSPI_REG_INDIRECTTRIGGER); - /* Configure SRAM partition for read. */ writel(CQSPI_REG_SRAM_PARTITION_RD, plat->regbase + CQSPI_REG_SRAMPARTITION); @@ -731,9 +731,6 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat, cmdlen, (unsigned int)cmdbuf); return -EINVAL; } - /* Setup the indirect trigger address */ - writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK), - plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
writel(CQSPI_REG_SRAM_PARTITION_WR, plat->regbase + CQSPI_REG_SRAMPARTITION);
Indirect read/write start addresses are flash start addresses for indirect read or write transfers. These should be absolute flash addresses instead of offsets.
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- drivers/spi/cadence_qspi_apb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index e1e1315..7be67a9 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -647,7 +647,8 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
/* Get address */ addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR); + writel((u32)plat->ahbbase + addr_value, + plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
/* The remaining lenght is dummy bytes. */ dummy_bytes = cmdlen - addr_bytes - 1; @@ -741,7 +742,8 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
/* Setup write address. */ reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR); + writel((u32)plat->ahbbase + reg, + plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
reg = readl(plat->regbase + CQSPI_REG_SIZE); reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
This patch is to separate the base trigger from the read/write transfer start addresses.
Base trigger register address (0x1c register) corresponds to the address which should be put on AHB bus to handle indirect transfer triggered before. To handle indirect transfer we need to issue addresses from (value of 0x1c) to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM location). There are no obstacles in issuing const address just equal to 0x1c. Important thing to note is that indirect trigger address has nothing in common with your physical or mapped NOR Flash address.
Transfer read/write start addresses (offset 0x68/0x78)should be programmed with the absolute flash address to be read/written.
plat->ahbbase has been renamed to plat->flashbase for clarity. plat->triggerbase is added in device tree for mapped spi flash address.
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- arch/arm/dts/socfpga.dtsi | 1 + arch/arm/dts/stv0991.dts | 3 ++- drivers/spi/cadence_qspi.c | 13 +++++++------ drivers/spi/cadence_qspi.h | 5 +++-- drivers/spi/cadence_qspi_apb.c | 12 +++++------- 5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index bf791c5..d89c974 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -634,6 +634,7 @@ #size-cells = <0>; reg = <0xff705000 0x1000>, <0xffa00000 0x1000>; + <0x00000000 0x0010>; interrupts = <0 151 4>; clocks = <&qspi_clk>; ext-decoder = <0>; /* external decoder */ diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts index 3b1efca..72399ff 100644 --- a/arch/arm/dts/stv0991.dts +++ b/arch/arm/dts/stv0991.dts @@ -30,7 +30,8 @@ #address-cells = <1>; #size-cells = <0>; reg = <0x80203000 0x100>, - <0x40000000 0x1000000>; + <0x40000000 0x1000000>, + <0x40000000 0x0000010>; clocks = <3750000>; ext-decoder = <0>; /* external decoder */ num-cs = <4>; diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index a75fc46..b23461d 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus) struct cadence_spi_priv *priv = dev_get_priv(bus);
priv->regbase = plat->regbase; - priv->ahbbase = plat->ahbbase; + priv->flashbase = plat->flashbase;
if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat); @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) const void *blob = gd->fdt_blob; int node = bus->of_offset; int subnode; - u32 data[4]; + u32 data[6]; int ret;
/* 2 base addresses are needed, lets get them from the DT */ @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) }
plat->regbase = (void *)data[0]; - plat->ahbbase = (void *)data[2]; + plat->flashbase = (void *)data[2]; + plat->trigger_base = (void *)data[4];
/* Use 500KHz as a suitable default */ plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", @@ -310,9 +311,9 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20); plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
- debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n", - __func__, plat->regbase, plat->ahbbase, plat->max_hz, - plat->page_size); + debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d \ + page-size=%d\n", __func__, plat->regbase, plat->flashbase, + plat->trigger_base, plat->max_hz, plat->page_size);
return 0; } diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index c9a6142..5ea4581 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -17,7 +17,8 @@ struct cadence_spi_platdata { unsigned int max_hz; void *regbase; - void *ahbbase; + void *flashbase; + void *trigger_base;
u32 page_size; u32 block_size; @@ -29,7 +30,7 @@ struct cadence_spi_platdata {
struct cadence_spi_priv { void *regbase; - void *ahbbase; + void *flashbase; size_t cmd_len; u8 cmd_buf[32]; size_t data_len; diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 7be67a9..2994158 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -50,7 +50,6 @@ #define CQSPI_INST_TYPE_QUAD (2)
#define CQSPI_STIG_DATA_LEN_MAX (8) -#define CQSPI_INDIRECTTRIGGER_ADDR_MASK (0xFFFFF)
#define CQSPI_DUMMY_CLKS_PER_BYTE (8) #define CQSPI_DUMMY_BYTES_MAX (4) @@ -471,9 +470,8 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat) /* Configure the remap address register, no remap */ writel(0, plat->regbase + CQSPI_REG_REMAP);
- /* Setup the indirect trigger address */ - writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK), - plat->regbase + CQSPI_REG_INDIRECTTRIGGER); + writel((u32)plat->trigger_base, + plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
/* Disable all interrupts */ writel(0, plat->regbase + CQSPI_REG_IRQMASK); @@ -647,7 +645,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
/* Get address */ addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel((u32)plat->ahbbase + addr_value, + writel((u32)plat->flashbase + addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
/* The remaining lenght is dummy bytes. */ @@ -696,7 +694,7 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat, udelay(1);
if (cadence_qspi_apb_read_fifo_data((void *)rxbuf, - (const void *)plat->ahbbase, rxlen)) + (const void *)plat->trigger_base, rxlen)) goto failrd;
/* Check flash indirect controller */ @@ -742,7 +740,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
/* Setup write address. */ reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); - writel((u32)plat->ahbbase + reg, + writel((u32)plat->flashbase + reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
reg = readl(plat->regbase + CQSPI_REG_SIZE);
Fifo width could be different on different socs, e.g. stv0991 & altera soc have different fifo width.
Signed-off-by: Vikas Manocha vikas.manocha@st.com --- arch/arm/dts/socfpga.dtsi | 1 + arch/arm/dts/stv0991.dts | 1 + drivers/spi/cadence_qspi.c | 1 + drivers/spi/cadence_qspi.h | 1 + drivers/spi/cadence_qspi_apb.c | 13 ++++--------- 5 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index d89c974..a2a2029 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -640,6 +640,7 @@ ext-decoder = <0>; /* external decoder */ num-cs = <4>; fifo-depth = <128>; + fifo-width = <4>; bus-num = <2>; status = "disabled"; }; diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts index 72399ff..433fcd1 100644 --- a/arch/arm/dts/stv0991.dts +++ b/arch/arm/dts/stv0991.dts @@ -36,6 +36,7 @@ ext-decoder = <0>; /* external decoder */ num-cs = <4>; fifo-depth = <256>; + fifo-width = <8>; bus-num = <0>; status = "okay";
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index b23461d..dfdd9e3 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -310,6 +310,7 @@ 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->fifo_width = fdtdec_get_int(blob, node, "fifo-width", 4);
debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d \ page-size=%d\n", __func__, plat->regbase, plat->flashbase, diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 5ea4581..2be1b43 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -26,6 +26,7 @@ struct cadence_spi_platdata { u32 tsd2d_ns; u32 tchsh_ns; u32 tslch_ns; + u32 fifo_width; };
struct cadence_spi_priv { diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 2994158..61fc95a 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -34,8 +34,6 @@ #define CQSPI_REG_RETRY (10000) #define CQSPI_POLL_IDLE_RETRY (3)
-#define CQSPI_FIFO_WIDTH (4) - /* Controller sram size in word */ #define CQSPI_REG_SRAM_SIZE_WORD (128) #define CQSPI_REG_SRAM_RESV_WORDS (2) @@ -54,9 +52,6 @@ #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) ****************************************************************************/ @@ -235,11 +230,11 @@ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat, page_size : remaining;
remaining -= wr_bytes; - while (wr_bytes >= CQSPI_FIFO_WIDTH) { - for (i = 0; i < CQSPI_FIFO_WIDTH/sizeof(dest_addr); i++) + while (wr_bytes >= plat->fifo_width) { + for (i = 0; i < plat->fifo_width/sizeof(dest_addr); i++) writel(*(src_ptr+i), dest_addr+i); - src_ptr += CQSPI_FIFO_WIDTH/sizeof(dest_addr); - wr_bytes -= CQSPI_FIFO_WIDTH; + src_ptr += plat->fifo_width/sizeof(dest_addr); + wr_bytes -= plat->fifo_width; } if (wr_bytes) { /* dangling bytes */
Hi Vikas,
I finally got to testing your latest patchset. And have some comments / problems:
On 17.06.2015 04:14, Vikas Manocha wrote:
This patchset:
- removes sram polling while reading/writing from flash.
- fixes trigger base & transfer start address register programming. This fix
superseeds the previous patch "spi: cadence_qspi: Fix the indirect ahb trigger address setting"
- adds support to get fifo width from device tree
Vikas Manocha (7): spi: cadence_qspi: remove sram polling from flash read spi: cadence_qspi: read can be independent of fifo width spi: cadence_qspi: remove sram polling from flash write spi: cadence_qspi: move trigger base configuration in init spi: cadence_qspi: fix indirect read/write start address spi: cadence_qspi: fix base trigger address & transfer start address spi: cadence_qspi: get fifo width from device tree
arch/arm/dts/socfpga.dtsi | 2 + arch/arm/dts/stv0991.dts | 4 +- drivers/spi/cadence_qspi.c | 14 ++-- drivers/spi/cadence_qspi.h | 6 +- drivers/spi/cadence_qspi_apb.c | 140 ++++++++++------------------------------ 5 files changed, 50 insertions(+), 116 deletions(-)
With these patches applied, I see this compilation error:
$ make -s -j10 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_arria5_socdk.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[2]: *** [arch/arm/dts/socfpga_cyclone5_socdk.dtb] Error 1 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_cyclone5_socrates.dtb] Error 1
The socfpga.dtsi has incorrect syntax. Here a quick fix for this - please add this to your next version. And please also compile-test for e.g. socrates.
$ gd diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index a2a2029..448870e 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -633,8 +633,8 @@ #address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>, - <0xffa00000 0x1000>; - <0x00000000 0x0010>; + <0xffa00000 0x1000>, + <0x00000000 0x0010>; interrupts = <0 151 4>; clocks = <&qspi_clk>; ext-decoder = <0>; /* external decoder */
Okay. After installing the resulting image on the SoCrates, I get the following error while reading from SD-card:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf read 100000 0 100000 QSPI: indirect completion status error with reg 0x0000000c SF: 1048576 bytes @ 0x0 Read: ERROR
So there seems to be something breaking the SoCFPGA Cadence QSPI support. Any idea whats going wrong here?
Thanks, Stefan
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Thursday, June 18, 2015 5:02 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
I finally got to testing your latest patchset. And have some comments / problems:
On 17.06.2015 04:14, Vikas Manocha wrote:
This patchset:
- removes sram polling while reading/writing from flash.
- fixes trigger base & transfer start address register programming.
This fix superseeds the previous patch "spi: cadence_qspi: Fix the indirect ahb trigger address setting"
- adds support to get fifo width from device tree
Vikas Manocha (7): spi: cadence_qspi: remove sram polling from flash read spi: cadence_qspi: read can be independent of fifo width spi: cadence_qspi: remove sram polling from flash write spi: cadence_qspi: move trigger base configuration in init spi: cadence_qspi: fix indirect read/write start address spi: cadence_qspi: fix base trigger address & transfer start address spi: cadence_qspi: get fifo width from device tree
arch/arm/dts/socfpga.dtsi | 2 + arch/arm/dts/stv0991.dts | 4 +- drivers/spi/cadence_qspi.c | 14 ++-- drivers/spi/cadence_qspi.h | 6 +- drivers/spi/cadence_qspi_apb.c | 140 ++++++++++---------------------------
5 files changed, 50 insertions(+), 116 deletions(-)
With these patches applied, I see this compilation error:
$ make -s -j10 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_arria5_socdk.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[2]: *** [arch/arm/dts/socfpga_cyclone5_socdk.dtb] Error 1 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_cyclone5_socrates.dtb] Error 1
The socfpga.dtsi has incorrect syntax. Here a quick fix for this - please add this to your next version. And please also compile-test for e.g. socrates.
You are right, semicolon has to be replaced with comma. I will fix it in next version & do the compile-test also.
$ gd diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index a2a2029..448870e 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -633,8 +633,8 @@ #address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>,
<0xffa00000 0x1000>;<0x00000000 0x0010>;
<0xffa00000 0x1000>,<0x00000000 0x0010>; interrupts = <0 151 4>; clocks = <&qspi_clk>; ext-decoder = <0>; /* external decoder */Okay. After installing the resulting image on the SoCrates, I get the following error while reading from SD-card:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf read 100000 0 100000 QSPI: indirect completion status error with reg 0x0000000c SF: 1048576 bytes @ 0x0 Read: ERROR
So there seems to be something breaking the SoCFPGA Cadence QSPI support. Any idea whats going wrong here?
It means indirect read was not successful. Can you please:
- please check if "sf write" is also causing some error or is working fine. - git bisect or cherry-pick to find out which patch is breaking the read functionality.
Rgds, Vikas
Thanks, Stefan
Hi Vikas,
On 18.06.2015 20:05, Vikas MANOCHA wrote:
<snip>
$ make -s -j10 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_arria5_socdk.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[2]: *** [arch/arm/dts/socfpga_cyclone5_socdk.dtb] Error 1 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_cyclone5_socrates.dtb] Error 1
The socfpga.dtsi has incorrect syntax. Here a quick fix for this - please add this to your next version. And please also compile-test for e.g. socrates.
You are right, semicolon has to be replaced with comma. I will fix it in next version & do the compile-test also.
And please also take care of the correct indentation.
$ gd diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index a2a2029..448870e 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -633,8 +633,8 @@ #address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>,
<0xffa00000 0x1000>;<0x00000000 0x0010>;
<0xffa00000 0x1000>,<0x00000000 0x0010>; interrupts = <0 151 4>; clocks = <&qspi_clk>; ext-decoder = <0>; /* external decoder */Okay. After installing the resulting image on the SoCrates, I get the following error while reading from SD-card:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf read 100000 0 100000 QSPI: indirect completion status error with reg 0x0000000c SF: 1048576 bytes @ 0x0 Read: ERROR
So there seems to be something breaking the SoCFPGA Cadence QSPI support. Any idea whats going wrong here?
It means indirect read was not successful. Can you please:
- please check if "sf write" is also causing some error or is working fine.
Same error.
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Here the output from the complete patchset with DEBUG enabled:
=> sf probe cadence_spi_ofdata_to_platdata: regbase=ff705000 flashbase=ffa00000 trigger_base=00000000 max-frequency=500000 page-size=256 cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 500000Hz Div 0xf SF: Read data capture delay calibrated to 7 (0 - 15) cadence_spi_set_speed: speed=1000000 cadence_spi_xfer: len=1 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 cadence_spi_xfer: len=5 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_spi_set_speed: speed=1000000 => sf read 200000 100000 10000 cadence_spi_xfer: len=5 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 cadence_spi_xfer: len=65536 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 QSPI: indirect completion status error with reg 0x0000000c SF: 65536 bytes @ 0x100000 Read: ERROR
HTP.
Thanks, Stefan
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Thursday, June 18, 2015 11:16 PM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 18.06.2015 20:05, Vikas MANOCHA wrote:
<snip>
$ make -s -j10 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_arria5_socdk.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[2]: *** [arch/arm/dts/socfpga_cyclone5_socdk.dtb] Error 1 Error: arch/arm/dts/socfpga.dtsi:637.5-6 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [arch/arm/dts/socfpga_cyclone5_socrates.dtb] Error 1
The socfpga.dtsi has incorrect syntax. Here a quick fix for this - please add this to your next version. And please also compile-test for e.g.
socrates.
You are right, semicolon has to be replaced with comma. I will fix it in next version & do the compile-test also.
And please also take care of the correct indentation.
$ gd diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index a2a2029..448870e 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -633,8 +633,8 @@ #address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>,
<0xffa00000 0x1000>;<0x00000000 0x0010>;
<0xffa00000 0x1000>,<0x00000000 0x0010>; interrupts = <0 151 4>; clocks = <&qspi_clk>; ext-decoder = <0>; /* external decoder */Okay. After installing the resulting image on the SoCrates, I get the following error while reading from SD-card:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf read 100000 0 100000 QSPI: indirect completion status error with reg 0x0000000c SF: 1048576 bytes @ 0x0 Read: ERROR
So there seems to be something breaking the SoCFPGA Cadence QSPI support. Any idea whats going wrong here?
It means indirect read was not successful. Can you please:
- please check if "sf write" is also causing some error or is working fine.
Same error.
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
Rgds, Vikas
Here the output from the complete patchset with DEBUG enabled:
=> sf probe cadence_spi_ofdata_to_platdata: regbase=ff705000 flashbase=ffa00000 trigger_base=00000000 max-frequency=500000 page-size=256 cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 500000Hz Div 0xf SF: Read data capture delay calibrated to 7 (0 - 15) cadence_spi_set_speed: speed=1000000 cadence_spi_xfer: len=1 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 cadence_spi_xfer: len=5 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR cadence_qspi_apb_config_baudrate_div: ref_clk 400000000Hz sclk 1000000Hz Div 0xf cadence_spi_set_speed: speed=1000000 => sf read 200000 100000 10000 cadence_spi_xfer: len=5 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 cadence_spi_xfer: len=65536 [bytes] cadence_qspi_apb_chipselect : chipselect 0 decode 0 QSPI: indirect completion status error with reg 0x0000000c SF: 65536 bytes @ 0x100000 Read: ERROR
HTP.
Thanks, Stefan
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function 'qpsi_write_sram_fifo_push': drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct cadence_spi_platdata' has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again. So there seems to be something wrong with the previous patches as well. here the output with the patches reverted:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 35.872s, speed 29929 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 Total of 1048576 byte(s) were the same
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Thanks, Stefan
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, June 22, 2015 1:35 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function 'qpsi_write_sram_fifo_push': drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct cadence_spi_platdata' has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
Sorry for this dumb mistake which happened during rebase :-(, I will correct in next patch series.
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again.
Thanks again Stefan for these tests. It is not easy for me to figure out this instability without the board but I don 't see any logical error in the patches. Off-course code is more optimized & fast with these patches. Access timing in the log provided by you also confirms it.
I can suggest following checks on the hardware:
- Figure out the issue is with read or write, which means comparing the flash with ram after read/write. - Put some random microsecond delays in the read/write to confirm it is timing issue.
So there seems to be something wrong with the previous patches as well. here the output with the patches reverted:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 35.872s, speed 29929 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 Total of 1048576 byte(s) were the same
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be 0xFFA0_0000. Normally it should work, Graham also thinks the same, Let's wait for his discussion with the Altera designers.
Rgds, Vikas
Thanks, Stefan
On 06/22/2015 06:31 PM, Vikas MANOCHA wrote: ...
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be 0xFFA0_0000. Normally it should work, Graham also thinks the same, Let's wait for his discussion with the Altera designers.
Wait a minute, on SoCFPGA, the flashbase is 0xffa00000, and the trigger base is 0x00000000. The point of having a different address was that they needed to be different on SoCFPGA, right?
As for why they're different, the Altera Cyclone5 SoCFPGA has a complex multilevel interconnect where the QSPI is three levels down, and is the only slave of an AHB master. At that level of the interconnect, the base address has long been stripped off, it was used to select down to the final master. The QSPI is the only thing on that AHB bus, so its address is zero. Or at least that's how I understand it.
-Graham
Hi Graham,
-----Original Message----- From: Graham Moore [mailto:grmoore@opensource.altera.com] Sent: Tuesday, June 23, 2015 7:37 AM To: Vikas MANOCHA Cc: Stefan Roese; u-boot@lists.denx.de; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
On 06/22/2015 06:31 PM, Vikas MANOCHA wrote: ...
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be
0xFFA0_0000.
Normally it should work, Graham also thinks the same, Let's wait for his
discussion with the Altera designers.
Wait a minute, on SoCFPGA, the flashbase is 0xffa00000, and the trigger base is 0x00000000. The point of having a different address was that they needed to be different on SoCFPGA, right?
As for why they're different, the Altera Cyclone5 SoCFPGA has a complex multilevel interconnect where the QSPI is three levels down, and is the only slave of an AHB master. At that level of the interconnect, the base address has long been stripped off, it was used to select down to the final master. The QSPI is the only thing on that AHB bus, so its address is zero. Or at least that's how I understand it.
Hmm ok, let us keep trigger base as 0x0 for SofFPGA cyclone5.
Rgds, Vikas
-Graham
Hi Graham,
-----Original Message----- From: Graham Moore [mailto:grmoore@opensource.altera.com] Sent: Tuesday, June 23, 2015 7:37 AM To: Vikas MANOCHA Cc: Stefan Roese; u-boot@lists.denx.de; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
On 06/22/2015 06:31 PM, Vikas MANOCHA wrote: ...
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be
0xFFA0_0000.
Normally it should work, Graham also thinks the same, Let's wait for his
discussion with the Altera designers.
Wait a minute, on SoCFPGA, the flashbase is 0xffa00000, and the trigger base is 0x00000000. The point of having a different address was that they needed to be different on SoCFPGA, right?
As for why they're different, the Altera Cyclone5 SoCFPGA has a complex multilevel interconnect where the QSPI is three levels down, and is the only slave of an AHB master. At that level of the interconnect, the base address has long been stripped off, it was used to select down to the final master. The QSPI is the only thing on that AHB bus, so its address is zero. Or at least that's how I understand it.
If we check the code for reading/writing to the FIFO/SRAM of the controller, complete base address is being used. Which means CPU is using address 0xFFA0_0000, so does not seems like base address is stripped off. It is only for the trigger address which needs programming to 0x0 ?
Regardless, it is something specific to socfpga architecture & should not be part of cadence qspi driver.
Rgds, Vikas
-Graham
On 07/02/2015 12:50 PM, Vikas MANOCHA wrote:
Hi Graham,
-----Original Message----- From: Graham Moore [mailto:grmoore@opensource.altera.com] Sent: Tuesday, June 23, 2015 7:37 AM To: Vikas MANOCHA Cc: Stefan Roese; u-boot@lists.denx.de; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
On 06/22/2015 06:31 PM, Vikas MANOCHA wrote: ...
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be
0xFFA0_0000.
Normally it should work, Graham also thinks the same, Let's wait for his
discussion with the Altera designers.
Wait a minute, on SoCFPGA, the flashbase is 0xffa00000, and the trigger base is 0x00000000. The point of having a different address was that they needed to be different on SoCFPGA, right?
As for why they're different, the Altera Cyclone5 SoCFPGA has a complex multilevel interconnect where the QSPI is three levels down, and is the only slave of an AHB master. At that level of the interconnect, the base address has long been stripped off, it was used to select down to the final master. The QSPI is the only thing on that AHB bus, so its address is zero. Or at least that's how I understand it.
If we check the code for reading/writing to the FIFO/SRAM of the controller, complete base address is being used. Which means CPU is using address 0xFFA0_0000, so does not seems like base address is stripped off. It is only for the trigger address which needs programming to 0x0 ?
Regardless, it is something specific to socfpga architecture & should not be part of cadence qspi driver.
I'm sorry, I don't understand your comment. What is 'it' that should not be part of the cadence qspi driver?
-Graham
Rgds, Vikas
-Graham
Hi Graham,
-----Original Message----- From: Graham Moore [mailto:grmoore@opensource.altera.com] Sent: Monday, July 06, 2015 10:57 AM To: Vikas MANOCHA Cc: Stefan Roese; u-boot@lists.denx.de; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
On 07/02/2015 12:50 PM, Vikas MANOCHA wrote:
Hi Graham,
-----Original Message----- From: Graham Moore [mailto:grmoore@opensource.altera.com] Sent: Tuesday, June 23, 2015 7:37 AM To: Vikas MANOCHA Cc: Stefan Roese; u-boot@lists.denx.de; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
On 06/22/2015 06:31 PM, Vikas MANOCHA wrote: ...
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving
same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be
0xFFA0_0000.
Normally it should work, Graham also thinks the same, Let's wait for his
discussion with the Altera designers.
Wait a minute, on SoCFPGA, the flashbase is 0xffa00000, and the trigger base is 0x00000000. The point of having a different address was that they needed to be different on SoCFPGA, right?
As for why they're different, the Altera Cyclone5 SoCFPGA has a complex multilevel interconnect where the QSPI is three levels down, and is the only slave of an AHB master. At that level of the interconnect, the base address has long been stripped off, it was used to
select down to the final master.
The QSPI is the only thing on that AHB bus, so its address is zero. Or at least that's how I understand it.
If we check the code for reading/writing to the FIFO/SRAM of the
controller, complete base address is being used.
Which means CPU is using address 0xFFA0_0000, so does not seems like
base address is stripped off. It is only for the trigger address which needs programming to 0x0 ?
Regardless, it is something specific to socfpga architecture & should not be
part of cadence qspi driver.
I'm sorry, I don't understand your comment. What is 'it' that should not be part of the cadence qspi driver?
I meant : Trigger base address programming to one address (which is 0x0 in case of socfpga) & then reading/writing from another locations (NOR location 0xFFA0_0000 in case of socfpga) : It shouldn't be like this in the driver as per the definition of trigger base address.
Trigger base address is the location which should be used to read/write in indirect mode. It actually makes it like reading sram & hence called indirect mode.
It is what was explained & implemented in attached patch. Stefan tested it & it didn't work on socfpga platform, there seems something specific to arch, to be debugged.
Rgds, Vikas
-Graham
Rgds, Vikas
-Graham
Hi Stefan,
-----Original Message----- From: Vikas MANOCHA Sent: Monday, June 22, 2015 4:31 PM To: 'Stefan Roese' Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, June 22, 2015 1:35 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function 'qpsi_write_sram_fifo_push': drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct
cadence_spi_platdata'
has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
Sorry for this dumb mistake which happened during rebase :-(, I will correct in next patch series.
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again.
Thanks again Stefan for these tests. It is not easy for me to figure out this instability without the board but I don 't see any logical error in the patches. Off-course code is more optimized & fast with these patches. Access timing in the log provided by you also confirms it.
I can suggest following checks on the hardware:
- Figure out the issue is with read or write, which means comparing the flash
with ram after read/write.
- Put some random microsecond delays in the read/write to confirm it is
timing issue.
Any update on it ?
Rgds, Vikas
So there seems to be something wrong with the previous patches as well. here the output with the patches reverted:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 35.872s, speed 29929 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 Total of 1048576 byte(s) were the same
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be 0xFFA0_0000. Normally it should work, Graham also thinks the same, Let's wait for his discussion with the Altera designers.
Rgds, Vikas
Thanks, Stefan
Hi Stefan,
-----Original Message----- From: Vikas MANOCHA Sent: Wednesday, July 01, 2015 9:25 AM To: 'Stefan Roese' Cc: 'u-boot@lists.denx.de'; 'grmoore@opensource.altera.com'; 'dinguyen@opensource.altera.com'; 'jteki@openedev.com' Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Stefan,
-----Original Message----- From: Vikas MANOCHA Sent: Monday, June 22, 2015 4:31 PM To: 'Stefan Roese' Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, June 22, 2015 1:35 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
- git bisect or cherry-pick to find out which patch is breaking the read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function
'qpsi_write_sram_fifo_push':
drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct
cadence_spi_platdata'
has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
Sorry for this dumb mistake which happened during rebase :-(, I will correct in next patch series.
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again.
Thanks again Stefan for these tests. It is not easy for me to figure out this instability without the board but I don 't see any logical error in the
patches.
Off-course code is more optimized & fast with these patches. Access timing in the log provided by you also confirms it.
I can suggest following checks on the hardware:
- Figure out the issue is with read or write, which means comparing
the flash with ram after read/write.
- Put some random microsecond delays in the read/write to confirm it
is timing issue.
Any update on it ?
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
I am ready to check it but would need the hardware platform.
Rgds, Vikas
Rgds, Vikas
So there seems to be something wrong with the previous patches as
well.
here the output with the patches reverted:
=> sf probe SF: Detected N25Q256 with page size 256 Bytes, erase size 4 KiB, total 32 MiB SF: Warning - Only lower 16MiB accessible, Full access #define CONFIG_SPI_FLASH_BAR => sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 35.872s, speed 29929 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 Total of 1048576 byte(s) were the same
The point is if after applying above mentioned patch (...: fix indirect read/write start address), Read/write are working fine, then trigger_base value of 0xFFA00_0000 should also work fine. Can you please modify the trigger_base value from 0x0 to 0xFFA0_0000 in Socfpga.dtsi & check. If it works, it would mean both (socfpga & stv0991) are behaving same.
No. With this change, the "sf read" command crashes / hangs on the SoCFPGA board.
Ok, I don't know why socfpga is not working with trigger_base to be 0xFFA0_0000. Normally it should work, Graham also thinks the same, Let's wait for his discussion with the Altera designers.
Rgds, Vikas
Thanks, Stefan
Hi Vikas,
On 09.07.2015 03:29, Vikas MANOCHA wrote:
-----Original Message----- From: Vikas MANOCHA Sent: Wednesday, July 01, 2015 9:25 AM To: 'Stefan Roese' Cc: 'u-boot@lists.denx.de'; 'grmoore@opensource.altera.com'; 'dinguyen@opensource.altera.com'; 'jteki@openedev.com' Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Stefan,
-----Original Message----- From: Vikas MANOCHA Sent: Monday, June 22, 2015 4:31 PM To: 'Stefan Roese' Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, June 22, 2015 1:35 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
> - git bisect or cherry-pick to find out which patch is breaking the > read functionality.
This one is the first introducing this breakage:
spi: cadence_qspi: fix base trigger address & transfer start address
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function
'qpsi_write_sram_fifo_push':
drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct
cadence_spi_platdata'
has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
Sorry for this dumb mistake which happened during rebase :-(, I will correct in next patch series.
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000 100000 byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again.
Thanks again Stefan for these tests. It is not easy for me to figure out this instability without the board but I don 't see any logical error in the
patches.
Off-course code is more optimized & fast with these patches. Access timing in the log provided by you also confirms it.
I can suggest following checks on the hardware:
- Figure out the issue is with read or write, which means comparing
the flash with ram after read/write.
- Put some random microsecond delays in the read/write to confirm it
is timing issue.
Any update on it ?
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
I am ready to check it but would need the hardware platform.
Understood.
Thanks, Stefan
Hi Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, July 13, 2015 2:01 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 09.07.2015 03:29, Vikas MANOCHA wrote:
-----Original Message----- From: Vikas MANOCHA Sent: Wednesday, July 01, 2015 9:25 AM To: 'Stefan Roese' Cc: 'u-boot@lists.denx.de'; 'grmoore@opensource.altera.com'; 'dinguyen@opensource.altera.com'; 'jteki@openedev.com' Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Stefan,
-----Original Message----- From: Vikas MANOCHA Sent: Monday, June 22, 2015 4:31 PM To: 'Stefan Roese' Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: RE: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Thanks Stefan,
-----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Monday, June 22, 2015 1:35 AM To: Vikas MANOCHA Cc: u-boot@lists.denx.de; grmoore@opensource.altera.com; dinguyen@opensource.altera.com; jteki@openedev.com Subject: Re: [PATCH RESEND 0/7] spi: cadence_qspi: optimize & fix indirect rd-writes
Hi Vikas,
On 19.06.2015 23:38, Vikas MANOCHA wrote:
>> - git bisect or cherry-pick to find out which patch is breaking the >> read functionality. > > This one is the first introducing this breakage: > > spi: cadence_qspi: fix base trigger address & transfer start > address >
Ok, can you confirm applying patches upto (& including) "spi: cadence_qspi: fix indirect read/write start address" , read/write to flash are working fine.
Please note that with this patch the code does not compile:
drivers/spi/cadence_qspi_apb.c: In function
'qpsi_write_sram_fifo_push':
drivers/spi/cadence_qspi_apb.c:227:32: error: 'struct
cadence_spi_platdata'
has no member named 'trigger_base' unsigned int *dest_addr = plat->trigger_base;
I've manually fixed this trivial change (trigger_base -> ahbbase) and tests with these patches applied show some problems with "sf" stability (bit-flips):
Sorry for this dumb mistake which happened during rebase :-(, I will correct in next patch series.
=> sf update 400000 100000 100000 1048576 bytes written, 0 bytes skipped in 34.196s, speed 31395 B/s => sf read 500000 100000 100000 SF: 1048576 bytes @ 0x100000 Read: OK => cmp.b 400000 500000
100000
byte at 0x0040001e (0x9f) != byte at 0x0050001e (0x8f) Total of 30 byte(s) were the same
This is new - removing all your patches seems to solve this issue again.
Thanks again Stefan for these tests. It is not easy for me to figure out this instability without the board but I don 't see any logical error in the
patches.
Off-course code is more optimized & fast with these patches. Access timing in the log provided by you also confirms it.
I can suggest following checks on the hardware:
- Figure out the issue is with read or write, which means comparing
the flash with ram after read/write.
- Put some random microsecond delays in the read/write to confirm it
is timing issue.
Any update on it ?
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Rgds, Vikas
I am ready to check it but would need the hardware platform.
Understood.
Thanks, Stefan
Hi Vikas,
On 15.07.2015 23:14, Vikas MANOCHA wrote:
<snip>
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
Thanks, Stefan
Hi Vikas,
On 16.07.2015 08:46, Stefan Roese wrote:
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
I couldn't find the required time to test these patches again yet. Sorry. And I'll leave for a short vacation tomorrow. So I won't be able to get to this issue before week 32.
Perhaps somebody else finds some time to look at this QSPI driver issue in the meantime
Thanks, Stefan
Hi Stefan,
On 07/23/2015 05:22 AM, Stefan Roese wrote:
Hi Vikas,
On 16.07.2015 08:46, Stefan Roese wrote:
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
I couldn't find the required time to test these patches again yet. Sorry. And I'll leave for a short vacation tomorrow. So I won't be able to get to this issue before week 32.
Perhaps somebody else finds some time to look at this QSPI driver issue in the meantime
Any update ?
Cheers, Vikas
Thanks, Stefan
.
Hi Vikas,
(added Marek to Cc)
On 11.08.2015 23:19, vikasm wrote:
On 07/23/2015 05:22 AM, Stefan Roese wrote:
Hi Vikas,
On 16.07.2015 08:46, Stefan Roese wrote:
In addition can you please check the patch causing this instability on socfpga. I don't like to bug you but to close this patchset, this info & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
I couldn't find the required time to test these patches again yet. Sorry. And I'll leave for a short vacation tomorrow. So I won't be able to get to this issue before week 32.
Perhaps somebody else finds some time to look at this QSPI driver issue in the meantime
Any update ?
Not from me, sorry. And I really won't be able to dig into this in the next weeks.
Again, perhaps someone else finds the time to look at this QSPI driver issue in the meantime?
Thanks, Stefan
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
Will get back again for my comments.
On 12 August 2015 at 17:06, Stefan Roese sr@denx.de wrote:
Hi Vikas,
(added Marek to Cc)
On 11.08.2015 23:19, vikasm wrote:
On 07/23/2015 05:22 AM, Stefan Roese wrote:
Hi Vikas,
On 16.07.2015 08:46, Stefan Roese wrote:
> > In addition can you please check the patch causing this instability > on > socfpga. I don't like to bug you but to close this patchset, this > info > & tests mentioned above seems to be required.
Okay. I'll try to find some time this week to do some testing here. It seems that the other cadence patchset from you ([v4 00/10] spi: cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled into mainline yet. To make it easier for me, could you perhaps publish a git repository that is based on current mainline. And has the mentioned above patch series included. And all the patches in the latest version that are currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
I couldn't find the required time to test these patches again yet. Sorry. And I'll leave for a short vacation tomorrow. So I won't be able to get to this issue before week 32.
Perhaps somebody else finds some time to look at this QSPI driver issue in the meantime
Any update ?
Not from me, sorry. And I really won't be able to dig into this in the next weeks.
Again, perhaps someone else finds the time to look at this QSPI driver issue in the meantime?
thanks!
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
Rgds, Vikas
Will get back again for my comments.
On 12 August 2015 at 17:06, Stefan Roese sr@denx.de wrote:
Hi Vikas,
(added Marek to Cc)
On 11.08.2015 23:19, vikasm wrote:
On 07/23/2015 05:22 AM, Stefan Roese wrote:
Hi Vikas,
On 16.07.2015 08:46, Stefan Roese wrote:
>> >> In addition can you please check the patch causing this instability >> on >> socfpga. I don't like to bug you but to close this patchset, this >> info >> & tests mentioned above seems to be required. > > > Okay. I'll try to find some time this week to do some testing here. > It seems > that the other cadence patchset from you ([v4 00/10] spi: > cadence_qspi: sram depth from DT & fix for FIFO width) is not pulled > into > mainline yet. To make it easier for me, could you perhaps publish a > git > repository that is based on current mainline. And has the mentioned > above > patch series included. And all the patches in the latest version that > are > currently causing these problems on SoCFPGA?
The patchset was in u-boot-spi repository, yesterday pulled by Tom in mainline. I will rebase the patchset in discussion (spi: cadence_qspi: optimize & fix indirect rd-writes) on mainline master & send the V2.
Let me know if it is ok.
Okay, I'll try to find some time later this week or next week for some tests.
I couldn't find the required time to test these patches again yet. Sorry. And I'll leave for a short vacation tomorrow. So I won't be able to get to this issue before week 32.
Perhaps somebody else finds some time to look at this QSPI driver issue in the meantime
Any update ?
Not from me, sorry. And I really won't be able to dig into this in the next weeks.
Again, perhaps someone else finds the time to look at this QSPI driver issue in the meantime?
thanks!
On Wednesday, August 12, 2015 at 07:52:28 PM, vikasm wrote:
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
I'll happily help, can you please rebase it on top of u-boot/master and repost? I can't seem to be able to apply it. Also, please keep me in Cc.
Thanks
Best regards, Marek Vasut
Hi Marek,
On 08/12/2015 01:22 PM, Marek Vasut wrote:
On Wednesday, August 12, 2015 at 07:52:28 PM, vikasm wrote:
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
I'll happily help, can you please rebase it on top of u-boot/master and repost? I can't seem to be able to apply it. Also, please keep me in Cc.
There is v2 of the patchset rebased on master, please use that.
Cheers, Vikas
Thanks
Best regards, Marek Vasut
On Thursday, August 13, 2015 at 02:16:21 AM, vikasm wrote:
Hi Marek,
On 08/12/2015 01:22 PM, Marek Vasut wrote:
On Wednesday, August 12, 2015 at 07:52:28 PM, vikasm wrote:
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
I'll happily help, can you please rebase it on top of u-boot/master and repost? I can't seem to be able to apply it. Also, please keep me in Cc.
There is v2 of the patchset rebased on master, please use that.
I only see this stuff in the mailing list ; did the patches get renamed or something ? Can you please provide patchwork links or at least some ML archive link , that'd really help.
Best regards, Marek Vasut
Hi Marek,
On 08/12/2015 05:26 PM, Marek Vasut wrote:
On Thursday, August 13, 2015 at 02:16:21 AM, vikasm wrote:
Hi Marek,
On 08/12/2015 01:22 PM, Marek Vasut wrote:
On Wednesday, August 12, 2015 at 07:52:28 PM, vikasm wrote:
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
I'll happily help, can you please rebase it on top of u-boot/master and repost? I can't seem to be able to apply it. Also, please keep me in Cc.
There is v2 of the patchset rebased on master, please use that.
I only see this stuff in the mailing list ; did the patches get renamed or something ? Can you please provide patchwork links or at least some ML archive link , that'd really help.
I just forwarded you one of the v2 patch & here is the patchwork link: https://patchwork.ozlabs.org/patch/496526/
Rgds, Vikas
Best regards, Marek Vasut .
On Thursday, August 13, 2015 at 02:36:27 AM, vikasm wrote:
Hi Marek,
Hi!
On 08/12/2015 05:26 PM, Marek Vasut wrote:
On Thursday, August 13, 2015 at 02:16:21 AM, vikasm wrote:
Hi Marek,
On 08/12/2015 01:22 PM, Marek Vasut wrote:
On Wednesday, August 12, 2015 at 07:52:28 PM, vikasm wrote:
Thanks Jagan,
On 08/12/2015 05:01 AM, Jagan Teki wrote:
Vikas,
Did you verified on board, can you just verified with 'sf update' before and after.. I just wanted to see if you get any performance improvement with these optimization fixes.
The main point is someone needs to debug the patchset on socfpga platform. It works on stv0991 platform. But which patch is really creating issue on socfpga & debug that patch.
I'll happily help, can you please rebase it on top of u-boot/master and repost? I can't seem to be able to apply it. Also, please keep me in Cc.
There is v2 of the patchset rebased on master, please use that.
I only see this stuff in the mailing list ; did the patches get renamed or something ? Can you please provide patchwork links or at least some ML archive link , that'd really help.
I just forwarded you one of the v2 patch & here is the patchwork link: https://patchwork.ozlabs.org/patch/496526/
I took a brief look and commented on the patches.
Best regards, Marek Vasut
participants (7)
-
Graham Moore -
Jagan Teki -
Marek Vasut -
Stefan Roese -
Vikas MANOCHA -
Vikas Manocha
-
vikasm