[U-Boot] [PATCH 1/4] spi: cadence_qspi: Ensure spi_calibration is run when sclk change
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; }
Fix the fdt read for spi-max-frequency as it's contained in the child node. Current state of code is always returning default value.
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 300934e..a00af87 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -291,10 +291,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->regbase = (void *)data[0]; plat->ahbbase = (void *)data[2];
- /* Use 500KHz as a suitable default */ - plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency", - 500000); - /* All other paramters are embedded in the child node */ subnode = fdt_first_subnode(blob, node); if (subnode < 0) { @@ -302,6 +298,10 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return -ENODEV; }
+ /* Use 500KHz as a suitable default */ + plat->max_hz = fdtdec_get_int(blob, subnode, "spi-max-frequency", + 500000); + /* Read other parameters from DT */ plat->page_size = fdtdec_get_int(blob, subnode, "page-size", 256); plat->block_size = fdtdec_get_int(blob, subnode, "block-size", 16);
On Thursday, September 03, 2015 at 03:42:00 PM, Chin Liang See wrote:
Fix the fdt read for spi-max-frequency as it's contained in the child node. Current state of code is always returning default value.
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 300934e..a00af87 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -291,10 +291,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->regbase = (void *)data[0]; plat->ahbbase = (void *)data[2];
- /* Use 500KHz as a suitable default */
- plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
500000);- /* All other paramters are embedded in the child node */ subnode = fdt_first_subnode(blob, node); if (subnode < 0) {
@@ -302,6 +298,10 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return -ENODEV; }
- /* Use 500KHz as a suitable default */
- plat->max_hz = fdtdec_get_int(blob, subnode, "spi-max-frequency",
500000);
Use fdtdec_get_u32() or such, since the value is unsigned int (have you ever seen negative frequency ? ;-) ). Then check the fdtdec_get_u32() return value.
/* Read other parameters from DT */ plat->page_size = fdtdec_get_int(blob, subnode, "page-size", 256); plat->block_size = fdtdec_get_int(blob, subnode, "block-size", 16);
Best regards, Marek Vasut
Hi,
On Thu, 2015-09-03 at 16:19 +0200, marex@denx.de wrote:
On Thursday, September 03, 2015 at 03:42:00 PM, Chin Liang See wrote:
Fix the fdt read for spi-max-frequency as it's contained in the child node. Current state of code is always returning default value.
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 300934e..a00af87 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -291,10 +291,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->regbase = (void *)data[0]; plat->ahbbase = (void *)data[2];
- /* Use 500KHz as a suitable default */
- plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
500000);- /* All other paramters are embedded in the child node */ subnode = fdt_first_subnode(blob, node); if (subnode < 0) {
@@ -302,6 +298,10 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return -ENODEV; }
- /* Use 500KHz as a suitable default */
- plat->max_hz = fdtdec_get_int(blob, subnode, "spi-max-frequency",
500000);Use fdtdec_get_u32() or such, since the value is unsigned int (have you ever seen negative frequency ? ;-) ). Then check the fdtdec_get_u32() return value.
I git grep and no unsigned version. But I can add unsigned casting to avoid that.
Thanks Chin Liang
/* Read other parameters from DT */ plat->page_size = fdtdec_get_int(blob, subnode, "page-size", 256); plat->block_size = fdtdec_get_int(blob, subnode, "block-size", 16);
Best regards, Marek Vasut
On Tuesday, September 08, 2015 at 03:16:58 AM, Chin Liang See wrote:
Hi,
Hi!
On Thu, 2015-09-03 at 16:19 +0200, marex@denx.de wrote:
On Thursday, September 03, 2015 at 03:42:00 PM, Chin Liang See wrote:
Fix the fdt read for spi-max-frequency as it's contained in the child node. Current state of code is always returning default value.
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 300934e..a00af87 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -291,10 +291,6 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) plat->regbase = (void *)data[0];
plat->ahbbase = (void *)data[2];
/* Use 500KHz as a suitable default */
plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
500000);/* All other paramters are embedded in the child node */ subnode = fdt_first_subnode(blob, node); if (subnode < 0) {
@@ -302,6 +298,10 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return -ENODEV;
}
- /* Use 500KHz as a suitable default */
- plat->max_hz = fdtdec_get_int(blob, subnode, "spi-max-frequency",
500000);Use fdtdec_get_u32() or such, since the value is unsigned int (have you ever seen negative frequency ? ;-) ). Then check the fdtdec_get_u32() return value.
I git grep and no unsigned version. But I can add unsigned casting to avoid that.
I think fdt_getprop_u32() is what you're looking for.
Best regards, Marek Vasut
Ensure the intended SCLK frequency not exceeding the maximum frequency. If that happen, SCLK will set to maximum 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 | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index a00af87..52c29d5 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -123,6 +123,11 @@ static int cadence_spi_set_speed(struct udevice *bus, uint hz) struct cadence_spi_priv *priv = dev_get_priv(bus); int err;
+ if (hz > plat->max_hz) { + hz = plat->max_hz; + puts("SF: Default to maximum supported SCLK frequency\n"); + } + /* Disable QSPI */ cadence_qspi_apb_controller_disable(priv->regbase);
With a working QSPI calibration, the SCLK can now run up to 100MHz
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 --- arch/arm/dts/socfpga_cyclone5_socdk.dts | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/dts/socfpga_cyclone5_socdk.dts b/arch/arm/dts/socfpga_cyclone5_socdk.dts index 9650eb0..04e5695 100644 --- a/arch/arm/dts/socfpga_cyclone5_socdk.dts +++ b/arch/arm/dts/socfpga_cyclone5_socdk.dts @@ -86,7 +86,7 @@ #size-cells = <1>; compatible = "n25q00"; reg = <0>; /* chip select */ - spi-max-frequency = <50000000>; + spi-max-frequency = <100000000>; m25p,fast-read; page-size = <256>; block-size = <16>; /* 2^16, 64KB */
On Thursday, September 03, 2015 at 03:42:02 PM, Chin Liang See wrote:
With a working QSPI calibration, the SCLK can now run up to 100MHz
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
Reviewed-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
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
participants (2)
-
Chin Liang See -
Marek Vasut