[U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change

[Marek Vasut]

On Thursday, September 03, 2015 at 03:41:59 PM, Chin Liang See wrote:
> Ensuring spi_calibration is run when there is a change of sclk
> frequency. This will ensure the qspi flash access works for high
> sclk frequency
> 
> Signed-off-by: Chin Liang See <clsee at altera.com>
> Cc: Dinh Nguyen <dinguyen at opensource.altera.com>
> Cc: Dinh Nguyen <dinh.linux at gmail.com>
> Cc: Marek Vasut <marex at denx.de>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Vikas Manocha <vikas.manocha at st.com>
> Cc: Jagannadh Teki <jteki at openedev.com>
> Cc: Pavel Machek <pavel at denx.de>
> ---
>  drivers/spi/cadence_qspi.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> index 34a0f46..300934e 100644
> --- a/drivers/spi/cadence_qspi.c
> +++ b/drivers/spi/cadence_qspi.c
> @@ -37,7 +37,7 @@ static int cadence_spi_write_speed(struct udevice *bus,
> uint hz) }
> 
>  /* Calibration sequence to determine the read data capture delay register
> */ -static int spi_calibration(struct udevice *bus)
> +static int spi_calibration(struct udevice *bus, uint hz)
>  {
>  	struct cadence_spi_platdata *plat = bus->platdata;
>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
> @@ -64,7 +64,7 @@ static int spi_calibration(struct udevice *bus)
>  	}
> 
>  	/* use back the intended clock and find low range */
> -	cadence_spi_write_speed(bus, plat->max_hz);
> +	cadence_spi_write_speed(bus, hz);
>  	for (i = 0; i < CQSPI_READ_CAPTURE_MAX_DELAY; i++) {
>  		/* Disable QSPI */
>  		cadence_qspi_apb_controller_disable(base);
> @@ -111,7 +111,7 @@ static int spi_calibration(struct udevice *bus)
>  	      (range_hi + range_lo) / 2, range_lo, range_hi);
> 
>  	/* just to ensure we do once only when speed or chip select change */
> -	priv->qspi_calibrated_hz = plat->max_hz;
> +	priv->qspi_calibrated_hz = hz;
>  	priv->qspi_calibrated_cs = spi_chip_select(bus);
> 
>  	return 0;
> @@ -131,7 +131,7 @@ static int cadence_spi_set_speed(struct udevice *bus,

Hi,

My impression is that the logic here should be like this:

if (hz > plat->max_hz) {
    printf("error, freq. too high");
    return -EINVAL;
}
if (priv->previous_hz != hz) /* Bus frequency changed, re-calibrate */
    spi_calibrate(bus, hz)
cadence_spi_write_speed(bus, priv->qspi_calibrated_hz);
priv->previous_hz = hz;

Note that you need a new private variable, previous_hz, to hold the previous
value of "hz". This is needed since the calibrated frequency might not be
equal to requested frequency.

> uint hz) /* Calibration required for different SCLK speed or chip select
> */ if (priv->qspi_calibrated_hz != plat->max_hz ||
>  	    priv->qspi_calibrated_cs != spi_chip_select(bus)) {
> -		err = spi_calibration(bus);
> +		err = spi_calibration(bus, hz);

This is called after the frequency is configured in this function, this looks
really backwards.

>  		if (err)
>  			return err;
>  	}

Best regards,
Marek Vasut