cadence-quadspi: Fix issues with non 32bit aligned accesses

PATCH Superseded Vignesh R 5 participants 17 messages Jan 9, 2018 – Jan 23, 2018 SPI

Share & tools

Vignesh R

author

2018-01-09 13:19 UTC #0 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

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

Signature · 1 line

2.15.1

Vignesh R

author

2018-01-09 13:19 UTC #1 lore raw

PATCH v2 1/3 Revert "spi: cadence_qspi_apb: Use 32 bit indirect read transaction when possible"

From: Goldschmidt Simon <[email protected]>

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 <[email protected]>

Signed-off-by Vignesh R <[email protected]>

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

drivers/spi/cadence_qspi_apb.c +6 ~~-16~~

index 8309ab87940c..c7cb33aa8fc3 100644
@@ -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;
 }

Signature · 1 line

2.15.1

Vignesh R

author

2018-01-09 13:19 UTC #2 lore raw

PATCH v2 2/3 Revert "spi: cadence_qspi_apb: Use 32 bit indirect write transaction when possible"

In reply to 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 <[email protected]>

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

drivers/spi/cadence_qspi_apb.c +6 ~~-20~~

index c7cb33aa8fc3..fa8af33ae19b 100644
@@ -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;
 }

include/configs/k2g_evm.h +0 -1

index 535e7124fc80..0a38922a519e 100644
@@ -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

include/configs/socfpga_common.h +0 -1

index ec8bb500504a..f6607b101ec5 100644
@@ -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

include/configs/stv0991.h +0 -1

index fd96979bf897..beb8f1ae9a92 100644
@@ -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

Signature · 1 line

2.15.1

Vignesh R

author

2018-01-09 13:19 UTC #3 lore raw

PATCH v2 3/3 spi: cadence_qspi_apb: Make flash writes 32 bit aligned

In reply to 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 <[email protected]>

---

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

drivers/spi/cadence_qspi_apb.c +25 -6

index fa8af33ae19b..23441964a543 100644
@@ -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;
 }

Signature · 1 line

2.15.1

Vignesh R

author

2018-01-15 11:36 UTC #4 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

In reply to Vignesh R

Marek,

Quoted 8 lines · 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.

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?

Quoted 15 lines

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

Marek Vasut

2018-01-15 11:45 UTC #5 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 9 lines · Vignesh R

Marek,
Quoted 6 lines · 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/

Quoted 9 lines

Quoted 2 lines
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 <[email protected]>

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

Signature · 2 lines

Best regards,
Marek Vasut

Simon Goldschmidt

2018-01-15 12:01 UTC #6 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

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

Quoted 14 lines · Marek Vasut

Quoted 13 lines · Vignesh R
Marek,
Quoted 8 lines · 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.
Could you ack this series, if you are okay with the changes?

Quoted 8 lines

Quoted 4 lines
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 <[email protected]>
although you should get ack from Jason and Simon instead, those matter.

Acked-by Simon Goldschmidt <[email protected]>

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.

Jason Rush

2018-01-15 13:47 UTC #7 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 33 lines · Simon Goldschmidt

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
Quoted 14 lines · Marek Vasut
Quoted 13 lines · Vignesh R
Marek,
Quoted 8 lines · 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.
Could you ack this series, if you are okay with the changes?
Quoted 8 lines
Quoted 4 lines
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 <[email protected]>
although you should get ack from Jason and Simon instead, those matter.
Acked-by Simon Goldschmidt <[email protected]>
If that "Simon" above was me :-) I thought ack was sent by maintainers and TB by me, sorry.

Quoted 8 lines

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 <[email protected]>

Acked-by Jason Rush <[email protected]>

Simon Goldschmidt

2018-01-23 08:12 UTC #8 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

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

Quoted 45 lines · Jason Rush

