Message ID | 1437013654-29387-4-git-send-email-vikas.manocha@st.com |
---|---|
State | Superseded |
Delegated to: | Jagannadha Sutradharudu Teki |
Headers | show |
On Thursday, July 16, 2015 at 04:27:31 AM, Vikas Manocha wrote: > 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. Please see the question I posed in 2/6 v2 . > Signed-off-by: Vikas Manocha <vikas.manocha@st.com> > --- > > Changes in v2: Rebased to master > > drivers/spi/cadence_qspi_apb.c | 61 > ++++++++++------------------------------ 1 file changed, 15 insertions(+), > 46 deletions(-) > > diff --git a/drivers/spi/cadence_qspi_apb.c > b/drivers/spi/cadence_qspi_apb.c index 487bbef..ad8b79a 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -214,67 +214,36 @@ 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 = 0; > - int i; > - int remaining = bytes; > - unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr; > - unsigned int *src_ptr = (unsigned int *)src; > - > - while (remaining >= CQSPI_FIFO_WIDTH) { > - for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--) > - writel(*(src_ptr+i), dest_ptr+i); > - src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr); > - remaining -= CQSPI_FIFO_WIDTH; > - } > - if (remaining) { > - /* dangling bytes */ > - i = remaining/sizeof(dest_ptr); > - memcpy(&temp, src_ptr+i, remaining % sizeof(dest_ptr)); > - writel(temp, dest_ptr+i); > - for (--i; i >= 0; i--) > - writel(*(src_ptr+i), dest_ptr+i); > - } > - 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->ahbbase; > 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); Why don't you use memcpy instead , didn't you say you're copying data to/from SRAM? Is src_ptr value always aligned ? > + 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; > }
Hi Marek, On 08/12/2015 07:11 PM, Marek Vasut wrote: > On Thursday, July 16, 2015 at 04:27:31 AM, Vikas Manocha wrote: >> 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. > > Please see the question I posed in 2/6 v2 . replied in 2/6 v2. > >> Signed-off-by: Vikas Manocha <vikas.manocha@st.com> >> --- >> >> Changes in v2: Rebased to master >> >> drivers/spi/cadence_qspi_apb.c | 61 >> ++++++++++------------------------------ 1 file changed, 15 insertions(+), >> 46 deletions(-) >> >> diff --git a/drivers/spi/cadence_qspi_apb.c >> b/drivers/spi/cadence_qspi_apb.c index 487bbef..ad8b79a 100644 >> --- a/drivers/spi/cadence_qspi_apb.c >> +++ b/drivers/spi/cadence_qspi_apb.c >> @@ -214,67 +214,36 @@ 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 = 0; >> - int i; >> - int remaining = bytes; >> - unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr; >> - unsigned int *src_ptr = (unsigned int *)src; >> - >> - while (remaining >= CQSPI_FIFO_WIDTH) { >> - for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--) >> - writel(*(src_ptr+i), dest_ptr+i); >> - src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr); >> - remaining -= CQSPI_FIFO_WIDTH; >> - } >> - if (remaining) { >> - /* dangling bytes */ >> - i = remaining/sizeof(dest_ptr); >> - memcpy(&temp, src_ptr+i, remaining % sizeof(dest_ptr)); >> - writel(temp, dest_ptr+i); >> - for (--i; i >= 0; i--) >> - writel(*(src_ptr+i), dest_ptr+i); >> - } >> - 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->ahbbase; >> 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); > > Why don't you use memcpy instead , didn't you say you're copying data to/from > SRAM? Is src_ptr value always aligned ? i think you are right, i will check it & fix in the next version. Rgds, Vikas > >> + 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; >> } > . >
On Thursday, August 13, 2015 at 06:30:37 PM, vikasm wrote: > Hi Marek, > > On 08/12/2015 07:11 PM, Marek Vasut wrote: > > On Thursday, July 16, 2015 at 04:27:31 AM, Vikas Manocha wrote: > >> 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. > > > > Please see the question I posed in 2/6 v2 . > > replied in 2/6 v2. [...] > >> /* 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->ahbbase; > >> > >> 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); > > > > Why don't you use memcpy instead , didn't you say you're copying data > > to/from SRAM? Is src_ptr value always aligned ? > > i think you are right, i will check it & fix in the next version. But is memcpy() really correct ? It does byte access and doesn't enforce ordering in any way.
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index 487bbef..ad8b79a 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -214,67 +214,36 @@ 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 = 0; - int i; - int remaining = bytes; - unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr; - unsigned int *src_ptr = (unsigned int *)src; - - while (remaining >= CQSPI_FIFO_WIDTH) { - for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--) - writel(*(src_ptr+i), dest_ptr+i); - src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr); - remaining -= CQSPI_FIFO_WIDTH; - } - if (remaining) { - /* dangling bytes */ - i = remaining/sizeof(dest_ptr); - memcpy(&temp, src_ptr+i, remaining % sizeof(dest_ptr)); - writel(temp, dest_ptr+i); - for (--i; i >= 0; i--) - writel(*(src_ptr+i), dest_ptr+i); - } - 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->ahbbase; 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; }
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> --- Changes in v2: Rebased to master drivers/spi/cadence_qspi_apb.c | 61 ++++++++++------------------------------ 1 file changed, 15 insertions(+), 46 deletions(-)