[U-Boot] [v3 0/4] spi: cadence_qspi: optimize & fix indirect rd-writes

[U-Boot] [v3 0/4] spi: cadence_qspi: optimize & fix indirect rd-writes

[Vikas Manocha]

This patchset:
- fixes trigger base & transfer start address register programming. This fix
superseeds the previous patch "spi: cadence_qspi: Fix the indirect ahb trigger
address setting".
- adds support to get fifo width from device tree

Changes in v3:
- removed two patches which were bypassing the sram level check.
- format string in patch corrected 3/4
- added commit message in patch 1/4

Changes in v2:
- rebased to master.
- removed patch "spi: cadence_qspi: read can be independent of fifo width", it
  was implemented in other patchset, in mainline now.

Vikas Manocha (4):
  spi: cadence_qspi: move trigger base configuration in init
  spi: cadence_qspi: fix indirect read/write start address
  spi: cadence_qspi: fix base trigger address & transfer start address
  spi: cadence_qspi: get fifo width from device tree

 arch/arm/dts/socfpga.dtsi      |    4 +++-
 arch/arm/dts/stv0991.dts       |    4 +++-
 drivers/spi/cadence_qspi.c     |   15 +++++++-------
 drivers/spi/cadence_qspi.h     |    6 ++++--
 drivers/spi/cadence_qspi_apb.c |   42 ++++++++++++++++------------------------
 5 files changed, 35 insertions(+), 36 deletions(-)

-- 
1.7.9.5

[U-Boot] [v3 1/4] spi: cadence_qspi: move trigger base configuration in init

[Vikas Manocha]

No need to configure indirect trigger address for every read/write.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changes in v3: added commit message & removed extra bracket.
Changes in v2: Rebased to master

 drivers/spi/cadence_qspi_apb.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index d053407..b46e5fe 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
 
 	/* Indirect mode configurations */
 	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
+	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
+				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Disable all interrupts */
 	writel(0, plat->regbase + CQSPI_REG_IRQMASK);
@@ -693,10 +695,6 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
 		/* for normal read (only ramtron as of now) */
 		addr_bytes = cmdlen - 1;
 
-	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
-
 	/* Configure the opcode */
 	rd_reg = cmdbuf[0] << CQSPI_REG_RD_INSTR_OPCODE_LSB;
 
@@ -790,9 +788,6 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 		       cmdlen, (unsigned int)cmdbuf);
 		return -EINVAL;
 	}