Quoted 31 lines · Simon Goldschmidt
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
Quoted 14 lines · Marek Vasut
Quoted 13 lines · Vignesh R
Marek,
Quoted 8 lines · 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.
Could you ack this series, if you are okay with the changes?
Quoted 8 lines
Quoted 4 lines
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 <[email protected]>
although you should get ack from Jason and Simon instead, those matter.
Acked-by Simon Goldschmidt <[email protected]>
If that "Simon" above was me :-) I thought ack was sent by maintainers and TB by me, sorry.
Quoted 8 lines
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 <[email protected]>
Acked-by Jason Rush <[email protected]>

Jagan Teki

2018-01-23 08:45 UTC #9 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 5 lines · 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?

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

Vignesh R

author

2018-01-23 09:02 UTC #10 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 10 lines · Jagan Teki

Quoted 5 lines · 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?
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);     \
         ^~~~

Signature · 2 lines

Regards
Vignesh

Jagan Teki

2018-01-23 09:07 UTC #11 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 21 lines · Vignesh R

Quoted 10 lines · Jagan Teki
Quoted 5 lines · 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?
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)

Quoted 11 lines

                               ^
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

Vignesh R

author

2018-01-23 09:16 UTC #12 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 34 lines · Jagan Teki

Quoted 21 lines · Vignesh R
Quoted 10 lines · Jagan Teki
Quoted 5 lines · 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?
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)
Quoted 11 lines
                               ^
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);     \
         ^~~~

Quoted 4 lines

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?

Signature · 2 lines

Regards
Vignesh

Jagan Teki

2018-01-23 09:24 UTC #13 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 42 lines · Vignesh R

Quoted 34 lines · Jagan Teki
Quoted 21 lines · Vignesh R
Quoted 10 lines · Jagan Teki
Quoted 5 lines · 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?
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)
Quoted 11 lines
                               ^
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);     \
         ^~~~
Quoted 4 lines
Did you change wait_for_bit function with _le32? [1]
[1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8

Quoted 7 lines

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?

Simon Goldschmidt

2018-01-23 09:26 UTC #14 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

In reply to Vignesh R

Quoted 36 lines · Vignesh R

Quoted 30 lines · Jagan Teki
Quoted 18 lines · Vignesh R
Quoted 9 lines · Jagan Teki
Quoted 5 lines · 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?
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)
Quoted 11 lines
                                ^
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);     \
          ^~~~
Quoted 4 lines
Did you change wait_for_bit function with _le32? [1]
[1] http://git.denx.de/?p=u-boot-spi.git;a=commitdiff;h=9a5ff2669ef185d3a4bf73415c531e8d013993d8

Quoted 9 lines

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

Jagan Teki

2018-01-23 09:31 UTC #15 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

Quoted 56 lines · Simon Goldschmidt

Quoted 55 lines · Vignesh R
Quoted 43 lines · Jagan Teki
Quoted 35 lines · Vignesh R
Quoted 7 lines · Jagan Teki
Quoted 4 lines · Simon Goldschmidt
Jagan,
Quoted 6 lines
Quoted 3 lines
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)
Quoted 5 lines
                                ^
include/wait_bit.h:48:9: note: in definition of macro
'BUILD_WAIT_FOR_BIT'
    val = read(reg);     \
          ^~~~
Quoted 5 lines
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.

Quoted 8 lines

Quoted 5 lines
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

Vignesh R

author

2018-01-23 10:05 UTC #16 lore raw

v2 cover cadence-quadspi: Fix issues with non 32bit aligned accesses

On Tuesday 23 January 2018 03:01 PM, Jagan Teki wrote:
[...]

Quoted 39 lines

Quoted 39 lines
Quoted 38 lines
Quoted 24 lines
Quoted 18 lines
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)
Quoted 5 lines
                                ^
include/wait_bit.h:48:9: note: in definition of macro
'BUILD_WAIT_FOR_BIT'
    val = read(reg);     \
          ^~~~
Quoted 5 lines
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?

Quoted 8 lines

Quoted 2 lines
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.

Signature · 2 lines

Regards
Vignesh