Hi,
On Wed, 2015-09-23 at 11:13 +0800, Chin Liang See wrote:
>
> From: [email protected]
> 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 <[email protected]>
> > Cc: Dinh Nguyen <[email protected]>
> > Cc: Dinh Nguyen <[email protected]>
> > Cc: Marek Vasut <[email protected]>
> > Cc: Stefan Roese <[email protected]>
> > Cc: Vikas Manocha <[email protected]>
> > Cc: Jagannadh Teki <[email protected]>
> > Cc: Pavel Machek <[email protected]>
> > Acked-by: Marek Vasut <[email protected]>
> > Reviewed-by: Jagan Teki <[email protected]>
> > ---
> > 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
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