-	/* Setup the indirect trigger address */
-	writel(((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK),
-	       plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Configure the opcode */
 	reg = cmdbuf[0] << CQSPI_REG_WR_INSTR_OPCODE_LSB;
-- 
1.7.9.5

[U-Boot] [v3 1/4] spi: cadence_qspi: move trigger base configuration in init

[Marek Vasut]

On Saturday, August 15, 2015 at 04:15:57 AM, Vikas Manocha wrote:
> No need to configure indirect trigger address for every read/write.
> 
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> ---
> 
> Changes in v3: added commit message & removed extra bracket.
> Changes in v2: Rebased to master
> 
>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c index d053407..b46e5fe 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
> cadence_spi_platdata *plat)
> 
>  	/* Indirect mode configurations */
>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
> +	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
> +				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Please drop this (u32) cast, it's misleading and problematic.

Best regards,
Marek Vasut

[U-Boot] [v3 1/4] spi: cadence_qspi: move trigger base configuration in init

[vikas]

Hi,

On 08/19/2015 08:46 PM, Marek Vasut wrote:
> On Saturday, August 15, 2015 at 04:15:57 AM, Vikas Manocha wrote:
>> No need to configure indirect trigger address for every read/write.
>>
>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>> ---
>>
>> Changes in v3: added commit message & removed extra bracket.
>> Changes in v2: Rebased to master
>>
>>  drivers/spi/cadence_qspi_apb.c |    9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/spi/cadence_qspi_apb.c
>> b/drivers/spi/cadence_qspi_apb.c index d053407..b46e5fe 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -534,6 +534,8 @@ void cadence_qspi_apb_controller_init(struct
>> cadence_spi_platdata *plat)
>>
>>  	/* Indirect mode configurations */
>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>> +	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
>> +				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> 
> Please drop this (u32) cast, it's misleading and problematic.

ok, will fix in v4.

Rgds,
Vikas

> 
> Best regards,
> Marek Vasut
> .
> 

[U-Boot] [v3 2/4] spi: cadence_qspi: fix indirect read/write start address

[Vikas Manocha]

Indirect read/write start addresses are flash start addresses for indirect read
or write transfers. These should be absolute flash addresses instead of
offsets.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changes in v3: none
Changes in v2: Rebased to master

 drivers/spi/cadence_qspi_apb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index b46e5fe..6b5ae30 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -705,7 +705,8 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
 
 	/* Get address */
 	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
-	writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
+	writel((u32)plat->ahbbase + addr_value,
+			plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
 
 	/* The remaining lenght is dummy bytes. */
 	dummy_bytes = cmdlen - addr_bytes - 1;
@@ -795,7 +796,8 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 
 	/* Setup write address. */
 	reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
-	writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
+	writel((u32)plat->ahbbase + reg,
+			plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
 
 	reg = readl(plat->regbase + CQSPI_REG_SIZE);
 	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
-- 
1.7.9.5

[U-Boot] [v3 2/4] spi: cadence_qspi: fix indirect read/write start address

[Marek Vasut]

On Saturday, August 15, 2015 at 04:15:58 AM, Vikas Manocha wrote:
> Indirect read/write start addresses are flash start addresses for indirect
> read or write transfers. These should be absolute flash addresses instead
> of offsets.
> 
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> ---
> 
> Changes in v3: none
> Changes in v2: Rebased to master
> 
>  drivers/spi/cadence_qspi_apb.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c index b46e5fe..6b5ae30 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -705,7 +705,8 @@ int cadence_qspi_apb_indirect_read_setup(struct
> cadence_spi_platdata *plat,
> 
>  	/* Get address */
>  	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
> -	writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
> +	writel((u32)plat->ahbbase + addr_value,
> +			plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);

Please drop this (u32) cast, it's misleading and problematic.

>  	/* The remaining lenght is dummy bytes. */
>  	dummy_bytes = cmdlen - addr_bytes - 1;
> @@ -795,7 +796,8 @@ int cadence_qspi_apb_indirect_write_setup(struct
> cadence_spi_platdata *plat,
> 
>  	/* Setup write address. */
>  	reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
> -	writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
> +	writel((u32)plat->ahbbase + reg,
> +			plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);

Please drop this (u32) cast, it's misleading and problematic.

Best regards,
Marek Vasut

[U-Boot] [v3 2/4] spi: cadence_qspi: fix indirect read/write start address

[vikas]

Hi,

On 08/19/2015 08:46 PM, Marek Vasut wrote:
> On Saturday, August 15, 2015 at 04:15:58 AM, Vikas Manocha wrote:
>> Indirect read/write start addresses are flash start addresses for indirect
>> read or write transfers. These should be absolute flash addresses instead
>> of offsets.
>>
>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>> ---
>>
>> Changes in v3: none
>> Changes in v2: Rebased to master
>>
>>  drivers/spi/cadence_qspi_apb.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/cadence_qspi_apb.c
>> b/drivers/spi/cadence_qspi_apb.c index b46e5fe..6b5ae30 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -705,7 +705,8 @@ int cadence_qspi_apb_indirect_read_setup(struct
>> cadence_spi_platdata *plat,
>>
>>  	/* Get address */
>>  	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
>> -	writel(addr_value, plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
>> +	writel((u32)plat->ahbbase + addr_value,
>> +			plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
> 
> Please drop this (u32) cast, it's misleading and problematic.

ok, Thanks for it.

> 
>>  	/* The remaining lenght is dummy bytes. */
>>  	dummy_bytes = cmdlen - addr_bytes - 1;
>> @@ -795,7 +796,8 @@ int cadence_qspi_apb_indirect_write_setup(struct
>> cadence_spi_platdata *plat,
>>
>>  	/* Setup write address. */
>>  	reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
>> -	writel(reg, plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
>> +	writel((u32)plat->ahbbase + reg,
>> +			plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
> 
> Please drop this (u32) cast, it's misleading and problematic.

ok, Thanks for pointing it out.

Rgds,
Vikas

> 
> Best regards,
> Marek Vasut
> .
> 

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[Vikas Manocha]

This patch is to separate the base trigger from the read/write transfer start
addresses.

Base trigger register address (0x1c register) corresponds to the address which
should be put on AHB bus to handle indirect transfer triggered before.

To handle indirect transfer we need to issue addresses from (value of 0x1c) to
(value of 0x1c) + 15*4 ("4" corresponds to size of SRAM location).
There are no obstacles in issuing const address just equal to 0x1c. Important
thing to note is that indirect trigger address has nothing in common with your
physical or mapped NOR Flash address.

Transfer read/write start addresses (offset 0x68/0x78)should be programmed with
the absolute flash address to be read/written.

plat->ahbbase has been renamed to plat->flashbase for clarity.
plat->triggerbase is added in device tree for mapped spi flash address.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changes in v3: formatted string breaking fixed.
Changes in v2: Rebased to master

 arch/arm/dts/socfpga.dtsi      |    3 ++-
 arch/arm/dts/stv0991.dts       |    3 ++-
 drivers/spi/cadence_qspi.c     |   14 +++++++-------
 drivers/spi/cadence_qspi.h     |    5 +++--
 drivers/spi/cadence_qspi_apb.c |   11 +++++------
 5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index 9b12420..25053ec 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -633,7 +633,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0xff705000 0x1000>,
-				<0xffa00000 0x1000>;
+				<0xffa00000 0x1000>,
+				<0xffa00000 0x0010>;
 			interrupts = <0 151 4>;
 			clocks = <&qspi_clk>;
 			ext-decoder = <0>;  /* external decoder */
diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
index fa3fd64..e23d4fd 100644
--- a/arch/arm/dts/stv0991.dts
+++ b/arch/arm/dts/stv0991.dts
@@ -30,7 +30,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			reg = <0x80203000 0x100>,
-				<0x40000000 0x1000000>;
+				<0x40000000 0x1000000>,
+				<0x40000000 0x0000010>;
 			clocks = <3750000>;
 			sram-size = <256>;
 			status = "okay";
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 34a0f46..01eff71 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
 	struct cadence_spi_priv *priv = dev_get_priv(bus);
 
 	priv->regbase = plat->regbase;
-	priv->ahbbase = plat->ahbbase;
+	priv->flashbase = plat->flashbase;
 
 	if (!priv->qspi_is_init) {
 		cadence_qspi_apb_controller_init(plat);
@@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	const void *blob = gd->fdt_blob;
 	int node = bus->of_offset;
 	int subnode;
-	u32 data[4];
+	u32 data[6];
 	int ret;
 
 	/* 2 base addresses are needed, lets get them from the DT */
@@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	}
 
 	plat->regbase = (void *)data[0];
-	plat->ahbbase = (void *)data[2];
+	plat->flashbase = (void *)data[2];
+	plat->trigger_base = (void *)data[4];
 
 	/* Use 500KHz as a suitable default */
 	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
@@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
 	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
 
-	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
-	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
-	      plat->page_size);
-
+	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d page-size=%d\n",
+		__func__, plat->regbase, plat->flashbase, plat->trigger_base,
+		plat->max_hz, plat->page_size);
 	return 0;
 }
 
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index 98e57aa..7341339 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -17,7 +17,8 @@
 struct cadence_spi_platdata {
 	unsigned int	max_hz;
 	void		*regbase;
-	void		*ahbbase;
+	void		*flashbase;
+	void		*trigger_base;
 
 	u32		page_size;
 	u32		block_size;
@@ -30,7 +31,7 @@ struct cadence_spi_platdata {
 
 struct cadence_spi_priv {
 	void		*regbase;
-	void		*ahbbase;
+	void		*flashbase;
 	size_t		cmd_len;
 	u8		cmd_buf[32];
 	size_t		data_len;
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 6b5ae30..25a7ed2 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -44,7 +44,6 @@
 #define CQSPI_INST_TYPE_QUAD			(2)
 
 #define CQSPI_STIG_DATA_LEN_MAX			(8)
-#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
 
 #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
 #define CQSPI_DUMMY_BYTES_MAX			(4)
@@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat,
 				const void *src_addr, unsigned int num_bytes)
 {
 	const void *reg_base = plat->regbase;
-	void *dest_addr = plat->ahbbase;
+	void *dest_addr = plat->trigger_base;
 	unsigned int retry = CQSPI_REG_RETRY;
 	unsigned int sram_level;
 	unsigned int wr_bytes;
@@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct cadence_spi_platdata *plat)
 
 	/* Indirect mode configurations */
 	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
-	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
+	writel((u32)plat->trigger_base,
 				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
 
 	/* Disable all interrupts */
@@ -705,7 +704,7 @@ int cadence_qspi_apb_indirect_read_setup(struct cadence_spi_platdata *plat,
 
 	/* Get address */
 	addr_value = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
-	writel((u32)plat->ahbbase + addr_value,
+	writel((u32)plat->flashbase + addr_value,
 			plat->regbase + CQSPI_REG_INDIRECTRDSTARTADDR);
 
 	/* The remaining lenght is dummy bytes. */
@@ -753,7 +752,7 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 
 	if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf,
-				     (const void *)plat->ahbbase, rxlen))
+				     (const void *)plat->trigger_base, rxlen))
 		goto failrd;
 
 	/* Check flash indirect controller */
@@ -796,7 +795,7 @@ int cadence_qspi_apb_indirect_write_setup(struct cadence_spi_platdata *plat,
 
 	/* Setup write address. */
 	reg = cadence_qspi_apb_cmd2addr(&cmdbuf[1], addr_bytes);
-	writel((u32)plat->ahbbase + reg,
+	writel((u32)plat->flashbase + reg,
 			plat->regbase + CQSPI_REG_INDIRECTWRSTARTADDR);
 
 	reg = readl(plat->regbase + CQSPI_REG_SIZE);
-- 
1.7.9.5

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[Marek Vasut]

On Saturday, August 15, 2015 at 04:15:59 AM, Vikas Manocha wrote:
> This patch is to separate the base trigger from the read/write transfer
> start addresses.
> 
> Base trigger register address (0x1c register) corresponds to the address
> which should be put on AHB bus to handle indirect transfer triggered
> before.
> 
> To handle indirect transfer we need to issue addresses from (value of 0x1c)
> to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM location).
> There are no obstacles in issuing const address just equal to 0x1c.
> Important thing to note is that indirect trigger address has nothing in
> common with your physical or mapped NOR Flash address.
> 
> Transfer read/write start addresses (offset 0x68/0x78)should be programmed
> with the absolute flash address to be read/written.
> 
> plat->ahbbase has been renamed to plat->flashbase for clarity.
> plat->triggerbase is added in device tree for mapped spi flash address.
> 
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> ---
> 
> Changes in v3: formatted string breaking fixed.
> Changes in v2: Rebased to master
> 
>  arch/arm/dts/socfpga.dtsi      |    3 ++-
>  arch/arm/dts/stv0991.dts       |    3 ++-
>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
>  drivers/spi/cadence_qspi.h     |    5 +++--
>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
>  5 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> index 9b12420..25053ec 100644
> --- a/arch/arm/dts/socfpga.dtsi
> +++ b/arch/arm/dts/socfpga.dtsi
> @@ -633,7 +633,8 @@
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0xff705000 0x1000>,
> -				<0xffa00000 0x1000>;
> +				<0xffa00000 0x1000>,
> +				<0xffa00000 0x0010>;

Can you please follow the DT bindings Graham and me sent for Linux kernel 
instead of adding ad-hoc undocumented stuff ?

>  			interrupts = <0 151 4>;
>  			clocks = <&qspi_clk>;
>  			ext-decoder = <0>;  /* external decoder */
> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
> index fa3fd64..e23d4fd 100644
> --- a/arch/arm/dts/stv0991.dts
> +++ b/arch/arm/dts/stv0991.dts
> @@ -30,7 +30,8 @@
>  			#address-cells = <1>;
>  			#size-cells = <0>;
>  			reg = <0x80203000 0x100>,
> -				<0x40000000 0x1000000>;
> +				<0x40000000 0x1000000>,
> +				<0x40000000 0x0000010>;
>  			clocks = <3750000>;
>  			sram-size = <256>;
>  			status = "okay";
> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> index 34a0f46..01eff71 100644
> --- a/drivers/spi/cadence_qspi.c
> +++ b/drivers/spi/cadence_qspi.c
> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
> 
>  	priv->regbase = plat->regbase;
> -	priv->ahbbase = plat->ahbbase;
> +	priv->flashbase = plat->flashbase;

If you need to rename anything, it should be in a separate patch. Please
keep it separate.

>  	if (!priv->qspi_is_init) {
>  		cadence_qspi_apb_controller_init(plat);
> @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct
> udevice *bus) const void *blob = gd->fdt_blob;
>  	int node = bus->of_offset;
>  	int subnode;
> -	u32 data[4];
> +	u32 data[6];
>  	int ret;
> 
>  	/* 2 base addresses are needed, lets get them from the DT */
> @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct
> udevice *bus) }
> 
>  	plat->regbase = (void *)data[0];
> -	plat->ahbbase = (void *)data[2];
> +	plat->flashbase = (void *)data[2];
> +	plat->trigger_base = (void *)data[4];

Please switch this to fdtdec_get_addr() instead ; if you use the Linux
binding, you will be able to do this easily.

>  	/* Use 500KHz as a suitable default */
>  	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
> @@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct
> udevice *bus) plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns",
> 20);
>  	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> 
> -	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
> -	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
> -	      plat->page_size);
> -
> +	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d
> page-size=%d\n", +		__func__, plat->regbase, plat->flashbase,
> plat->trigger_base,
> +		plat->max_hz, plat->page_size);

Please don't break the indent of the args. The original debug() statement
had the args correctly indented ; this patch breaks the indent.

>  	return 0;
>  }
> 
> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
> index 98e57aa..7341339 100644
> --- a/drivers/spi/cadence_qspi.h
> +++ b/drivers/spi/cadence_qspi.h
> @@ -17,7 +17,8 @@
>  struct cadence_spi_platdata {
>  	unsigned int	max_hz;
>  	void		*regbase;
> -	void		*ahbbase;
> +	void		*flashbase;
> +	void		*trigger_base;
> 
>  	u32		page_size;
>  	u32		block_size;
> @@ -30,7 +31,7 @@ struct cadence_spi_platdata {
> 
>  struct cadence_spi_priv {
>  	void		*regbase;
> -	void		*ahbbase;
> +	void		*flashbase;
>  	size_t		cmd_len;
>  	u8		cmd_buf[32];
>  	size_t		data_len;
> diff --git a/drivers/spi/cadence_qspi_apb.c
> b/drivers/spi/cadence_qspi_apb.c index 6b5ae30..25a7ed2 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -44,7 +44,6 @@
>  #define CQSPI_INST_TYPE_QUAD			(2)
> 
>  #define CQSPI_STIG_DATA_LEN_MAX			(8)
> -#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
> 
>  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
>  #define CQSPI_DUMMY_BYTES_MAX			(4)
> @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct
> cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes)
>  {
>  	const void *reg_base = plat->regbase;
> -	void *dest_addr = plat->ahbbase;
> +	void *dest_addr = plat->trigger_base;
>  	unsigned int retry = CQSPI_REG_RETRY;
>  	unsigned int sram_level;
>  	unsigned int wr_bytes;
> @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct
> cadence_spi_platdata *plat)
> 
>  	/* Indirect mode configurations */
>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
> -	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
> +	writel((u32)plat->trigger_base,
>  				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);

Here you actually changed to logic of the code, which breaks it for SoCFPGA.
plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK = 0x0 for SoCFPGA, but now
you changed it such that 0xffa00000 is written into the register. Same does
apply for all your changes below.

This patch needs fixing.

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[vikas]

Hi,

On 08/19/2015 08:54 PM, Marek Vasut wrote:
> On Saturday, August 15, 2015 at 04:15:59 AM, Vikas Manocha wrote:
>> This patch is to separate the base trigger from the read/write transfer
>> start addresses.
>>
>> Base trigger register address (0x1c register) corresponds to the address
>> which should be put on AHB bus to handle indirect transfer triggered
>> before.
>>
>> To handle indirect transfer we need to issue addresses from (value of 0x1c)
>> to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM location).
>> There are no obstacles in issuing const address just equal to 0x1c.
>> Important thing to note is that indirect trigger address has nothing in
>> common with your physical or mapped NOR Flash address.
>>
>> Transfer read/write start addresses (offset 0x68/0x78)should be programmed
>> with the absolute flash address to be read/written.
>>
>> plat->ahbbase has been renamed to plat->flashbase for clarity.
>> plat->triggerbase is added in device tree for mapped spi flash address.
>>
>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>> ---
>>
>> Changes in v3: formatted string breaking fixed.
>> Changes in v2: Rebased to master
>>
>>  arch/arm/dts/socfpga.dtsi      |    3 ++-
>>  arch/arm/dts/stv0991.dts       |    3 ++-
>>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
>>  drivers/spi/cadence_qspi.h     |    5 +++--
>>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
>>  5 files changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
>> index 9b12420..25053ec 100644
>> --- a/arch/arm/dts/socfpga.dtsi
>> +++ b/arch/arm/dts/socfpga.dtsi
>> @@ -633,7 +633,8 @@
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>>  			reg = <0xff705000 0x1000>,
>> -				<0xffa00000 0x1000>;
>> +				<0xffa00000 0x1000>,
>> +				<0xffa00000 0x0010>;
> 
> Can you please follow the DT bindings Graham and me sent for Linux kernel 
> instead of adding ad-hoc undocumented stuff ?

It is aligned to current DT binding of u-boot. 
I agree to align it to linux kernel DT binding but that should be a separate patch.

> 
>>  			interrupts = <0 151 4>;
>>  			clocks = <&qspi_clk>;
>>  			ext-decoder = <0>;  /* external decoder */
>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
>> index fa3fd64..e23d4fd 100644
>> --- a/arch/arm/dts/stv0991.dts
>> +++ b/arch/arm/dts/stv0991.dts
>> @@ -30,7 +30,8 @@
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>>  			reg = <0x80203000 0x100>,
>> -				<0x40000000 0x1000000>;
>> +				<0x40000000 0x1000000>,
>> +				<0x40000000 0x0000010>;
>>  			clocks = <3750000>;
>>  			sram-size = <256>;
>>  			status = "okay";
>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>> index 34a0f46..01eff71 100644
>> --- a/drivers/spi/cadence_qspi.c
>> +++ b/drivers/spi/cadence_qspi.c
>> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
>>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
>>
>>  	priv->regbase = plat->regbase;
>> -	priv->ahbbase = plat->ahbbase;
>> +	priv->flashbase = plat->flashbase;
> 
> If you need to rename anything, it should be in a separate patch. Please
> keep it separate.

this renaming is done here for more clarity as the earlier ahbbase usage is split between "flashbase" & "trigger base".
ahbbase is not relevant anymore.

> 
>>  	if (!priv->qspi_is_init) {
>>  		cadence_qspi_apb_controller_init(plat);
>> @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct
>> udevice *bus) const void *blob = gd->fdt_blob;
>>  	int node = bus->of_offset;
>>  	int subnode;
>> -	u32 data[4];
>> +	u32 data[6];
>>  	int ret;
>>
>>  	/* 2 base addresses are needed, lets get them from the DT */
>> @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct
>> udevice *bus) }
>>
>>  	plat->regbase = (void *)data[0];
>> -	plat->ahbbase = (void *)data[2];
>> +	plat->flashbase = (void *)data[2];
>> +	plat->trigger_base = (void *)data[4];
> 
> Please switch this to fdtdec_get_addr() instead ; if you use the Linux
> binding, you will be able to do this easily.

As above, should be separate patch to align to Linux binding.

> 
>>  	/* Use 500KHz as a suitable default */
>>  	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
>> @@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct
>> udevice *bus) plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns",
>> 20);
>>  	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
>>
>> -	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
>> -	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
>> -	      plat->page_size);
>> -
>> +	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d
>> page-size=%d\n", +		__func__, plat->regbase, plat->flashbase,
>> plat->trigger_base,
>> +		plat->max_hz, plat->page_size);
> 
> Please don't break the indent of the args. The original debug() statement
> had the args correctly indented ; this patch breaks the indent.

