[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Vignesh R]

This series reverts use of bounce_buf.c for non-DMA related alignment
restriction and replaces it with local bounce buffer to handle problems
with non 32 bit aligned writes on TI platforms.
Based on top of Jason's series:
https://patchwork.ozlabs.org/cover/856431/

Tested on K2G EVM.

Goldschmidt Simon (1):
  Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction
    when possible"

Vignesh R (2):
  Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction
    when possible"
  spi: cadence_qspi_apb: Make flash writes 32 bit aligned

 drivers/spi/cadence_qspi_apb.c   | 49 ++++++++++++++++++----------------------
 include/configs/k2g_evm.h        |  1 -
 include/configs/socfpga_common.h |  1 -
 include/configs/stv0991.h        |  1 -
 4 files changed, 22 insertions(+), 30 deletions(-)

-- 
2.15.1

[U-Boot] [PATCH v2 1/3] Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible"

[Vignesh R]

From: Goldschmidt Simon <sgoldschmidt@de.pepperl-fuchs.com>

This reverts commit b63b46313ed29e9b0c36b3d6b9407f6eade40c8f.

This commit changed cadence_qspi_apb to use bouncebuf.c, which invalidates
the data cache after reading. This is meant for dma transfers only and
breaks the cadence_qspi driver which copies via cpu only: data that is
copied by the cpu is in cache only and the cache invalidation at the end
throws away this data.

