On Wednesday, September 23, 2015 at 05:21:51 AM, Chin Liang See wrote:
> Hi,
Hi!
> On Wed, 2015-09-23 at 11:13 +0800, Chin Liang See wrote:
> > From: ma...@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 <cl...@altera.com>
> > > Cc: Dinh Nguyen <dingu...@opensource.altera.com>
> > > Cc: Dinh Nguyen <dinh.li...@gmail.com>
> > > Cc: Marek Vasut <ma...@denx.de>
> > > Cc: Stefan Roese <s...@denx.de>
> > > Cc: Vikas Manocha <vikas.mano...@st.com>
> > > Cc: Jagannadh Teki <jt...@openedev.com>
> > > Cc: Pavel Machek <pa...@denx.de>
> > > Acked-by: Marek Vasut <ma...@denx.de>
> > > Reviewed-by: Jagan Teki <jt...@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.
Ah, I see, that makes sense now, thanks!
Wouldn't it thus make more sense to place this check into the
cadence_spi_set_speed() function instead , so that the spi_calibrate()
function only really does calibration , no matter what ?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