I think it looks like this here as the line length is more than 80 chars for args. After applying
patch it looks ok. If not, please let me know how to fix it.

> 
>>  	return 0;
>>  }
>>
>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
>> index 98e57aa..7341339 100644
>> --- a/drivers/spi/cadence_qspi.h
>> +++ b/drivers/spi/cadence_qspi.h
>> @@ -17,7 +17,8 @@
>>  struct cadence_spi_platdata {
>>  	unsigned int	max_hz;
>>  	void		*regbase;
>> -	void		*ahbbase;
>> +	void		*flashbase;
>> +	void		*trigger_base;
>>
>>  	u32		page_size;
>>  	u32		block_size;
>> @@ -30,7 +31,7 @@ struct cadence_spi_platdata {
>>
>>  struct cadence_spi_priv {
>>  	void		*regbase;
>> -	void		*ahbbase;
>> +	void		*flashbase;
>>  	size_t		cmd_len;
>>  	u8		cmd_buf[32];
>>  	size_t		data_len;
>> diff --git a/drivers/spi/cadence_qspi_apb.c
>> b/drivers/spi/cadence_qspi_apb.c index 6b5ae30..25a7ed2 100644
>> --- a/drivers/spi/cadence_qspi_apb.c
>> +++ b/drivers/spi/cadence_qspi_apb.c
>> @@ -44,7 +44,6 @@
>>  #define CQSPI_INST_TYPE_QUAD			(2)
>>
>>  #define CQSPI_STIG_DATA_LEN_MAX			(8)
>> -#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
>>
>>  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
>>  #define CQSPI_DUMMY_BYTES_MAX			(4)
>> @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct
>> cadence_spi_platdata *plat, const void *src_addr, unsigned int num_bytes)
>>  {
>>  	const void *reg_base = plat->regbase;
>> -	void *dest_addr = plat->ahbbase;
>> +	void *dest_addr = plat->trigger_base;
>>  	unsigned int retry = CQSPI_REG_RETRY;
>>  	unsigned int sram_level;
>>  	unsigned int wr_bytes;
>> @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct
>> cadence_spi_platdata *plat)
>>
>>  	/* Indirect mode configurations */
>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>> -	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
>> +	writel((u32)plat->trigger_base,
>>  				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> 
> Here you actually changed to logic of the code, which breaks it for SoCFPGA.
> plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK = 0x0 for SoCFPGA, but now
> you changed it such that 0xffa00000 is written into the register. Same does
> apply for all your changes below.

If we pass 0x00 from device tree, 0x0 would be written in the register. We have to check on SOCFpga platform is
0xFFA0_0000 is correct value or only 0x0. Anyways i will change the DT value to 0x0 for SOCFpga in next version.

Rgds,
Vikas

> 
> This patch needs fixing.
> .
> 

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[Marek Vasut]

On Thursday, August 20, 2015 at 06:48:36 PM, vikas wrote:
> Hi,
> 
> On 08/19/2015 08:54 PM, Marek Vasut wrote:
> > On Saturday, August 15, 2015 at 04:15:59 AM, Vikas Manocha wrote:
> >> This patch is to separate the base trigger from the read/write transfer
> >> start addresses.
> >> 
> >> Base trigger register address (0x1c register) corresponds to the address
> >> which should be put on AHB bus to handle indirect transfer triggered
> >> before.
> >> 
> >> To handle indirect transfer we need to issue addresses from (value of
> >> 0x1c) to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM
> >> location). There are no obstacles in issuing const address just equal
> >> to 0x1c. Important thing to note is that indirect trigger address has
> >> nothing in common with your physical or mapped NOR Flash address.
> >> 
> >> Transfer read/write start addresses (offset 0x68/0x78)should be
> >> programmed with the absolute flash address to be read/written.
> >> 
> >> plat->ahbbase has been renamed to plat->flashbase for clarity.
> >> plat->triggerbase is added in device tree for mapped spi flash address.
> >> 
> >> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >> ---
> >> 
> >> Changes in v3: formatted string breaking fixed.
> >> Changes in v2: Rebased to master
> >> 
> >>  arch/arm/dts/socfpga.dtsi      |    3 ++-
> >>  arch/arm/dts/stv0991.dts       |    3 ++-
> >>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
> >>  drivers/spi/cadence_qspi.h     |    5 +++--
> >>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
> >>  5 files changed, 19 insertions(+), 17 deletions(-)
> >> 
> >> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> >> index 9b12420..25053ec 100644
> >> --- a/arch/arm/dts/socfpga.dtsi
> >> +++ b/arch/arm/dts/socfpga.dtsi
> >> @@ -633,7 +633,8 @@
> >> 
> >>  			#address-cells = <1>;
> >>  			#size-cells = <0>;
> >>  			reg = <0xff705000 0x1000>,
> >> 
> >> -				<0xffa00000 0x1000>;
> >> +				<0xffa00000 0x1000>,
> >> +				<0xffa00000 0x0010>;
> > 
> > Can you please follow the DT bindings Graham and me sent for Linux kernel
> > instead of adding ad-hoc undocumented stuff ?
> 
> It is aligned to current DT binding of u-boot.
> I agree to align it to linux kernel DT binding but that should be a
> separate patch.

And this separate patch should come in FIRST, we do not want to support two
sets of bindings, no way.

> >>  			interrupts = <0 151 4>;
> >>  			clocks = <&qspi_clk>;
> >>  			ext-decoder = <0>;  /* external decoder */
> >> 
> >> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
> >> index fa3fd64..e23d4fd 100644
> >> --- a/arch/arm/dts/stv0991.dts
> >> +++ b/arch/arm/dts/stv0991.dts
> >> @@ -30,7 +30,8 @@
> >> 
> >>  			#address-cells = <1>;
> >>  			#size-cells = <0>;
> >>  			reg = <0x80203000 0x100>,
> >> 
> >> -				<0x40000000 0x1000000>;
> >> +				<0x40000000 0x1000000>,
> >> +				<0x40000000 0x0000010>;
> >> 
> >>  			clocks = <3750000>;
> >>  			sram-size = <256>;
> >>  			status = "okay";
> >> 
> >> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> >> index 34a0f46..01eff71 100644
> >> --- a/drivers/spi/cadence_qspi.c
> >> +++ b/drivers/spi/cadence_qspi.c
> >> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
> >> 
> >>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
> >>  	
> >>  	priv->regbase = plat->regbase;
> >> 
> >> -	priv->ahbbase = plat->ahbbase;
> >> +	priv->flashbase = plat->flashbase;
> > 
> > If you need to rename anything, it should be in a separate patch. Please
> > keep it separate.
> 
> this renaming is done here for more clarity as the earlier ahbbase usage is
> split between "flashbase" & "trigger base". ahbbase is not relevant
> anymore.

Which is why it should happen in a separate patch :)

> >>  	if (!priv->qspi_is_init) {
> >>  	
> >>  		cadence_qspi_apb_controller_init(plat);
> >> 
> >> @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct
> >> udevice *bus) const void *blob = gd->fdt_blob;
> >> 
> >>  	int node = bus->of_offset;
> >>  	int subnode;
> >> 
> >> -	u32 data[4];
> >> +	u32 data[6];
> >> 
> >>  	int ret;
> >>  	
> >>  	/* 2 base addresses are needed, lets get them from the DT */
> >> 
> >> @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct
> >> udevice *bus) }
> >> 
> >>  	plat->regbase = (void *)data[0];
> >> 
> >> -	plat->ahbbase = (void *)data[2];
> >> +	plat->flashbase = (void *)data[2];
> >> +	plat->trigger_base = (void *)data[4];
> > 
> > Please switch this to fdtdec_get_addr() instead ; if you use the Linux
> > binding, you will be able to do this easily.
> 
> As above, should be separate patch to align to Linux binding.