Signed-off-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/cadence_qspi_apb.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 8309ab87940c..c7cb33aa8fc3 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -627,8 +627,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 {
 	unsigned int remaining = n_rx;
 	unsigned int bytes_to_read = 0;
-	struct bounce_buffer bb;
-	u8 *bb_rxbuf;
 	int ret;
 
 	writel(n_rx, plat->regbase + CQSPI_REG_INDIRECTRDBYTES);
@@ -637,11 +635,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 	writel(CQSPI_REG_INDIRECTRD_START,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
 
-	ret = bounce_buffer_start(&bb, (void *)rxbuf, n_rx, GEN_BB_WRITE);
-	if (ret)
-		return ret;
-	bb_rxbuf = bb.bounce_buffer;
-
 	while (remaining > 0) {
 		ret = cadence_qspi_wait_for_data(plat);
 		if (ret < 0) {
@@ -655,13 +648,12 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 			bytes_to_read *= plat->fifo_width;
 			bytes_to_read = bytes_to_read > remaining ?
 					remaining : bytes_to_read;
-			readsl(plat->ahbbase, bb_rxbuf, bytes_to_read >> 2);
-			if (bytes_to_read % 4)
-				readsb(plat->ahbbase,
-				       bb_rxbuf + rounddown(bytes_to_read, 4),
-				       bytes_to_read % 4);
-
-			bb_rxbuf += bytes_to_read;
+			/* Handle non-4-byte aligned access to avoid data abort. */
+			if (((uintptr_t)rxbuf % 4) || (bytes_to_read % 4))
+				readsb(plat->ahbbase, rxbuf, bytes_to_read);
+			else
+				readsl(plat->ahbbase, rxbuf, bytes_to_read >> 2);
+			rxbuf += bytes_to_read;
 			remaining -= bytes_to_read;
 			bytes_to_read = cadence_qspi_get_rd_sram_level(plat);
 		}
@@ -678,7 +670,6 @@ int cadence_qspi_apb_indirect_read_execute(struct cadence_spi_platdata *plat,
 	/* Clear indirect completion status */
 	writel(CQSPI_REG_INDIRECTRD_DONE,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
-	bounce_buffer_stop(&bb);
 
 	return 0;
 
@@ -686,7 +677,6 @@ failrd:
 	/* Cancel the indirect read */
 	writel(CQSPI_REG_INDIRECTRD_CANCEL,
 	       plat->regbase + CQSPI_REG_INDIRECTRD);
-	bounce_buffer_stop(&bb);
 	return ret;
 }
 
-- 
2.15.1

[U-Boot] [PATCH v2 2/3] Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible"

[Vignesh R]

This reverts commit 57897c13de03ac0136d64641a3eab526c6810387.

Using bounce_buf.c to handle non-DMA alignment problems is bad as
bounce_buf.c does cache manipulations which is not required. Therefore
revert this patch in favour of local bounce buffer solution in the next
patch.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---
 drivers/spi/cadence_qspi_apb.c   | 26 ++++++--------------------
 include/configs/k2g_evm.h        |  1 -
 include/configs/socfpga_common.h |  1 -
 include/configs/stv0991.h        |  1 -
 4 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index c7cb33aa8fc3..fa8af33ae19b 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -30,7 +30,6 @@
 #include <linux/errno.h>
 #include <wait_bit.h>
 #include <spi.h>
-#include <bouncebuf.h>
 #include "cadence_qspi.h"
 
 #define CQSPI_REG_POLL_US			1 /* 1us */
@@ -718,17 +717,6 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 	unsigned int remaining = n_tx;
 	unsigned int write_bytes;
 	int ret;
-	struct bounce_buffer bb;
-	u8 *bb_txbuf;
-
-	/*
-	 * Handle non-4-byte aligned accesses via bounce buffer to
-	 * avoid data abort.
-	 */
-	ret = bounce_buffer_start(&bb, (void *)txbuf, n_tx, GEN_BB_READ);
-	if (ret)
-		return ret;
-	bb_txbuf = bb.bounce_buffer;
 
 	/* Configure the indirect read transfer bytes */
 	writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
@@ -739,11 +727,11 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 
 	while (remaining > 0) {
 		write_bytes = remaining > page_size ? page_size : remaining;
-		writesl(plat->ahbbase, bb_txbuf, write_bytes >> 2);
-		if (write_bytes % 4)
-			writesb(plat->ahbbase,
-				bb_txbuf + rounddown(write_bytes, 4),
-				write_bytes % 4);
+		/* Handle non-4-byte aligned access to avoid data abort. */
+		if (((uintptr_t)txbuf % 4) || (write_bytes % 4))
+			writesb(plat->ahbbase, txbuf, write_bytes);
+		else
+			writesl(plat->ahbbase, txbuf, write_bytes >> 2);
 
 		ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL,
 				   CQSPI_REG_SDRAMLEVEL_WR_MASK <<
@@ -753,7 +741,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 			goto failwr;
 		}
 
-		bb_txbuf += write_bytes;
+		txbuf += write_bytes;
 		remaining -= write_bytes;
 	}
 
@@ -764,7 +752,6 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 		printf("Indirect write completion error (%i)\n", ret);
 		goto failwr;
 	}
-	bounce_buffer_stop(&bb);
 
 	/* Clear indirect completion status */
 	writel(CQSPI_REG_INDIRECTWR_DONE,
@@ -775,7 +762,6 @@ failwr:
 	/* Cancel the indirect write */
 	writel(CQSPI_REG_INDIRECTWR_CANCEL,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
-	bounce_buffer_stop(&bb);
 	return ret;
 }
 
diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h
index 535e7124fc80..0a38922a519e 100644
--- a/include/configs/k2g_evm.h
+++ b/include/configs/k2g_evm.h
@@ -93,7 +93,6 @@
 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_CADENCE_QSPI
 #define CONFIG_CQSPI_REF_CLK 384000000
-#define CONFIG_BOUNCE_BUFFER
 #endif
 
 #define SPI_MTD_PARTS	KEYSTONE_SPI1_MTD_PARTS
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
index ec8bb500504a..f6607b101ec5 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -184,7 +184,6 @@ unsigned int cm_get_l4_sp_clk_hz(void);
 unsigned int cm_get_qspi_controller_clk_hz(void);
 #define CONFIG_CQSPI_REF_CLK		cm_get_qspi_controller_clk_hz()
 #endif
-#define CONFIG_BOUNCE_BUFFER
 
 /*
  * Designware SPI support
diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
index fd96979bf897..beb8f1ae9a92 100644
--- a/include/configs/stv0991.h
+++ b/include/configs/stv0991.h
@@ -64,7 +64,6 @@
 + */
 #ifdef CONFIG_OF_CONTROL		/* QSPI is controlled via DT */
 #define CONFIG_CQSPI_REF_CLK		((30/4)/2)*1000*1000
-#define CONFIG_BOUNCE_BUFFER
 
 #endif
 
-- 
2.15.1

[U-Boot] [PATCH v2 3/3] spi: cadence_qspi_apb: Make flash writes 32 bit aligned

[Vignesh R]

Make flash writes 32 bit aligned by using bounce buffers to deal with
non 32 bit aligned buffers.
This is required because as per TI K2G TRM[1], the external master is
only permitted to issue 32-bit data interface writes until the last word
of an indirect transfer. Otherwise indirect writes is known to fail
sometimes.

[1] http://www.ti.com/lit/ug/spruhy8g/spruhy8g.pdf

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v2: Move bounce buffer within cadence_qspi_apb_indirect_write_execute()
    to handle arbitrary length buffers.


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

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index fa8af33ae19b..23441964a543 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -30,6 +30,7 @@
 #include <linux/errno.h>
 #include <wait_bit.h>
 #include <spi.h>
+#include <malloc.h>
 #include "cadence_qspi.h"
 
 #define CQSPI_REG_POLL_US			1 /* 1us */
@@ -715,9 +716,23 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 {
 	unsigned int page_size = plat->page_size;
 	unsigned int remaining = n_tx;
+	const u8 *bb_txbuf = txbuf;
+	void *bounce_buf = NULL;
 	unsigned int write_bytes;
 	int ret;
 
+	/*
+	 * Use bounce buffer for non 32 bit aligned txbuf to avoid data
+	 * aborts
+	 */
+	if ((uintptr_t)txbuf % 4) {
+		bounce_buf = malloc(n_tx);
+		if (!bounce_buf)
+			return -ENOMEM;
+		memcpy(bounce_buf, txbuf, n_tx);
+		bb_txbuf = bounce_buf;
+	}
+
 	/* Configure the indirect read transfer bytes */
 	writel(n_tx, plat->regbase + CQSPI_REG_INDIRECTWRBYTES);
 
@@ -727,11 +742,11 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 
 	while (remaining > 0) {
 		write_bytes = remaining > page_size ? page_size : remaining;
-		/* Handle non-4-byte aligned access to avoid data abort. */
-		if (((uintptr_t)txbuf % 4) || (write_bytes % 4))
-			writesb(plat->ahbbase, txbuf, write_bytes);
-		else
-			writesl(plat->ahbbase, txbuf, write_bytes >> 2);
+		writesl(plat->ahbbase, bb_txbuf, write_bytes >> 2);
+		if (write_bytes % 4)
+			writesb(plat->ahbbase,
+				bb_txbuf + rounddown(write_bytes, 4),
+				write_bytes % 4);
 
 		ret = wait_for_bit("QSPI", plat->regbase + CQSPI_REG_SDRAMLEVEL,
 				   CQSPI_REG_SDRAMLEVEL_WR_MASK <<
@@ -741,7 +756,7 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 			goto failwr;
 		}
 
-		txbuf += write_bytes;
+		bb_txbuf += write_bytes;
 		remaining -= write_bytes;
 	}
 
@@ -756,12 +771,16 @@ int cadence_qspi_apb_indirect_write_execute(struct cadence_spi_platdata *plat,
 	/* Clear indirect completion status */
 	writel(CQSPI_REG_INDIRECTWR_DONE,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
+	if (bounce_buf)
+		free(bounce_buf);
 	return 0;
 
 failwr:
 	/* Cancel the indirect write */
 	writel(CQSPI_REG_INDIRECTWR_CANCEL,
 	       plat->regbase + CQSPI_REG_INDIRECTWR);
+	if (bounce_buf)
+		free(bounce_buf);
 	return ret;
 }
 
-- 
2.15.1

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Vignesh R]

Marek,

On 09-Jan-18 6:49 PM, Vignesh R wrote:
> This series reverts use of bounce_buf.c for non-DMA related alignment
> restriction and replaces it with local bounce buffer to handle problems
> with non 32 bit aligned writes on TI platforms.
> Based on top of Jason's series:
> https://patchwork.ozlabs.org/cover/856431/
> 
> Tested on K2G EVM.
> 

Could you ack this series, if you are okay with the changes?

Jagan,
Could you pick this up(along with the above dependent patches) for
2018.03 once Marek's Ack is in place?

> Goldschmidt Simon (1):
>   Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction
>     when possible"
> 
> Vignesh R (2):
>   Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction
>     when possible"
>   spi: cadence_qspi_apb: Make flash writes 32 bit aligned
> 
>  drivers/spi/cadence_qspi_apb.c   | 49 ++++++++++++++++++----------------------
>  include/configs/k2g_evm.h        |  1 -
>  include/configs/socfpga_common.h |  1 -
>  include/configs/stv0991.h        |  1 -
>  4 files changed, 22 insertions(+), 30 deletions(-)
> 

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Marek Vasut]

On 01/15/2018 12:36 PM, Vignesh R wrote:
> Marek,
> 
> On 09-Jan-18 6:49 PM, Vignesh R wrote:
>> This series reverts use of bounce_buf.c for non-DMA related alignment
>> restriction and replaces it with local bounce buffer to handle problems
>> with non 32 bit aligned writes on TI platforms.
>> Based on top of Jason's series:
>> https://patchwork.ozlabs.org/cover/856431/
>>
>> Tested on K2G EVM.
>>
> 
> Could you ack this series, if you are okay with the changes?
> 
> Jagan,
> Could you pick this up(along with the above dependent patches) for
> 2018.03 once Marek's Ack is in place?
> 

Acked-by: Marek Vasut <marex@denx.de>

although you should get ack from Jason and Simon instead, those matter.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Simon Goldschmidt]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8-sig", Size: 2164 bytes --]


Pepperl+Fuchs GmbH, Mannheim
Geschaeftsfuehrer/Managing Directors: Dr.-Ing. Gunther Kegel (Vors./CEO), Werner Guthier, Mehmet Hatiboglu
Vorsitzender des Aufsichtsrats/Chairman of the supervisory board: Claus Michael
Registergericht/Register Court: AG Mannheim HRB 4713
On 15.01.2018 12:45, Marek Vasut wrote:
> On 01/15/2018 12:36 PM, Vignesh R wrote:
>> Marek,
>>
>> On 09-Jan-18 6:49 PM, Vignesh R wrote:
>>> This series reverts use of bounce_buf.c for non-DMA related alignment
>>> restriction and replaces it with local bounce buffer to handle problems
>>> with non 32 bit aligned writes on TI platforms.
>>> Based on top of Jason's series:
>>> https://patchwork.ozlabs.org/cover/856431/
>>>
>>> Tested on K2G EVM.
>>>
>> Could you ack this series, if you are okay with the changes?
>>
>> Jagan,
>> Could you pick this up(along with the above dependent patches) for
>> 2018.03 once Marek's Ack is in place?
>>
> Acked-by: Marek Vasut <marex@denx.de>
>
> although you should get ack from Jason and Simon instead, those matter.
>

Acked-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>

If that "Simon" above was me :-) I thought ack was sent by maintainers 
and TB by me, sorry.

Simon

Wichtiger Hinweis:
Diese E-Mail einschliesslich ihrer Anhaenge enthaelt vertrauliche und rechtlich geschuetzte Informationen, die nur fuer den Adressaten bestimmt sind. 
Sollten Sie nicht der bezeichnete Adressat sein, so teilen Sie dies bitte dem Absender umgehend mit und loeschen Sie diese Nachricht und ihre Anhaenge. Die unbefugte Weitergabe, das Anfertigen von Kopien und jede Veraenderung der E-Mail ist untersagt. Der Absender haftet nicht fuer Inhalte von veraenderten E-Mails.


Important Information:
This e-mail message including its attachments contains confidential and legally protected information solely intended for the addressee. If you are not the intended addressee of this message, please contact the addresser immediately and delete this message including its attachments. The unauthorized dissemination, copying and change of this e-mail are strictly forbidden. The addresser shall not be liable for the content of such changed e-mails.

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Jason Rush]

