[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

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@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. */
-- 
1.7.7.4

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

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 ?

[...]

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

On Wed, 2015-08-26 at 08:57 +0200, marex at 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


> [...]

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

On Wednesday, August 26, 2015 at 09:30:07 AM, Chin Liang See wrote:
> On Wed, 2015-08-26 at 08:57 +0200, marex at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

On Wed, 2015-08-26 at 09:57 +0200, marex at 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 at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

On Wednesday, August 26, 2015 at 10:42:57 AM, Chin Liang See wrote:
> On Wed, 2015-08-26 at 09:57 +0200, marex at 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 at 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.

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

On Wed, 2015-08-26 at 11:07 +0200, marex at 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 at 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 at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

On Wed, 2015-08-26 at 15:37 +0200, marex at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[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 at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

On Wed, 2015-08-26 at 15:58 +0200, marex at 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 at 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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[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 at 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 at 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
> 
> 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.

OK

> 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

Cool, thanks!

Best regards,
Marek Vasut

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Jagan Teki]

On 26 August 2015 at 13:00, Chin Liang See <clsee@altera.com> wrote:
> On Wed, 2015-08-26 at 08:57 +0200, marex at 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!
-- 
Jagan | openedev.

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Chin Liang See]

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 at 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!

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

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 at 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 ?

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Jagan Teki]

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 at 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!
-- 
Jagan | openedev.

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

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 at 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 ?

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Jagan Teki]

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 at 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!
-- 
Jagan | openedev.

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Tom Rini]

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 at 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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150826/8246508e/attachment.sig>

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Jagan Teki]

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 at 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.

[1] http://snag.gy/Pc8GG.jpg

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Simon Glass]

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

[U-Boot] [PATCH] spi: cadence_qspi: Enable quad mode for read and programming

[Marek Vasut]

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 :)