[U-Boot] [PATCH v2 7/8] spi: cadence_qspi: Fix CS timings

See, Chin Liang chin.liang.see at intel.com
Mon Nov 28 13:48:39 CET 2016


Hi Phil,

On Jum, 2016-11-25 at 14:38 +0000, Phil Edworthy wrote:
> 
> The Cadence QSPI controller has specified overheads for the various
> CS
> times that are in addition to those programmed in to the Device Delay
> register. The overheads are different for the delays.
> 
> In addition, the existing code does not handle the case when the
> delay
> is less than a SCLK period.
> 
> This change accurately calculates the additional delays in Ref
> clocks.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com>
> ---
> v2:
>  Was "spi: cadence_qspi: Fix CQSPI_CAL_DELAY calculation"
>  Note only did the existing code not cope with the delay less than
>  an SCLK period, but the calculation didn't round properly, and
>  didn't take into account the delays that the QSPI Controller adds
>  to those programmed into the Device Delay reg.
> ---
>  drivers/spi/cadence_qspi_apb.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c
> index 1cd636a..56ad952 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -169,9 +169,6 @@
>         ((readl(base + CQSPI_REG_CONFIG) >>             \
>                 CQSPI_REG_CONFIG_IDLE_LSB) & 0x1)
> 
> -#define CQSPI_CAL_DELAY(tdelay_ns, tref_ns, tsclk_ns)          \
> -       ((((tdelay_ns) - (tsclk_ns)) / (tref_ns)))
> -
>  #define CQSPI_GET_RD_SRAM_LEVEL(reg_base)                      \
>         (((readl(reg_base + CQSPI_REG_SDRAMLEVEL)) >>   \
>         CQSPI_REG_SDRAMLEVEL_RD_LSB) & CQSPI_REG_SDRAMLEVEL_RD_MASK)
> @@ -357,16 +354,20 @@ void cadence_qspi_apb_delay(void *reg_base,
>         cadence_qspi_apb_controller_disable(reg_base);
> 
>         /* Convert to ns. */
> -       ref_clk_ns = (1000000000) / ref_clk;
> +       ref_clk_ns = DIV_ROUND_UP(1000000000, ref_clk);
> 
>         /* Convert to ns. */
> -       sclk_ns = (1000000000) / sclk_hz;
> -
> -       /* Plus 1 to round up 1 clock cycle. */
> -       tshsl = CQSPI_CAL_DELAY(tshsl_ns, ref_clk_ns, sclk_ns) + 1;
> -       tchsh = CQSPI_CAL_DELAY(tchsh_ns, ref_clk_ns, sclk_ns) + 1;
> -       tslch = CQSPI_CAL_DELAY(tslch_ns, ref_clk_ns, sclk_ns) + 1;
> -       tsd2d = CQSPI_CAL_DELAY(tsd2d_ns, ref_clk_ns, sclk_ns) + 1;
> +       sclk_ns = DIV_ROUND_UP(1000000000, sclk_hz);
> +
> +       /* The controller adds additional delay to that programmed in
> the reg */
> +       if (tshsl_ns >= sclk_ns + ref_clk_ns)
> +               tshsl_ns -= sclk_ns + ref_clk_ns;
> +       if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
> +               tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
Any reason we need this?
The DIV_ROUND_UP or previous + 1 in algo will ensure its more than a
SCLK period.

Thanks
Chin Liang

> 
> +       tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
> +       tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
> +       tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
> +       tsd2d = DIV_ROUND_UP(tsd2d_ns, ref_clk_ns);
> 
>         reg = ((tshsl & CQSPI_REG_DELAY_TSHSL_MASK)
>                         << CQSPI_REG_DELAY_TSHSL_LSB);
> --
> 2.7.4
> 
> 
> ________________________________
> 
> Confidentiality Notice.
> This message may contain information that is confidential or
> otherwise protected from disclosure. If you are not the intended
> recipient, you are hereby notified that any use, disclosure,
> dissemination, distribution, or copying of this message, or any
> attachments, is strictly prohibited. If you have received this
> message in error, please advise the sender by reply e-mail, and
> delete the message and any attachments. Thank you.


More information about the U-Boot mailing list