On 1/15/2018 6:01 AM, Simon Goldschmidt wrote:
>
> Pepperl+Fuchs GmbH, Mannheim
> Geschaeftsfuehrer/Managing Directors: Dr.-Ing. Gunther Kegel (Vors./CEO), Werner Guthier, Mehmet Hatiboglu
> Vorsitzender des Aufsichtsrats/Chairman of the supervisory board: Claus Michael
> Registergericht/Register Court: AG Mannheim HRB 4713
> On 15.01.2018 12:45, Marek Vasut wrote:
>> On 01/15/2018 12:36 PM, Vignesh R wrote:
>>> Marek,
>>>
>>> On 09-Jan-18 6:49 PM, Vignesh R wrote:
>>>> This series reverts use of bounce_buf.c for non-DMA related alignment
>>>> restriction and replaces it with local bounce buffer to handle problems
>>>> with non 32 bit aligned writes on TI platforms.
>>>> Based on top of Jason's series:
>>>> https://patchwork.ozlabs.org/cover/856431/
>>>>
>>>> Tested on K2G EVM.
>>>>
>>> Could you ack this series, if you are okay with the changes?
>>>
>>> Jagan,
>>> Could you pick this up(along with the above dependent patches) for
>>> 2018.03 once Marek's Ack is in place?
>>>
>> Acked-by: Marek Vasut <marex@denx.de>
>>
>> although you should get ack from Jason and Simon instead, those matter.
>>
>
> Acked-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
>
> If that "Simon" above was me :-) I thought ack was sent by maintainers and TB by me, sorry.
>
> Simon
>
> Wichtiger Hinweis:
> Diese E-Mail einschliesslich ihrer Anhaenge enthaelt vertrauliche und rechtlich geschuetzte Informationen, die nur fuer den Adressaten bestimmt sind. Sollten Sie nicht der bezeichnete Adressat sein, so teilen Sie dies bitte dem Absender umgehend mit und loeschen Sie diese Nachricht und ihre Anhaenge. Die unbefugte Weitergabe, das Anfertigen von Kopien und jede Veraenderung der E-Mail ist untersagt. Der Absender haftet nicht fuer Inhalte von veraenderten E-Mails.
>
>
> Important Information:
> This e-mail message including its attachments contains confidential and legally protected information solely intended for the addressee. If you are not the intended addressee of this message, please contact the addresser immediately and delete this message including its attachments. The unauthorized dissemination, copying and change of this e-mail are strictly forbidden. The addresser shall not be liable for the content of such changed e-mails.