Correct ; and it should go in before this one. We don't want to add crap
we know about first and fix it afterward, we do it the other way around.
First we remove the crap which is in the way, then we add features.

> >>  	/* Use 500KHz as a suitable default */
> >>  	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
> >> 
> >> @@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct
> >> udevice *bus) plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns",
> >> 20);
> >> 
> >>  	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
> >> 
> >> -	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
> >> -	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
> >> -	      plat->page_size);
> >> -
> >> +	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d
> >> page-size=%d\n", +		__func__, plat->regbase, plat->flashbase,
> >> plat->trigger_base,
> >> +		plat->max_hz, plat->page_size);
> > 
> > Please don't break the indent of the args. The original debug() statement
> > had the args correctly indented ; this patch breaks the indent.
> 
> I think it looks like this here as the line length is more than 80 chars
> for args. After applying patch it looks ok. If not, please let me know how
> to fix it.

I'm talking about the change from 6 spaces to one tab in front of the args.

> >>  	return 0;
> >>  
> >>  }
> >> 
> >> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
> >> index 98e57aa..7341339 100644
> >> --- a/drivers/spi/cadence_qspi.h
> >> +++ b/drivers/spi/cadence_qspi.h
> >> @@ -17,7 +17,8 @@
> >> 
> >>  struct cadence_spi_platdata {
> >>  
> >>  	unsigned int	max_hz;
> >>  	void		*regbase;
> >> 
> >> -	void		*ahbbase;
> >> +	void		*flashbase;
> >> +	void		*trigger_base;
> >> 
> >>  	u32		page_size;
> >>  	u32		block_size;
> >> 
> >> @@ -30,7 +31,7 @@ struct cadence_spi_platdata {
> >> 
> >>  struct cadence_spi_priv {
> >>  
> >>  	void		*regbase;
> >> 
> >> -	void		*ahbbase;
> >> +	void		*flashbase;
> >> 
> >>  	size_t		cmd_len;
> >>  	u8		cmd_buf[32];
> >>  	size_t		data_len;
> >> 
> >> diff --git a/drivers/spi/cadence_qspi_apb.c
> >> b/drivers/spi/cadence_qspi_apb.c index 6b5ae30..25a7ed2 100644
> >> --- a/drivers/spi/cadence_qspi_apb.c
> >> +++ b/drivers/spi/cadence_qspi_apb.c
> >> @@ -44,7 +44,6 @@
> >> 
> >>  #define CQSPI_INST_TYPE_QUAD			(2)
> >>  
> >>  #define CQSPI_STIG_DATA_LEN_MAX			(8)
> >> 
> >> -#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
> >> 
> >>  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
> >>  #define CQSPI_DUMMY_BYTES_MAX			(4)
> >> 
> >> @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct
> >> cadence_spi_platdata *plat, const void *src_addr, unsigned int
> >> num_bytes)
> >> 
> >>  {
> >>  
> >>  	const void *reg_base = plat->regbase;
> >> 
> >> -	void *dest_addr = plat->ahbbase;
> >> +	void *dest_addr = plat->trigger_base;
> >> 
> >>  	unsigned int retry = CQSPI_REG_RETRY;
> >>  	unsigned int sram_level;
> >>  	unsigned int wr_bytes;
> >> 
> >> @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct
> >> cadence_spi_platdata *plat)
> >> 
> >>  	/* Indirect mode configurations */
> >>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
> >> 
> >> -	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
> >> +	writel((u32)plat->trigger_base,
> >> 
> >>  				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
> > 
> > Here you actually changed to logic of the code, which breaks it for
> > SoCFPGA. plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK = 0x0 for
> > SoCFPGA, but now you changed it such that 0xffa00000 is written into the
> > register. Same does apply for all your changes below.
> 
> If we pass 0x00 from device tree, 0x0 would be written in the register. We
> have to check on SOCFpga platform is 0xFFA0_0000 is correct value or only
> 0x0. Anyways i will change the DT value to 0x0 for SOCFpga in next
> version.

Excellent.

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[vikas]

Hi,

On 08/20/2015 02:56 PM, Marek Vasut wrote:
> On Thursday, August 20, 2015 at 06:48:36 PM, vikas wrote:
>> Hi,
>>
>> On 08/19/2015 08:54 PM, Marek Vasut wrote:
>>> On Saturday, August 15, 2015 at 04:15:59 AM, Vikas Manocha wrote:
>>>> This patch is to separate the base trigger from the read/write transfer
>>>> start addresses.
>>>>
>>>> Base trigger register address (0x1c register) corresponds to the address
>>>> which should be put on AHB bus to handle indirect transfer triggered
>>>> before.
>>>>
>>>> To handle indirect transfer we need to issue addresses from (value of
>>>> 0x1c) to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM
>>>> location). There are no obstacles in issuing const address just equal
>>>> to 0x1c. Important thing to note is that indirect trigger address has
>>>> nothing in common with your physical or mapped NOR Flash address.
>>>>
>>>> Transfer read/write start addresses (offset 0x68/0x78)should be
>>>> programmed with the absolute flash address to be read/written.
>>>>
>>>> plat->ahbbase has been renamed to plat->flashbase for clarity.
>>>> plat->triggerbase is added in device tree for mapped spi flash address.
>>>>
>>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
>>>> ---
>>>>
>>>> Changes in v3: formatted string breaking fixed.
>>>> Changes in v2: Rebased to master
>>>>
>>>>  arch/arm/dts/socfpga.dtsi      |    3 ++-
>>>>  arch/arm/dts/stv0991.dts       |    3 ++-
>>>>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
>>>>  drivers/spi/cadence_qspi.h     |    5 +++--
>>>>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
>>>>  5 files changed, 19 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
>>>> index 9b12420..25053ec 100644
>>>> --- a/arch/arm/dts/socfpga.dtsi
>>>> +++ b/arch/arm/dts/socfpga.dtsi
>>>> @@ -633,7 +633,8 @@
>>>>
>>>>  			#address-cells = <1>;
>>>>  			#size-cells = <0>;
>>>>  			reg = <0xff705000 0x1000>,
>>>>
>>>> -				<0xffa00000 0x1000>;
>>>> +				<0xffa00000 0x1000>,
>>>> +				<0xffa00000 0x0010>;
>>>
>>> Can you please follow the DT bindings Graham and me sent for Linux kernel
>>> instead of adding ad-hoc undocumented stuff ?
>>
>> It is aligned to current DT binding of u-boot.
>> I agree to align it to linux kernel DT binding but that should be a
>> separate patch.
> 
> And this separate patch should come in FIRST, we do not want to support two
> sets of bindings, no way.

As you know linux DT binding is under review & current linux patch needs to be fixed. It does not make sense to
support something which is under review & discussion are ongoing for it.

> 
>>>>  			interrupts = <0 151 4>;
>>>>  			clocks = <&qspi_clk>;
>>>>  			ext-decoder = <0>;  /* external decoder */
>>>>
>>>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
>>>> index fa3fd64..e23d4fd 100644
>>>> --- a/arch/arm/dts/stv0991.dts
>>>> +++ b/arch/arm/dts/stv0991.dts
>>>> @@ -30,7 +30,8 @@
>>>>
>>>>  			#address-cells = <1>;
>>>>  			#size-cells = <0>;
>>>>  			reg = <0x80203000 0x100>,
>>>>
>>>> -				<0x40000000 0x1000000>;
>>>> +				<0x40000000 0x1000000>,
>>>> +				<0x40000000 0x0000010>;
>>>>
>>>>  			clocks = <3750000>;
>>>>  			sram-size = <256>;
>>>>  			status = "okay";
>>>>
>>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>>>> index 34a0f46..01eff71 100644
>>>> --- a/drivers/spi/cadence_qspi.c
>>>> +++ b/drivers/spi/cadence_qspi.c
>>>> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
>>>>
>>>>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
>>>>  	
>>>>  	priv->regbase = plat->regbase;
>>>>
>>>> -	priv->ahbbase = plat->ahbbase;
>>>> +	priv->flashbase = plat->flashbase;
>>>
>>> If you need to rename anything, it should be in a separate patch. Please
>>> keep it separate.
>>
>> this renaming is done here for more clarity as the earlier ahbbase usage is
>> split between "flashbase" & "trigger base". ahbbase is not relevant
>> anymore.
> 
> Which is why it should happen in a separate patch :)

Hmmm.. Not adding any value but i will do it.

> 
>>>>  	if (!priv->qspi_is_init) {
>>>>  	
>>>>  		cadence_qspi_apb_controller_init(plat);
>>>>
>>>> @@ -278,7 +278,7 @@ static int cadence_spi_ofdata_to_platdata(struct
>>>> udevice *bus) const void *blob = gd->fdt_blob;
>>>>
>>>>  	int node = bus->of_offset;
>>>>  	int subnode;
>>>>
>>>> -	u32 data[4];
>>>> +	u32 data[6];
>>>>
>>>>  	int ret;
>>>>  	
>>>>  	/* 2 base addresses are needed, lets get them from the DT */
>>>>
>>>> @@ -289,7 +289,8 @@ static int cadence_spi_ofdata_to_platdata(struct
>>>> udevice *bus) }
>>>>
>>>>  	plat->regbase = (void *)data[0];
>>>>
>>>> -	plat->ahbbase = (void *)data[2];
>>>> +	plat->flashbase = (void *)data[2];
>>>> +	plat->trigger_base = (void *)data[4];
>>>
>>> Please switch this to fdtdec_get_addr() instead ; if you use the Linux
>>> binding, you will be able to do this easily.
>>
>> As above, should be separate patch to align to Linux binding.
> 
> Correct ; and it should go in before this one. We don't want to add crap
> we know about first and fix it afterward, we do it the other way around.
> First we remove the crap which is in the way, then we add features.

ditto.

> 
>>>>  	/* Use 500KHz as a suitable default */
>>>>  	plat->max_hz = fdtdec_get_int(blob, node, "spi-max-frequency",
>>>>
>>>> @@ -311,10 +312,9 @@ static int cadence_spi_ofdata_to_platdata(struct
>>>> udevice *bus) plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns",
>>>> 20);
>>>>
>>>>  	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
>>>>
>>>> -	debug("%s: regbase=%p ahbbase=%p max-frequency=%d page-size=%d\n",
>>>> -	      __func__, plat->regbase, plat->ahbbase, plat->max_hz,
>>>> -	      plat->page_size);
>>>> -
>>>> +	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d
>>>> page-size=%d\n", +		__func__, plat->regbase, plat->flashbase,
>>>> plat->trigger_base,
>>>> +		plat->max_hz, plat->page_size);
>>>
>>> Please don't break the indent of the args. The original debug() statement
>>> had the args correctly indented ; this patch breaks the indent.
>>
>> I think it looks like this here as the line length is more than 80 chars
>> for args. After applying patch it looks ok. If not, please let me know how
>> to fix it.
> 
> I'm talking about the change from 6 spaces to one tab in front of the args.

