Hi,

On Wed, 2015-09-23 at 11:13 +0800, Chin Liang See wrote:

From: marex@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@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: Marek Vasut marex@denx.de Cc: Stefan Roese sr@denx.de Cc: Vikas Manocha vikas.manocha@st.com Cc: Jagannadh Teki jteki@openedev.com Cc: Pavel Machek pavel@denx.de Acked-by: Marek Vasut marex@denx.de Reviewed-by: Jagan Teki jteki@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