Looks good to me.  Thanks Vignesh.

Reviewed-by: Jason Rush <jarush@gmail.com>
Acked-by: Jason Rush <jarush@gmail.com>

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Simon Goldschmidt]

Jagan,


the merge window is now less than 1 week for 2018.03 if I'm not 
mistaken. Can you apply this, please?


Thanks,

Simon


On 15.01.2018 14:47, Jason Rush wrote:
> On 1/15/2018 6:01 AM, Simon Goldschmidt wrote:
>> Pepperl+Fuchs GmbH, Mannheim
>> Geschaeftsfuehrer/Managing Directors: Dr.-Ing. Gunther Kegel (Vors./CEO), Werner Guthier, Mehmet Hatiboglu
>> Vorsitzender des Aufsichtsrats/Chairman of the supervisory board: Claus Michael
>> Registergericht/Register Court: AG Mannheim HRB 4713
>> On 15.01.2018 12:45, Marek Vasut wrote:
>>> On 01/15/2018 12:36 PM, Vignesh R wrote:
>>>> Marek,
>>>>
>>>> On 09-Jan-18 6:49 PM, Vignesh R wrote:
>>>>> This series reverts use of bounce_buf.c for non-DMA related alignment
>>>>> restriction and replaces it with local bounce buffer to handle problems
>>>>> with non 32 bit aligned writes on TI platforms.
>>>>> Based on top of Jason's series:
>>>>> https://patchwork.ozlabs.org/cover/856431/
>>>>>
>>>>> Tested on K2G EVM.
>>>>>
>>>> Could you ack this series, if you are okay with the changes?
>>>>
>>>> Jagan,
>>>> Could you pick this up(along with the above dependent patches) for
>>>> 2018.03 once Marek's Ack is in place?
>>>>
>>> Acked-by: Marek Vasut <marex@denx.de>
>>>
>>> although you should get ack from Jason and Simon instead, those matter.
>>>
>> Acked-by: Simon Goldschmidt <sgoldschmidt@de.pepperl-fuchs.com>
>>
>> If that "Simon" above was me :-) I thought ack was sent by maintainers and TB by me, sorry.
>>
>> Simon
>>
>> Wichtiger Hinweis:
>> Diese E-Mail einschliesslich ihrer Anhaenge enthaelt vertrauliche und rechtlich geschuetzte Informationen, die nur fuer den Adressaten bestimmt sind. Sollten Sie nicht der bezeichnete Adressat sein, so teilen Sie dies bitte dem Absender umgehend mit und loeschen Sie diese Nachricht und ihre Anhaenge. Die unbefugte Weitergabe, das Anfertigen von Kopien und jede Veraenderung der E-Mail ist untersagt. Der Absender haftet nicht fuer Inhalte von veraenderten E-Mails.
>>
>>
>> Important Information:
>> This e-mail message including its attachments contains confidential and legally protected information solely intended for the addressee. If you are not the intended addressee of this message, please contact the addresser immediately and delete this message including its attachments. The unauthorized dissemination, copying and change of this e-mail are strictly forbidden. The addresser shall not be liable for the content of such changed e-mails.
> Looks good to me.  Thanks Vignesh.
>
> Reviewed-by: Jason Rush <jarush@gmail.com>
> Acked-by: Jason Rush <jarush@gmail.com>
>

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Jagan Teki]

