Hi Chin Liang, On 28 November 2016 12:49 See, Chin Liang wrote: > On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote: > > > > The Cadence QSPI controller has specified overheads for the various > > CS > > times that are in addition to those programmed in to the Device Delay > > register. The overheads are different for the delays. > > > > In addition, the existing code does not handle the case when the > > delay > > is less than a SCLK period. > > > > This change accurately calculates the additional delays in Ref > > clocks. > > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com> > > --- > > v2: > > Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation" > > Note only did the existing code not cope with the delay less than > > an SCLK period, but the calculation didn't round properly, and > > didn't take into account the delays that the QSPI Controller adds > > to those programmed into the Device Delay reg. > > --- > > drivers/spi/cadence_qspi_apb.c | 23 ++++++++++++----------- > > 1 file changed, 12 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/spi/cadence_qspi_apb.c > > b/drivers/spi/cadence_qspi_apb.c > > index 1cd636a..56ad952 100644 > > --- a/drivers/spi/cadence_qspi_apb.c > > +++ b/drivers/spi/cadence_qspi_apb.c > > @@ -169,9 +169,6 @@ > > ((readl(base + CQSPI_REG_CONFIG) >> \ > > CQSPI_REG_CONFIG_IDLE_LSB) & 0x1) > > > > -#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns) \ > > - ((((tdelay_ns) - (tsclk_ns)) / (tref_ns))) > > - > > #define CQSPI_GET_RD_SRAM_LEVEL(reg_base) \ > > (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >> \ > > CQSPI_REG_SDRAMLEVEL_RD_LSB) & > CQSPI_REG_SDRAMLEVEL_RD_MASK) > > @@ -357,16 +354,20 @@ void cadence_qspi_apb_delay(void *reg_base, > > cadence_qspi_apb_controller_disable(reg_base); > > > > /* Convert to ns. */ > > - ref_clk_ns = (1000000000) / ref_clk; > > + ref_clk_ns = DIV_ROUND_UP(1000000000, ref_clk); > > > > /* Convert to ns. */ > > - sclk_ns = (1000000000) / sclk_hz; > > - > > - /* Plus 1 to round up 1 clock cycle. */ > > - tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1; > > - tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1; > > - tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1; > > - tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1; > > + sclk_ns = DIV_ROUND_UP(1000000000, sclk_hz); > > + > > + /* The controller adds additional delay to that programmed in > > the reg */ > > + if (tshsl_ns >= sclk_ns + ref_clk_ns) > > + tshsl_ns -= sclk_ns + ref_clk_ns; > > + if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns) > > + tchsh_ns -= sclk_ns + 3 * ref_clk_ns; > Any reason we need this? > The DIV_ROUND_UP or previous + 1 in algo will ensure its more than a > SCLK period. In general, all of these CS timing should be optimal. I measured differences in throughput with the sub-optimal setting. Admittedly, the difference is small but still we should set it correctly.
> Thanks > Chin Liang > > > > > + tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns); > > + tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns); > > + tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns); > > + tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns); > > > > reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK) > > << CQSPI_REG_DELAY_TSHSL_LSB); > > -- > > 2.7.4 > > Thanks Phil _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
