Hi Marek,
On Thu, 2015-09-24 at 09:16 +0200, ma...@denx.de wrote:
> 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!
Cool
>
> 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 ?
Sound right to me :)
Let me update this into v4.
Thanks
Chin Liang
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