On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> Jagan,
>
>
> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
> Can you apply this, please?

Can you resend these on-top of u-boot-spi/master, have few changes on
wait_for_bit good to test those as well.

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Vignesh R]

On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>> Jagan,
>>
>>
>> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
>> Can you apply this, please?
> 
> Can you resend these on-top of u-boot-spi/master, have few changes on
> wait_for_bit good to test those as well.

u-boot-spi/master throws few warnings when I build for my platform:
~/workspace/u-boot:0f520af57c60:~ make distclean;make k2g_evm_defconfig;make -j4 -s;

In file included from drivers/spi/cadence_qspi_apb.c:31:0:
include/wait_bit.h: In function 'wait_for_bit_be16':
include/wait_bit.h:76:31: warning: implicit declaration of function 'readw_be' [-Wimplicit-function-declaration]
 BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
                               ^
include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
   val = read(reg);     \
         ^~~~
include/wait_bit.h: In function 'wait_for_bit_be32':
include/wait_bit.h:78:31: warning: implicit declaration of function 'readl_be' [-Wimplicit-function-declaration]
 BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
                               ^
include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
   val = read(reg);     \
         ^~~~


-- 
Regards
Vignesh

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Jagan Teki]

On Tue, Jan 23, 2018 at 2:32 PM, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
>> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>> Jagan,
>>>
>>>
>>> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
>>> Can you apply this, please?
>>
>> Can you resend these on-top of u-boot-spi/master, have few changes on
>> wait_for_bit good to test those as well.
>
> u-boot-spi/master throws few warnings when I build for my platform:
> ~/workspace/u-boot:0f520af57c60:~ make distclean;make k2g_evm_defconfig;make -j4 -s;
>
> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
> include/wait_bit.h: In function 'wait_for_bit_be16':
> include/wait_bit.h:76:31: warning: implicit declaration of function 'readw_be' [-Wimplicit-function-declaration]
>  BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>                                ^
> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>    val = read(reg);     \
>          ^~~~
> include/wait_bit.h: In function 'wait_for_bit_be32':
> include/wait_bit.h:78:31: warning: implicit declaration of function 'readl_be' [-Wimplicit-function-declaration]
>  BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>                                ^
> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>    val = read(reg);     \
>          ^~~~

