Message ID | 1441287722-2250-1-git-send-email-clsee@altera.com |
---|---|
State | Superseded |
Delegated to: | Marek Vasut |
Headers | show |
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@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> > --- > 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
Hi, On Thu, 2015-09-03 at 16:17 +0200, marex@denx.de wrote: > 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@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> > > --- > > 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. > I know where you coming from. Yah, calibration can be skipped when requested frequency same as previous or calibrated one. Let me add that. > > 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. Yah, let me remove the old code. Thanks Chin Liang > > > if (err) > > return err; > > } > > Best regards, > Marek Vasut
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, 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); if (err) return err; }
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> --- drivers/spi/cadence_qspi.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)