[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de --- drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{ + struct spi_slave *slave = dev_get_parentdata(dev); + + /* Cadence QSPI controller can support quad read and program */ + slave->op_mode_rx = SPI_OPM_RX_QOF; + slave->op_mode_tx = SPI_OPM_TX_QPP; + return 0; +} + static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), + .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe, }; diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
#define CQSPI_REG_POLL_US (1) /* 1us */ #define CQSPI_REG_RETRY (10000) @@ -82,6 +85,7 @@
#define CQSPI_REG_WR_INSTR 0x08 #define CQSPI_REG_WR_INSTR_OPCODE_LSB 0 +#define CQSPI_REG_WR_INSTR_TYPE_DATA_LSB 16
#define CQSPI_REG_DELAY 0x0C #define CQSPI_REG_DELAY_TSLCH_LSB 0 @@ -700,10 +704,10 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat, /* Configure the opcode */ rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
-#if (CONFIG_SPI_FLASH_QUAD == 1) - /* Instruction and address at DQ0, data at DQ0-3. */ - rd_reg |= CQSPI_INST_TYPE_QUAD << CQSPI_REG_RD_INSTR_TYPE_DATA_LSB; -#endif + if (cmdbuf[0] == CMD_READ_QUAD_OUTPUT_FAST) + /* Instruction and address at DQ0, data at DQ0-3. */ + rd_reg |= CQSPI_INST_TYPE_QUAD << + CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
/* Get address */ addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes); @@ -796,6 +800,10 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
/* Configure the opcode */ reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB; + if (cmdbuf[0] == CMD_QUAD_PAGE_PROGRAM) + /* Instruction and address at DQ0, data at DQ0-3. */ + reg |= CQSPI_INST_TYPE_QUAD << + CQSPI_REG_WR_INSTR_TYPE_DATA_LSB; writel(reg, plat->regbase + CQSPI_REG_WR_INSTR);
/* Setup write address. */
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
[...]
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Thanks Chin Liang
[...]
On Wednesday, August 26, 2015 at 09:30:07 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,
@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Ewww, I think we should implement something similar to:
https://lkml.org/lkml/2015/8/24/299 [PATCH linux-next v4 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
On Wed, 2015-08-26 at 09:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 09:30:07 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,
@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Ewww, I think we should implement something similar to:
https://lkml.org/lkml/2015/8/24/299 [PATCH linux-next v4 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
Thanks Chin Liang
On Wednesday, August 26, 2015 at 10:42:57 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 09:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 09:30:07 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,
@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Ewww, I think we should implement something similar to:
https://lkml.org/lkml/2015/8/24/299 [PATCH linux-next v4 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
On Wed, 2015-08-26 at 11:07 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 10:42:57 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 09:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 09:30:07 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,
@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Ewww, I think we should implement something similar to:
https://lkml.org/lkml/2015/8/24/299 [PATCH linux-next v4 1/5] mtd: spi-nor: notify (Q)SPI controller about protocol change
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time. I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
Thanks Chin Liang
On Wednesday, August 26, 2015 at 03:13:15 PM, Chin Liang See wrote:
Hi,
[...]
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time.
Sure, no problem, that should be in time for the next merge window.
I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
No, sorry, you can not get ack on a patch which is hacky. You cannot push hacky code into mainline just for the sake of getting something somehow working, that's not how it works. We either do things proper or not at all.
Best regards, Marek Vasut
On Wed, 2015-08-26 at 15:37 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:13:15 PM, Chin Liang See wrote:
Hi,
[...]
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time.
Sure, no problem, that should be in time for the next merge window.
I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
No, sorry, you can not get ack on a patch which is hacky. You cannot push hacky code into mainline just for the sake of getting something somehow working, that's not how it works. We either do things proper or not at all.
In this case, probably its something we can work together on enabling the spi-nor?
Thanks Chin Liang
Best regards, Marek Vasut
On Wednesday, August 26, 2015 at 03:42:36 PM, Chin Liang See wrote:
On Wed, 2015-08-26 at 15:37 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:13:15 PM, Chin Liang See wrote:
Hi,
[...]
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time.
Sure, no problem, that should be in time for the next merge window.
I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
No, sorry, you can not get ack on a patch which is hacky. You cannot push hacky code into mainline just for the sake of getting something somehow working, that's not how it works. We either do things proper or not at all.
In this case, probably its something we can work together on enabling the spi-nor?
Hi!
But the Cadence QSPI works in U-Boot, I'm using it on a board here. If you need additional features, I'm glad to ack patches which are done right.
Best regards, Marek Vasut
On Wed, 2015-08-26 at 15:58 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:42:36 PM, Chin Liang See wrote:
On Wed, 2015-08-26 at 15:37 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:13:15 PM, Chin Liang See wrote:
Hi,
[...]
Yah, that is a nice enhancement in order to keep up with controller enhancement. We definitely want to explore and enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time.
Sure, no problem, that should be in time for the next merge window.
I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
No, sorry, you can not get ack on a patch which is hacky. You cannot push hacky code into mainline just for the sake of getting something somehow working, that's not how it works. We either do things proper or not at all.
In this case, probably its something we can work together on enabling the spi-nor?
Hi!
But the Cadence QSPI works in U-Boot, I'm using it on a board here.
Yup, the basic function works. I am looking at 2 areas below
1. Quad mode Current driver is running at single IO mode. We yet to utilize the quad mode that is supported by the controller. Its for performance.
2. SCLK at 80MHz and beyond The driver fail to work at higher SCLK frequency. The github version work well at 100MHz. I am troubleshooting on this now
Thanks Chin Liang
If you need additional features, I'm glad to ack patches which are done right.
Best regards, Marek Vasut
On Thursday, August 27, 2015 at 07:14:07 AM, Chin Liang See wrote:
On Wed, 2015-08-26 at 15:58 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:42:36 PM, Chin Liang See wrote:
On Wed, 2015-08-26 at 15:37 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:13:15 PM, Chin Liang See wrote:
Hi,
[...]
> Yah, that is a nice enhancement in order to keep up with > controller enhancement. We definitely want to explore and > enable that at U-Boot in the future.
You mean you'll implement this functionality and then make your change to the QSPI driver to use it, in order to implement things properly ? :) In that case, I agree.
Sure, something I can look at in 1-2 months time.
Sure, no problem, that should be in time for the next merge window.
I still have laundry lists on ensuring the mainline SOCFPGA U-Boot has all the essential features as we have in github. Prior that happen, can I get an ACK from you? :)
No, sorry, you can not get ack on a patch which is hacky. You cannot push hacky code into mainline just for the sake of getting something somehow working, that's not how it works. We either do things proper or not at all.
In this case, probably its something we can work together on enabling the spi-nor?
Hi!
But the Cadence QSPI works in U-Boot, I'm using it on a board here.
Hi!
Yup, the basic function works. I am looking at 2 areas below
- Quad mode
Current driver is running at single IO mode. We yet to utilize the quad mode that is supported by the controller. Its for performance.
OK
- SCLK at 80MHz and beyond
The driver fail to work at higher SCLK frequency. The github version work well at 100MHz. I am troubleshooting on this now
Cool, thanks!
Best regards, Marek Vasut
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor
thanks!
On Wed, 2015-08-26 at 19:17 +0530, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor
In this case, I shall use the spi-nor for proper long term solution. Let me take a look and work that out with you.
Thanks Chin Liang
thanks!
On Wednesday, August 26, 2015 at 03:47:28 PM, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-no r
Is this stuff in any way compatible with the spi-nor stuff in Linux ?
On 26 August 2015 at 19:29, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:47:28 PM, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-no r
Is this stuff in any way compatible with the spi-nor stuff in Linux ?
Main intention is to compatible with Linux spi-nor, but instead of direct porting - this way is to make enhancements step by step.
thanks!
On Wednesday, August 26, 2015 at 04:39:40 PM, Jagan Teki wrote:
On 26 August 2015 at 19:29, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:47:28 PM, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe,
.probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@
#include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h"
+#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi- no r
Is this stuff in any way compatible with the spi-nor stuff in Linux ?
Main intention is to compatible with Linux spi-nor, but instead of direct porting - this way is to make enhancements step by step.
Why can't you port the SPI-NOR from Linux directly ?
On 26 August 2015 at 20:14, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 04:39:40 PM, Jagan Teki wrote:
On 26 August 2015 at 19:29, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 03:47:28 PM, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote: > Enable the quad output fast read and quad input fast program > support. Quad mode is supported by Cadence QSPI controller. > > Signed-off-by: Chin Liang See clsee@altera.com > Cc: Dinh Nguyen dinguyen@opensource.altera.com > 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 > Cc: Marek Vasut marex@denx.de > --- > > drivers/spi/cadence_qspi.c | 11 +++++++++++ > drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- > 2 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c > b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -318,6 +318,16 @@ static int > cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; > > } > > +static int cadence_spi_child_pre_probe(struct udevice *dev) > +{ > + struct spi_slave *slave = dev_get_parentdata(dev); > + > + /* Cadence QSPI controller can support quad read and program */ > + slave->op_mode_rx = SPI_OPM_RX_QOF; > + slave->op_mode_tx = SPI_OPM_TX_QPP; > + return 0; > +} > + > > static const struct dm_spi_ops cadence_spi_ops = { > > .xfer = cadence_spi_xfer, > .set_speed = cadence_spi_set_speed, > > @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { > > .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, > .platdata_auto_alloc_size = sizeof(struct > cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct > cadence_spi_priv), > > + .child_pre_probe = cadence_spi_child_pre_probe, > > .probe = cadence_spi_probe, > > };
Simon, can you please check if this DM bit is correct ?
> diff --git a/drivers/spi/cadence_qspi_apb.c > b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -29,6 +29,9 @@ > > #include <asm/io.h> > #include <asm/errno.h> > #include "cadence_qspi.h" > > +#include <spi.h> > +#include <spi_flash.h> > +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi- no r
Is this stuff in any way compatible with the spi-nor stuff in Linux ?
Main intention is to compatible with Linux spi-nor, but instead of direct porting - this way is to make enhancements step by step.
Why can't you port the SPI-NOR from Linux directly ?
There are some features that are not added in Linux, yet like BAR, dual_flash and way of handling ops like erase/read/write logic need to compatible to these features. I'm planning to make these ops logic sits as it is and add spi-nor on top it.
This way looks better for testing as I experienced so-far, and I'm hoping at the end these no much significant difference between Linux vs U-Boot except these feature sets.
thanks!
On Wed, Aug 26, 2015 at 07:17:28PM +0530, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor
What's not clear to me here is, are you implementing a similar framework to what the linux kernel has but with the same name, or slowly introducing the framework from the linux kernel? Thanks.
On 26 August 2015 at 21:27, Tom Rini trini@konsulko.com wrote:
On Wed, Aug 26, 2015 at 07:17:28PM +0530, Jagan Teki wrote:
On 26 August 2015 at 13:00, Chin Liang See clsee@altera.com wrote:
On Wed, 2015-08-26 at 08:57 +0200, marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
- struct spi_slave *slave = dev_get_parentdata(dev);
- /* Cadence QSPI controller can support quad read and program */
- slave->op_mode_rx = SPI_OPM_RX_QOF;
- slave->op_mode_tx = SPI_OPM_TX_QPP;
- return 0;
+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
- .child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,
};
Simon, can you please check if this DM bit is correct ?
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
Actually I am comparing the opcode to determine whether its a quad command. If yes, we need to setup the controller accordingly.
Sorry, this I wouldn't recommend as of now please assign quad directly instead setting up controller based on the flash stuff, Yes things need to change it on u-boot like spi-nor framework and currently we are working on it[1] will defiantly back with proper solution.
[1] http://git.denx.de/?p=u-boot/u-boot-spi.git;a=shortlog;h=refs/heads/spi-nor
What's not clear to me here is, are you implementing a similar framework to what the linux kernel has but with the same name, or slowly introducing the framework from the linux kernel? Thanks.
I can't say it's a direct Linux copy at least from the initial support, but with same compatibility means same style of approach and will slowly add missing futures.
Things which are differ from Linux, from base support - Code logic of spi_flash operations erase/read/write - As these flash ops are bound with BAR, dual_flash which are not there yet in Linux - Handling of spi-nor in Linux from high level with MTD, but here with spi_flash - No in-build MTD stuff
And these differ points, may implement same as Linux in future based on our u-boot design and need, but will take significant amount of time.
Apart from adding spi-nor, there is a quite significant tuning required on whole spi_flash handling code like, handling cmd_sf with the help of some spi_flash could be include/spi_flash.h or any common code in drivers/mtd/spi instead of direct calls to spi-uclass [1] this is to isolate flash handling directly to spi. And this will also an enhancement for spi-nand addition in future, where spi_flash will handling spi-nand.c at low level.
thanks!
Hi Marek,
On 25 August 2015 at 23:57, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0; }
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
struct spi_slave *slave = dev_get_parentdata(dev);/* Cadence QSPI controller can support quad read and program */slave->op_mode_rx = SPI_OPM_RX_QOF;slave->op_mode_tx = SPI_OPM_TX_QPP;return 0;+}
static const struct dm_spi_ops cadence_spi_ops = { .xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed, @@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = { .ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,};
Simon, can you please check if this DM bit is correct ?
It seems OK to me. I worry a bit that xfer() is causing SPI controller changes to happen based on inspection of the bytes being sent. I think that is what Marek is saying below. Do we need a new method for SPI to set the protocol?
The license header in cadence_qspi_apb.c should move to SPDX.
For the driver model bits:
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c index d053407..deffb6b 100644 --- a/drivers/spi/cadence_qspi_apb.c +++ b/drivers/spi/cadence_qspi_apb.c @@ -29,6 +29,9 @@ #include <asm/io.h> #include <asm/errno.h> #include "cadence_qspi.h" +#include <spi.h> +#include <spi_flash.h> +#include "../mtd/spi/sf_internal.h"
Why do you need this include ?
[...]
Regards, Simon
On Wednesday, August 26, 2015 at 03:19:21 PM, Simon Glass wrote:
Hi Marek,
On 25 August 2015 at 23:57, Marek Vasut marex@denx.de wrote:
On Wednesday, August 26, 2015 at 02:09:55 AM, Chin Liang See wrote:
Enable the quad output fast read and quad input fast program support. Quad mode is supported by Cadence QSPI controller.
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com 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 Cc: Marek Vasut marex@denx.de
drivers/spi/cadence_qspi.c | 11 +++++++++++ drivers/spi/cadence_qspi_apb.c | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 34a0f46..c6b69c4 100644 --- a/drivers/spi/cadence_qspi.c +++ b/drivers/spi/cadence_qspi.c @@ -318,6 +318,16 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus) return 0;
}
+static int cadence_spi_child_pre_probe(struct udevice *dev) +{
struct spi_slave *slave = dev_get_parentdata(dev);/* Cadence QSPI controller can support quad read and program */slave->op_mode_rx = SPI_OPM_RX_QOF;slave->op_mode_tx = SPI_OPM_TX_QPP;return 0;+}
static const struct dm_spi_ops cadence_spi_ops = {
.xfer = cadence_spi_xfer, .set_speed = cadence_spi_set_speed,@@ -341,5 +351,6 @@ U_BOOT_DRIVER(cadence_spi) = {
.ofdata_to_platdata = cadence_spi_ofdata_to_platdata, .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
.child_pre_probe = cadence_spi_child_pre_probe, .probe = cadence_spi_probe,};
Simon, can you please check if this DM bit is correct ?
It seems OK to me. I worry a bit that xfer() is causing SPI controller changes to happen based on inspection of the bytes being sent. I think that is what Marek is saying below. Do we need a new method for SPI to set the protocol?
Yes, that's what I'm saying.
The license header in cadence_qspi_apb.c should move to SPDX.
For the driver model bits:
Reviewed-by: Simon Glass sjg@chromium.org
Thanks :)
participants (5)
-
Chin Liang See -
Jagan Teki -
Marek Vasut -
Simon Glass -
Tom Rini