Did you change wait_for_bit function with _le32? [1]

[1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Vignesh R]

On Tuesday 23 January 2018 02:37 PM, Jagan Teki wrote:
> On Tue, Jan 23, 2018 at 2:32 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>>
>> On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
>>> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>> Jagan,
>>>>
>>>>
>>>> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
>>>> Can you apply this, please?
>>>
>>> Can you resend these on-top of u-boot-spi/master, have few changes on
>>> wait_for_bit good to test those as well.
>>
>> u-boot-spi/master throws few warnings when I build for my platform:
>> ~/workspace/u-boot:0f520af57c60:~ make distclean;make k2g_evm_defconfig;make -j4 -s;
>>
>> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
>> include/wait_bit.h: In function 'wait_for_bit_be16':
>> include/wait_bit.h:76:31: warning: implicit declaration of function 'readw_be' [-Wimplicit-function-declaration]
>>  BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>                                ^
>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>    val = read(reg);     \
>>          ^~~~
>> include/wait_bit.h: In function 'wait_for_bit_be32':
>> include/wait_bit.h:78:31: warning: implicit declaration of function 'readl_be' [-Wimplicit-function-declaration]
>>  BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>                                ^
>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>    val = read(reg);     \
>>          ^~~~
> 
> Did you change wait_for_bit function with _le32? [1]
> 
> [1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8
> 

Yes, I am building plain u-boot-spi/master(HEAD: 0f520af57c60f3baba8
("spi: kirkwood_spi: implement workaround for FE-9144572")). So, above
commit is part of the tree.

Problem is above commit adds
BUILD_WAIT_FOR_BIT(be32, u32, read*_be) variants.
But, readl_be and readw_be seems to be undefined for ARM?


-- 
Regards
Vignesh

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Jagan Teki]

