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

[Chin Liang See]

Hi,

On Wed, 2015-09-23 at 11:13 +0800, Chin Liang See wrote:
> 
> From: marex at denx.de 
> Sent: Thursday, September 10, 2015 7:52 PM
> To: Chin Liang See
> Cc: ZY - u-boot; Dinh Nguyen; Dinh Nguyen; ZY - sr; Vikas Manocha; Jagannadh Teki; ZY - pavel
> Subject: Re: [PATCH v3 1/5] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
> 
> On Thursday, September 10, 2015 at 07:01:12 AM, 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>
> > Acked-by: Marek Vasut <marex at denx.de>
> > Reviewed-by: Jagan Teki <jteki at openedev.com>
> > ---
> > Changes for v2
> > - remove frequency set before calibration
> > - introducing previous_hz to store requested frequency
> > - prevent calibration run when request same frequency
> > ---
> >  drivers/spi/cadence_qspi.c |   20 ++++++++++++--------
> >  drivers/spi/cadence_qspi.h |    1 +
> >  2 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> > index 34a0f46..0e7450c 100644
> > --- a/drivers/spi/cadence_qspi.c
> > +++ b/drivers/spi/cadence_qspi.c
> > @@ -37,15 +37,18 @@ 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);
> >  	void *base = priv->regbase;
> >  	u8 opcode_rdid = 0x9F;
> >  	unsigned int idcode = 0, temp = 0;
> >  	int err = 0, i, range_lo = -1, range_hi = -1;
> > 
> > +	/* if calibrated frequency same as reqeusted, skip it */
> > +	if (priv->qspi_calibrated_hz == hz)
> > +		return 0;
> 
> Do you need this check ? I think you're doing the same check in 
> cadence_spi_set_speed() down below here, so this is redundant.

Oh this is checked against calibrated value. It works together with the
check of previous_hz. This is to cater the incident where the calibrated
frequency = max frequency when requested frequency exceed the max
frequency. Some examples as below

Request 100MHz, Max 50MHz,  run, calibrated 50MHz, previous 100MHz
Request 100MHz, Max 50MHz, skip, calibrated 50MHz, previous 100MHz
Request  90MHz, Max 50MHz, skip, calibrated 50MHz, previous  90MHz

The check here will skip calibration for third incident.
Hope that explains.

Thanks
Chin Liang


> 
> >  	/* start with slowest clock (1 MHz) */
> >  	cadence_spi_write_speed(bus, 1000000);
> > 
> > @@ -64,7 +67,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 +114,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;
> > @@ -126,14 +129,15 @@ static int cadence_spi_set_speed(struct udevice *bus,
> > uint hz) /* Disable QSPI */
> >  	cadence_qspi_apb_controller_disable(priv->regbase);
> > 
> > -	cadence_spi_write_speed(bus, hz);
> > -
> >  	/* Calibration required for different SCLK speed or chip select */
> > -	if (priv->qspi_calibrated_hz != plat->max_hz ||
> > +	if (priv->previous_hz != hz ||
> >  	    priv->qspi_calibrated_cs != spi_chip_select(bus)) {
> 
> This check here is what I'm referring to.
> 
> > -		err = spi_calibration(bus);
> > +		err = spi_calibration(bus, hz);
> >  		if (err)
> >  			return err;
> > +
> > +		/* prevent calibration run when same as previous request */
> > +		priv->previous_hz = hz;
> >  	}
> > 
> >  	/* Enable QSPI */
> > diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
> > index 98e57aa..2912e36 100644
> > --- a/drivers/spi/cadence_qspi.h
> > +++ b/drivers/spi/cadence_qspi.h
> > @@ -38,6 +38,7 @@ struct cadence_spi_priv {
> >  	int		qspi_is_init;
> >  	unsigned int	qspi_calibrated_hz;
> >  	unsigned int	qspi_calibrated_cs;
> > +	unsigned int	previous_hz;
> >  };
> > 
> >  /* Functions call declaration */
> > --
> > 1.7.7.4