ok.

Cheers,
Vikas

> 
>>>>  	return 0;
>>>>  
>>>>  }
>>>>
>>>> diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
>>>> index 98e57aa..7341339 100644
>>>> --- a/drivers/spi/cadence_qspi.h
>>>> +++ b/drivers/spi/cadence_qspi.h
>>>> @@ -17,7 +17,8 @@
>>>>
>>>>  struct cadence_spi_platdata {
>>>>  
>>>>  	unsigned int	max_hz;
>>>>  	void		*regbase;
>>>>
>>>> -	void		*ahbbase;
>>>> +	void		*flashbase;
>>>> +	void		*trigger_base;
>>>>
>>>>  	u32		page_size;
>>>>  	u32		block_size;
>>>>
>>>> @@ -30,7 +31,7 @@ struct cadence_spi_platdata {
>>>>
>>>>  struct cadence_spi_priv {
>>>>  
>>>>  	void		*regbase;
>>>>
>>>> -	void		*ahbbase;
>>>> +	void		*flashbase;
>>>>
>>>>  	size_t		cmd_len;
>>>>  	u8		cmd_buf[32];
>>>>  	size_t		data_len;
>>>>
>>>> diff --git a/drivers/spi/cadence_qspi_apb.c
>>>> b/drivers/spi/cadence_qspi_apb.c index 6b5ae30..25a7ed2 100644
>>>> --- a/drivers/spi/cadence_qspi_apb.c
>>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>>> @@ -44,7 +44,6 @@
>>>>
>>>>  #define CQSPI_INST_TYPE_QUAD			(2)
>>>>  
>>>>  #define CQSPI_STIG_DATA_LEN_MAX			(8)
>>>>
>>>> -#define CQSPI_INDIRECTTRIGGER_ADDR_MASK		(0xFFFFF)
>>>>
>>>>  #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
>>>>  #define CQSPI_DUMMY_BYTES_MAX			(4)
>>>>
>>>> @@ -281,7 +280,7 @@ static int qpsi_write_sram_fifo_push(struct
>>>> cadence_spi_platdata *plat, const void *src_addr, unsigned int
>>>> num_bytes)
>>>>
>>>>  {
>>>>  
>>>>  	const void *reg_base = plat->regbase;
>>>>
>>>> -	void *dest_addr = plat->ahbbase;
>>>> +	void *dest_addr = plat->trigger_base;
>>>>
>>>>  	unsigned int retry = CQSPI_REG_RETRY;
>>>>  	unsigned int sram_level;
>>>>  	unsigned int wr_bytes;
>>>>
>>>> @@ -534,7 +533,7 @@ void cadence_qspi_apb_controller_init(struct
>>>> cadence_spi_platdata *plat)
>>>>
>>>>  	/* Indirect mode configurations */
>>>>  	writel((plat->sram_size/2), plat->regbase + CQSPI_REG_SRAMPARTITION);
>>>>
>>>> -	writel((u32)plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK,
>>>> +	writel((u32)plat->trigger_base,
>>>>
>>>>  				plat->regbase + CQSPI_REG_INDIRECTTRIGGER);
>>>
>>> Here you actually changed to logic of the code, which breaks it for
>>> SoCFPGA. plat->ahbbase & CQSPI_INDIRECTTRIGGER_ADDR_MASK = 0x0 for
>>> SoCFPGA, but now you changed it such that 0xffa00000 is written into the
>>> register. Same does apply for all your changes below.
>>
>> If we pass 0x00 from device tree, 0x0 would be written in the register. We
>> have to check on SOCFpga platform is 0xFFA0_0000 is correct value or only
>> 0x0. Anyways i will change the DT value to 0x0 for SOCFpga in next
>> version.
> 
> Excellent.
> .
> 

[U-Boot] [v3 3/4] spi: cadence_qspi: fix base trigger address & transfer start address

[Marek Vasut]

On Friday, August 21, 2015 at 12:25:54 AM, vikas wrote:
> Hi,

Hi,

> On 08/20/2015 02:56 PM, Marek Vasut wrote:
> > On Thursday, August 20, 2015 at 06:48:36 PM, vikas wrote:
> >> Hi,
> >> 
> >> On 08/19/2015 08:54 PM, Marek Vasut wrote:
> >>> On Saturday, August 15, 2015 at 04:15:59 AM, Vikas Manocha wrote:
> >>>> This patch is to separate the base trigger from the read/write
> >>>> transfer start addresses.
> >>>> 
> >>>> Base trigger register address (0x1c register) corresponds to the
> >>>> address which should be put on AHB bus to handle indirect transfer
> >>>> triggered before.
> >>>> 
> >>>> To handle indirect transfer we need to issue addresses from (value of
> >>>> 0x1c) to (value of 0x1c) + 15*4 ("4" corresponds to size of SRAM
> >>>> location). There are no obstacles in issuing const address just equal
> >>>> to 0x1c. Important thing to note is that indirect trigger address has
> >>>> nothing in common with your physical or mapped NOR Flash address.
> >>>> 
> >>>> Transfer read/write start addresses (offset 0x68/0x78)should be
> >>>> programmed with the absolute flash address to be read/written.
> >>>> 
> >>>> plat->ahbbase has been renamed to plat->flashbase for clarity.
> >>>> plat->triggerbase is added in device tree for mapped spi flash
> >>>> address.
> >>>> 
> >>>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >>>> ---
> >>>> 
> >>>> Changes in v3: formatted string breaking fixed.
> >>>> Changes in v2: Rebased to master
> >>>> 
> >>>>  arch/arm/dts/socfpga.dtsi      |    3 ++-
> >>>>  arch/arm/dts/stv0991.dts       |    3 ++-
> >>>>  drivers/spi/cadence_qspi.c     |   14 +++++++-------
> >>>>  drivers/spi/cadence_qspi.h     |    5 +++--
> >>>>  drivers/spi/cadence_qspi_apb.c |   11 +++++------
> >>>>  5 files changed, 19 insertions(+), 17 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
> >>>> index 9b12420..25053ec 100644
> >>>> --- a/arch/arm/dts/socfpga.dtsi
> >>>> +++ b/arch/arm/dts/socfpga.dtsi
> >>>> @@ -633,7 +633,8 @@
> >>>> 
> >>>>  			#address-cells = <1>;
> >>>>  			#size-cells = <0>;
> >>>>  			reg = <0xff705000 0x1000>,
> >>>> 
> >>>> -				<0xffa00000 0x1000>;
> >>>> +				<0xffa00000 0x1000>,
> >>>> +				<0xffa00000 0x0010>;
> >>> 
> >>> Can you please follow the DT bindings Graham and me sent for Linux
> >>> kernel instead of adding ad-hoc undocumented stuff ?
> >> 
> >> It is aligned to current DT binding of u-boot.
> >> I agree to align it to linux kernel DT binding but that should be a
> >> separate patch.
> > 
> > And this separate patch should come in FIRST, we do not want to support
> > two sets of bindings, no way.
> 
> As you know linux DT binding is under review & current linux patch needs to
> be fixed. It does not make sense to support something which is under
> review & discussion are ongoing for it.

So you propose that in the meantime, we grow our own, completely different,
in-house set of binding instead of using the ones which are currently being
reviewed for inclusion into Linux kernel. The only comments on those bindings
are from you and are about wording of some piece of text, not even about the
binding themselves.

No, sorry, I'm not buying this.

> >>>>  			interrupts = <0 151 4>;
> >>>>  			clocks = <&qspi_clk>;
> >>>>  			ext-decoder = <0>;  /* external decoder */
> >>>> 
> >>>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
> >>>> index fa3fd64..e23d4fd 100644
> >>>> --- a/arch/arm/dts/stv0991.dts
> >>>> +++ b/arch/arm/dts/stv0991.dts
> >>>> @@ -30,7 +30,8 @@
> >>>> 
> >>>>  			#address-cells = <1>;
> >>>>  			#size-cells = <0>;
> >>>>  			reg = <0x80203000 0x100>,
> >>>> 
> >>>> -				<0x40000000 0x1000000>;
> >>>> +				<0x40000000 0x1000000>,
> >>>> +				<0x40000000 0x0000010>;
> >>>> 
> >>>>  			clocks = <3750000>;
> >>>>  			sram-size = <256>;
> >>>>  			status = "okay";
> >>>> 
> >>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> >>>> index 34a0f46..01eff71 100644
> >>>> --- a/drivers/spi/cadence_qspi.c
> >>>> +++ b/drivers/spi/cadence_qspi.c
> >>>> @@ -150,7 +150,7 @@ static int cadence_spi_probe(struct udevice *bus)
> >>>> 
> >>>>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
> >>>>  	
> >>>>  	priv->regbase = plat->regbase;
> >>>> 
> >>>> -	priv->ahbbase = plat->ahbbase;
> >>>> +	priv->flashbase = plat->flashbase;
> >>> 
> >>> If you need to rename anything, it should be in a separate patch.
> >>> Please keep it separate.
> >> 
> >> this renaming is done here for more clarity as the earlier ahbbase usage
> >> is split between "flashbase" & "trigger base". ahbbase is not relevant
> >> anymore.
> > 
> > Which is why it should happen in a separate patch :)
> 
> Hmmm.. Not adding any value but i will do it.

The value is in keeping separate changes separate, reviewable and bisectable.
You might want to read up on this.

[...]

[U-Boot] [v3 4/4] spi: cadence_qspi: get fifo width from device tree

[Vikas Manocha]

Fifo width could be different on different socs, e.g. stv0991 & altera soc
have different fifo width.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changes in v3: none
Changes in v2: Rebased to master

 arch/arm/dts/socfpga.dtsi      |    1 +
 arch/arm/dts/stv0991.dts       |    1 +
 drivers/spi/cadence_qspi.c     |    1 +
 drivers/spi/cadence_qspi.h     |    1 +
 drivers/spi/cadence_qspi_apb.c |   24 ++++++++++--------------
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi
index 25053ec..265a5b8 100644
--- a/arch/arm/dts/socfpga.dtsi
+++ b/arch/arm/dts/socfpga.dtsi
@@ -640,6 +640,7 @@
 			ext-decoder = <0>;  /* external decoder */
 			num-cs = <4>;
 			fifo-depth = <128>;
+			fifo-width = <4>;
 			sram-size = <128>;
 			bus-num = <2>;
 			status = "disabled";
diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
index e23d4fd..f0f450b 100644
--- a/arch/arm/dts/stv0991.dts
+++ b/arch/arm/dts/stv0991.dts
@@ -34,6 +34,7 @@
 				<0x40000000 0x0000010>;
 			clocks = <3750000>;
 			sram-size = <256>;
+			fifo-width = <8>;
 			status = "okay";
 
 			flash0: n25q32 at 0 {
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 01eff71..54b3c36 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -311,6 +311,7 @@ static int cadence_spi_ofdata_to_platdata(struct udevice *bus)
 	plat->tchsh_ns = fdtdec_get_int(blob, subnode, "tchsh-ns", 20);
 	plat->tslch_ns = fdtdec_get_int(blob, subnode, "tslch-ns", 20);
 	plat->sram_size = fdtdec_get_int(blob, node, "sram-size", 128);
+	plat->fifo_width = fdtdec_get_int(blob, node, "fifo-width", 4);
 
 	debug("%s: regbase=%p flashbase=%p trigger_base=%p max-frequency=%d page-size=%d\n",
 		__func__, plat->regbase, plat->flashbase, plat->trigger_base,
diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h
index 7341339..91f38f1 100644
--- a/drivers/spi/cadence_qspi.h
+++ b/drivers/spi/cadence_qspi.h
@@ -27,6 +27,7 @@ struct cadence_spi_platdata {
 	u32		tchsh_ns;
 	u32		tslch_ns;
 	u32		sram_size;
+	u32		fifo_width;
 };
 
 struct cadence_spi_priv {
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 25a7ed2..e534c06 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -34,8 +34,6 @@
 #define CQSPI_REG_RETRY				(10000)
 #define CQSPI_POLL_IDLE_RETRY			(3)
 
-#define CQSPI_FIFO_WIDTH			(4)
-
 #define CQSPI_REG_SRAM_THRESHOLD_WORDS		(50)
 
 /* Transfer mode */
@@ -48,9 +46,6 @@
 #define CQSPI_DUMMY_CLKS_PER_BYTE		(8)
 #define CQSPI_DUMMY_BYTES_MAX			(4)
 
-
-#define CQSPI_REG_SRAM_FILL_THRESHOLD	\
-	((CQSPI_REG_SRAM_SIZE_WORD / 2) * CQSPI_FIFO_WIDTH)
 /****************************************************************************
  * Controller's configuration and status register (offset from QSPI_BASE)
  ****************************************************************************/
@@ -214,7 +209,7 @@ static void cadence_qspi_apb_read_fifo_data(void *dest,
 }
 
 static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
-	const void *src, unsigned int bytes)
+	const void *src, unsigned int fifo_width, unsigned int bytes)
 {
 	unsigned int temp = 0;
 	int i;
@@ -222,11 +217,11 @@ static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
 	unsigned int *dest_ptr = (unsigned int *)dest_ahb_addr;
 	unsigned int *src_ptr = (unsigned int *)src;
 
-	while (remaining >= CQSPI_FIFO_WIDTH) {
-		for (i = CQSPI_FIFO_WIDTH/sizeof(src_ptr) - 1; i >= 0; i--)
+	while (remaining >= fifo_width) {
+		for (i = fifo_width/sizeof(src_ptr) - 1; i >= 0; i--)
 			writel(*(src_ptr+i), dest_ptr+i);
-		src_ptr += CQSPI_FIFO_WIDTH/sizeof(src_ptr);
-		remaining -= CQSPI_FIFO_WIDTH;
+		src_ptr += fifo_width/sizeof(src_ptr);
+		remaining -= fifo_width;
 	}
 	if (remaining) {
 		/* dangling bytes */
@@ -241,7 +236,7 @@ static void cadence_qspi_apb_write_fifo_data(const void *dest_ahb_addr,
 
 /* Read from SRAM FIFO with polling SRAM fill level. */
 static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr,
-			const void *src_addr,  unsigned int num_bytes)
+	const void *src_addr, unsigned int fifo_width, unsigned int num_bytes)
 {
 	unsigned int remaining = num_bytes;
 	unsigned int retry;
@@ -263,7 +258,7 @@ static int qspi_read_sram_fifo_poll(const void *reg_base, void *dest_addr,
 			return -1;
 		}
 
-		sram_level *= CQSPI_FIFO_WIDTH;
+		sram_level *= fifo_width;
 		sram_level = sram_level > remaining ? remaining : sram_level;
 
 		/* Read data from FIFO. */
@@ -305,7 +300,8 @@ static int qpsi_write_sram_fifo_push(struct cadence_spi_platdata *plat,
 		wr_bytes = (remaining > page_size) ?
 					page_size : remaining;
 
-		cadence_qspi_apb_write_fifo_data(dest_addr, src, wr_bytes);
+		cadence_qspi_apb_write_fifo_data(dest_addr, src, \
+				plat->fifo_width, wr_bytes);
 		src += wr_bytes;
 		remaining -= wr_bytes;
 	}
@@ -752,7 +748,7 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 
 	if (qspi_read_sram_fifo_poll(plat->regbase, (void *)rxbuf,
-				     (const void *)plat->trigger_base, rxlen))
+		(const void *)plat->trigger_base, plat->fifo_width, rxlen))
 		goto failrd;
 
 	/* Check flash indirect controller */
-- 
1.7.9.5

[U-Boot] [v3 4/4] spi: cadence_qspi: get fifo width from device tree

[Marek Vasut]

On Saturday, August 15, 2015 at 04:16:00 AM, Vikas Manocha wrote:
> Fifo width could be different on different socs, e.g. stv0991 & altera soc
> have different fifo width.
> 
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> ---
> 
> Changes in v3: none
> Changes in v2: Rebased to master
> 
>  arch/arm/dts/socfpga.dtsi      |    1 +
>  arch/arm/dts/stv0991.dts       |    1 +
>  drivers/spi/cadence_qspi.c     |    1 +
>  drivers/spi/cadence_qspi.h     |    1 +
>  drivers/spi/cadence_qspi_apb.c |   24 ++++++++++--------------
>  5 files changed, 14 insertions(+), 14 deletions(-)

Please use the Linux bindings, not this ad-hoc stuff.

Best regards,
Marek Vasut