On Tue, Jan 23, 2018 at 2:46 PM, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On Tuesday 23 January 2018 02:37 PM, Jagan Teki wrote:
>> On Tue, Jan 23, 2018 at 2:32 PM, Vignesh R <vigneshr@ti.com> wrote:
>>>
>>>
>>> On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
>>>> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>> Jagan,
>>>>>
>>>>>
>>>>> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
>>>>> Can you apply this, please?
>>>>
>>>> Can you resend these on-top of u-boot-spi/master, have few changes on
>>>> wait_for_bit good to test those as well.
>>>
>>> u-boot-spi/master throws few warnings when I build for my platform:
>>> ~/workspace/u-boot:0f520af57c60:~ make distclean;make k2g_evm_defconfig;make -j4 -s;
>>>
>>> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
>>> include/wait_bit.h: In function 'wait_for_bit_be16':
>>> include/wait_bit.h:76:31: warning: implicit declaration of function 'readw_be' [-Wimplicit-function-declaration]
>>>  BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>>                                ^
>>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>>    val = read(reg);     \
>>>          ^~~~
>>> include/wait_bit.h: In function 'wait_for_bit_be32':
>>> include/wait_bit.h:78:31: warning: implicit declaration of function 'readl_be' [-Wimplicit-function-declaration]
>>>  BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>>                                ^
>>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>>    val = read(reg);     \
>>>          ^~~~
>>
>> Did you change wait_for_bit function with _le32? [1]
>>
>> [1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8
>>
>
> Yes, I am building plain u-boot-spi/master(HEAD: 0f520af57c60f3baba8
> ("spi: kirkwood_spi: implement workaround for FE-9144572")). So, above
> commit is part of the tree.
>
> Problem is above commit adds
> BUILD_WAIT_FOR_BIT(be32, u32, read*_be) variants.
> But, readl_be and readw_be seems to be undefined for ARM?

yes, but wait_for_bit_le32 work for arm?

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Simon Goldschmidt]

On 23.01.2018 10:16, Vignesh R wrote:
>
> On Tuesday 23 January 2018 02:37 PM, Jagan Teki wrote:
>> On Tue, Jan 23, 2018 at 2:32 PM, Vignesh R <vigneshr@ti.com> wrote:
>>>
>>> On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
>>>> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>> Jagan,
>>>>>
>>>>>
>>>>> the merge window is now less than 1 week for 2018.03 if I'm not mistaken.
>>>>> Can you apply this, please?
>>>> Can you resend these on-top of u-boot-spi/master, have few changes on
>>>> wait_for_bit good to test those as well.
>>> u-boot-spi/master throws few warnings when I build for my platform:
>>> ~/workspace/u-boot:0f520af57c60:~ make distclean;make k2g_evm_defconfig;make -j4 -s;
>>>
>>> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
>>> include/wait_bit.h: In function 'wait_for_bit_be16':
>>> include/wait_bit.h:76:31: warning: implicit declaration of function 'readw_be' [-Wimplicit-function-declaration]
>>>   BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>>                                 ^
>>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>>     val = read(reg);     \
>>>           ^~~~
>>> include/wait_bit.h: In function 'wait_for_bit_be32':
>>> include/wait_bit.h:78:31: warning: implicit declaration of function 'readl_be' [-Wimplicit-function-declaration]
>>>   BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>>                                 ^
>>> include/wait_bit.h:48:9: note: in definition of macro 'BUILD_WAIT_FOR_BIT'
>>>     val = read(reg);     \
>>>           ^~~~
>> Did you change wait_for_bit function with _le32? [1]
>>
>> [1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8
>>
> Yes, I am building plain u-boot-spi/master(HEAD: 0f520af57c60f3baba8
> ("spi: kirkwood_spi: implement workaround for FE-9144572")). So, above
> commit is part of the tree.
>
> Problem is above commit adds
> BUILD_WAIT_FOR_BIT(be32, u32, read*_be) variants.
> But, readl_be and readw_be seems to be undefined for ARM?
>
>

Vignesh, I think there's an ongoing discussion about this on the u-boot 
list to fix this?

Regards,
Simon

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Jagan Teki]

