* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
@ 2015-09-08 1:17 Chin Liang See
2015-09-08 11:08 ` Marek Vasut
2015-09-08 12:18 ` Jagan Teki
0 siblings, 2 replies; 8+ messages in thread
From: Chin Liang See @ 2015-09-08 1:17 UTC (permalink / raw)
To: u-boot
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>
---
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 | 19 ++++++++++++-------
drivers/spi/cadence_qspi.h | 1 +
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 34a0f46..23c88d5 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);
@@ -46,6 +46,10 @@ static int spi_calibration(struct udevice *bus)
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;
+
/* start with slowest clock (1 MHz) */
cadence_spi_write_speed(bus, 1000000);
@@ -64,7 +68,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 +115,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 +130,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)) {
- 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-08 1:17 [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change Chin Liang See
@ 2015-09-08 11:08 ` Marek Vasut
2015-09-08 12:25 ` Jagan Teki
2015-09-08 12:18 ` Jagan Teki
1 sibling, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-09-08 11:08 UTC (permalink / raw)
To: u-boot
On Tuesday, September 08, 2015 at 03:17:49 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>
Jagan, please apply for -next or I'll pick it up. Which do you prefer ?
[...]
> 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;
Idea for future patch: we should get rid of these qspi_ prefixes, they are
really useless.
> + unsigned int previous_hz;
> };
>
> /* Functions call declaration */
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-08 1:17 [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change Chin Liang See
2015-09-08 11:08 ` Marek Vasut
@ 2015-09-08 12:18 ` Jagan Teki
1 sibling, 0 replies; 8+ messages in thread
From: Jagan Teki @ 2015-09-08 12:18 UTC (permalink / raw)
To: u-boot
On 8 September 2015 at 06:47, Chin Liang See <clsee@altera.com> 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>
> ---
> Changes for v2
> - remove frequency set before calibration
> - introducing previous_hz to store requested frequency
> - prevent calibration run when request same frequency
> ---
Reviewed-by: Jagan Teki <jteki@openedev.com>
thanks!
--
Jagan | openedev.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-08 11:08 ` Marek Vasut
@ 2015-09-08 12:25 ` Jagan Teki
2015-09-08 12:28 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Jagan Teki @ 2015-09-08 12:25 UTC (permalink / raw)
To: u-boot
On 8 September 2015 at 16:38, Marek Vasut <marex@denx.de> wrote:
> On Tuesday, September 08, 2015 at 03:17:49 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>
>
> Jagan, please apply for -next or I'll pick it up. Which do you prefer ?
You can pick, no issues. once 2/4, 3/4 ready.
>
> [...]
>
>> 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;
>
> Idea for future patch: we should get rid of these qspi_ prefixes, they are
> really useless.
>
>> + unsigned int previous_hz;
>> };
>>
>> /* Functions call declaration */
>
thanks!
--
Jagan | openedev.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-08 12:25 ` Jagan Teki
@ 2015-09-08 12:28 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2015-09-08 12:28 UTC (permalink / raw)
To: u-boot
On Tuesday, September 08, 2015 at 02:25:54 PM, Jagan Teki wrote:
> On 8 September 2015 at 16:38, Marek Vasut <marex@denx.de> wrote:
> > On Tuesday, September 08, 2015 at 03:17:49 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>
> >
> > Jagan, please apply for -next or I'll pick it up. Which do you prefer ?
>
> You can pick, no issues. once 2/4, 3/4 ready.
OK, will do.
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-03 14:17 ` Marek Vasut
@ 2015-09-08 1:16 ` Chin Liang See
0 siblings, 0 replies; 8+ messages in thread
From: Chin Liang See @ 2015-09-08 1:16 UTC (permalink / raw)
To: u-boot
Hi,
On Thu, 2015-09-03 at 16:17 +0200, marex at 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
2015-09-03 13:41 Chin Liang See
@ 2015-09-03 14:17 ` Marek Vasut
2015-09-08 1:16 ` Chin Liang See
0 siblings, 1 reply; 8+ messages in thread
From: Marek Vasut @ 2015-09-03 14:17 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
@ 2015-09-03 13:41 Chin Liang See
2015-09-03 14:17 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: Chin Liang See @ 2015-09-03 13:41 UTC (permalink / raw)
To: u-boot
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, 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;
}
--
1.7.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-09-08 12:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-08 1:17 [U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change Chin Liang See
2015-09-08 11:08 ` Marek Vasut
2015-09-08 12:25 ` Jagan Teki
2015-09-08 12:28 ` Marek Vasut
2015-09-08 12:18 ` Jagan Teki
-- strict thread matches above, loose matches on Subject: below --
2015-09-03 13:41 Chin Liang See
2015-09-03 14:17 ` Marek Vasut
2015-09-08 1:16 ` Chin Liang See
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.