On Tue, Jan 23, 2018 at 2:56 PM, Simon Goldschmidt
<sgoldschmidt@de.pepperl-fuchs.com> wrote:
> On 23.01.2018 10:16, Vignesh R wrote:
>>
>>
>> On Tuesday 23 January 2018 02:37 PM, Jagan Teki wrote:
>>>
>>> On Tue, Jan 23, 2018 at 2:32 PM, Vignesh R <vigneshr@ti.com> wrote:
>>>>
>>>>
>>>> On Tuesday 23 January 2018 02:15 PM, Jagan Teki wrote:
>>>>>
>>>>> On Tue, Jan 23, 2018 at 1:42 PM, Simon Goldschmidt
>>>>> <sgoldschmidt@de.pepperl-fuchs.com> wrote:
>>>>>>
>>>>>> Jagan,
>>>>>>
>>>>>>
>>>>>> the merge window is now less than 1 week for 2018.03 if I'm not
>>>>>> mistaken.
>>>>>> Can you apply this, please?
>>>>>
>>>>> Can you resend these on-top of u-boot-spi/master, have few changes on
>>>>> wait_for_bit good to test those as well.
>>>>
>>>> u-boot-spi/master throws few warnings when I build for my platform:
>>>> ~/workspace/u-boot:0f520af57c60:~ make distclean;make
>>>> k2g_evm_defconfig;make -j4 -s;
>>>>
>>>> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
>>>> include/wait_bit.h: In function 'wait_for_bit_be16':
>>>> include/wait_bit.h:76:31: warning: implicit declaration of function
>>>> 'readw_be' [-Wimplicit-function-declaration]
>>>>   BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>>>                                 ^
>>>> include/wait_bit.h:48:9: note: in definition of macro
>>>> 'BUILD_WAIT_FOR_BIT'
>>>>     val = read(reg);     \
>>>>           ^~~~
>>>> include/wait_bit.h: In function 'wait_for_bit_be32':
>>>> include/wait_bit.h:78:31: warning: implicit declaration of function
>>>> 'readl_be' [-Wimplicit-function-declaration]
>>>>   BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>>>                                 ^
>>>> include/wait_bit.h:48:9: note: in definition of macro
>>>> 'BUILD_WAIT_FOR_BIT'
>>>>     val = read(reg);     \
>>>>           ^~~~
>>>
>>> Did you change wait_for_bit function with _le32? [1]
>>>
>>> [1]
>>> http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8
>>>
>> Yes, I am building plain u-boot-spi/master(HEAD: 0f520af57c60f3baba8
>> ("spi: kirkwood_spi: implement workaround for FE-9144572")). So, above
>> commit is part of the tree.
>>
>> Problem is above commit adds
>> BUILD_WAIT_FOR_BIT(be32, u32, read*_be) variants.
>> But, readl_be and readw_be seems to be undefined for ARM?
>>
>>
>
> Vignesh, I think there's an ongoing discussion about this on the u-boot list
> to fix this?

didn't get what issue Vignesh, pointing at but with new wait_for_bit
need a fix[2] to resolve warnings.

[2] https://gist.github.com/Noltari/3e6ed4648b87484c73ca22e2f533f9b0

[U-Boot] [PATCH v2 0/3] cadence-quadspi: Fix issues with non 32bit aligned accesses

[Vignesh R]

On Tuesday 23 January 2018 03:01 PM, Jagan Teki wrote:
[...]
>>>>> u-boot-spi/master throws few warnings when I build for my platform:
>>>>> ~/workspace/u-boot:0f520af57c60:~ make distclean;make
>>>>> k2g_evm_defconfig;make -j4 -s;
>>>>>
>>>>> In file included from drivers/spi/cadence_qspi_apb.c:31:0:
>>>>> include/wait_bit.h: In function 'wait_for_bit_be16':
>>>>> include/wait_bit.h:76:31: warning: implicit declaration of function
>>>>> 'readw_be' [-Wimplicit-function-declaration]
>>>>>   BUILD_WAIT_FOR_BIT(be16, u16, readw_be)
>>>>>                                 ^
>>>>> include/wait_bit.h:48:9: note: in definition of macro
>>>>> 'BUILD_WAIT_FOR_BIT'
>>>>>     val = read(reg);     \
>>>>>           ^~~~
>>>>> include/wait_bit.h: In function 'wait_for_bit_be32':
>>>>> include/wait_bit.h:78:31: warning: implicit declaration of function
>>>>> 'readl_be' [-Wimplicit-function-declaration]
>>>>>   BUILD_WAIT_FOR_BIT(be32, u32, readl_be)
>>>>>                                 ^
>>>>> include/wait_bit.h:48:9: note: in definition of macro
>>>>> 'BUILD_WAIT_FOR_BIT'
>>>>>     val = read(reg);     \
>>>>>           ^~~~
>>>>
>>>> Did you change wait_for_bit function with _le32? [1]
>>>>
>>>> [1]
>>>> http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8
>>>>
>>> Yes, I am building plain u-boot-spi/master(HEAD: 0f520af57c60f3baba8
>>> ("spi: kirkwood_spi: implement workaround for FE-9144572")). So, above
>>> commit is part of the tree.
>>>
>>> Problem is above commit adds
>>> BUILD_WAIT_FOR_BIT(be32, u32, read*_be) variants.
>>> But, readl_be and readw_be seems to be undefined for ARM?
>>>
>>>
>>
>> Vignesh, I think there's an ongoing discussion about this on the u-boot list
>> to fix this?
> 
> didn't get what issue Vignesh, pointing at but with new wait_for_bit
> need a fix[2] to resolve warnings.
> 
> [2] https://gist.github.com/Noltari/3e6ed4648b87484c73ca22e2f533f9b0
> 
Yes, above patch should help to get rid of warnings. I will rebase, test
and resend.


-- 
Regards
Vignesh