All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem.
@ 2020-02-04 11:08 Masahiro Yamada
  2020-02-04 11:08 ` [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single() Masahiro Yamada
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:08 UTC (permalink / raw)
  To: u-boot


My main motivation of this series is the last patch
"mmc: sdhci: fix missing cache invalidation after reading by DMA".

Currently, read data are occasionally corrupted due to the
missing cache invalidation.

To fix it nicely (adds dma_unmap_single(), which follows the
Linux coding style), I did some cleaups first.

Patch 01 and 02 fix the prototypes of dma_{map,unmap}_single().

Patch 03-08 are code clean-ups.

Patch 09 fixes the bug.



Masahiro Yamada (9):
  dma-mapping: fix the prototype of dma_map_single()
  dma-mapping: fix the prototype of dma_unmap_single()
  mmc: sdhci: put the aligned buffer pointer to struct sdhci_host
  mmc: sdhci: reduce code duplication for aligned buffer
  mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting
    adma_addr
  mmc: sdhci: remove unneeded casts
  mmc: add mmc_get_dma_dir() helper
  mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA
  mmc: sdhci: fix missing cache invalidation after reading by DMA

 arch/arm/include/asm/dma-mapping.h   |  9 ++-
 arch/nds32/include/asm/dma-mapping.h |  9 ++-
 arch/riscv/include/asm/dma-mapping.h |  9 ++-
 arch/x86/include/asm/dma-mapping.h   |  9 ++-
 drivers/mmc/sdhci.c                  | 96 +++++++++++++---------------
 drivers/mmc/tmio-common.c            |  2 +-
 drivers/mtd/nand/raw/denali.c        |  2 +-
 drivers/net/macb.c                   |  2 +-
 drivers/usb/dwc3/core.c              |  6 +-
 drivers/usb/gadget/udc/udc-core.c    |  2 +-
 include/mmc.h                        |  6 ++
 include/sdhci.h                      |  3 +
 12 files changed, 78 insertions(+), 77 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
@ 2020-02-04 11:08 ` Masahiro Yamada
  2020-02-05  0:16   ` Simon Glass
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D0737@ATCPCS16.andestech.com>
  2020-02-04 11:08 ` [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single() Masahiro Yamada
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:08 UTC (permalink / raw)
  To: u-boot

Make dma_map_single() return the dma address, and remove the
pointless volatile.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/include/asm/dma-mapping.h   | 5 +++--
 arch/nds32/include/asm/dma-mapping.h | 5 +++--
 arch/riscv/include/asm/dma-mapping.h | 5 +++--
 arch/x86/include/asm/dma-mapping.h   | 5 +++--
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index d20703739fad..d0895a209666 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -11,6 +11,7 @@
 #include <asm/cache.h>
 #include <cpu_func.h>
 #include <linux/dma-direction.h>
+#include <linux/types.h>
 #include <malloc.h>
 
 #define	dma_mapping_error(x, y)	0
@@ -26,8 +27,8 @@ static inline void dma_free_coherent(void *addr)
 	free(addr);
 }
 
-static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
-					   enum dma_data_direction dir)
+static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
+					enum dma_data_direction dir)
 {
 	unsigned long addr = (unsigned long)vaddr;
 
diff --git a/arch/nds32/include/asm/dma-mapping.h b/arch/nds32/include/asm/dma-mapping.h
index c8876ceadda6..9387dec34768 100644
--- a/arch/nds32/include/asm/dma-mapping.h
+++ b/arch/nds32/include/asm/dma-mapping.h
@@ -10,6 +10,7 @@
 #include <asm/cache.h>
 #include <cpu_func.h>
 #include <linux/dma-direction.h>
+#include <linux/types.h>
 #include <malloc.h>
 
 static void *dma_alloc_coherent(size_t len, unsigned long *handle)
@@ -18,8 +19,8 @@ static void *dma_alloc_coherent(size_t len, unsigned long *handle)
 	return (void *)*handle;
 }
 
-static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
-					   enum dma_data_direction dir)
+static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
+					enum dma_data_direction dir)
 {
 	unsigned long addr = (unsigned long)vaddr;
 
diff --git a/arch/riscv/include/asm/dma-mapping.h b/arch/riscv/include/asm/dma-mapping.h
index 6cc39469590a..eac56f8fbdfa 100644
--- a/arch/riscv/include/asm/dma-mapping.h
+++ b/arch/riscv/include/asm/dma-mapping.h
@@ -10,6 +10,7 @@
 #define __ASM_RISCV_DMA_MAPPING_H
 
 #include <common.h>
+#include <linux/types.h>
 #include <asm/cache.h>
 #include <cpu_func.h>
 #include <linux/dma-direction.h>
@@ -28,8 +29,8 @@ static inline void dma_free_coherent(void *addr)
 	free(addr);
 }
 
-static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
-					   enum dma_data_direction dir)
+static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
+					enum dma_data_direction dir)
 {
 	unsigned long addr = (unsigned long)vaddr;
 
diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
index 900b99b8a69a..7e205e3313ac 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -11,6 +11,7 @@
 #include <asm/cache.h>
 #include <cpu_func.h>
 #include <linux/dma-direction.h>
+#include <linux/types.h>
 #include <malloc.h>
 
 #define	dma_mapping_error(x, y)	0
@@ -26,8 +27,8 @@ static inline void dma_free_coherent(void *addr)
 	free(addr);
 }
 
-static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
-					   enum dma_data_direction dir)
+static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
+					enum dma_data_direction dir)
 {
 	unsigned long addr = (unsigned long)vaddr;
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single()
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
  2020-02-04 11:08 ` [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single() Masahiro Yamada
@ 2020-02-04 11:08 ` Masahiro Yamada
  2020-02-05  0:16   ` Simon Glass
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D073E@ATCPCS16.andestech.com>
  2020-02-04 11:09 ` [PATCH 3/9] mmc: sdhci: put the aligned buffer pointer to struct sdhci_host Masahiro Yamada
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:08 UTC (permalink / raw)
  To: u-boot

dma_unmap_single() takes the dma address, not virtual address.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/arm/include/asm/dma-mapping.h   | 4 +---
 arch/nds32/include/asm/dma-mapping.h | 4 +---
 arch/riscv/include/asm/dma-mapping.h | 4 +---
 arch/x86/include/asm/dma-mapping.h   | 4 +---
 drivers/mmc/tmio-common.c            | 2 +-
 drivers/mtd/nand/raw/denali.c        | 2 +-
 drivers/net/macb.c                   | 2 +-
 drivers/usb/dwc3/core.c              | 6 +++---
 drivers/usb/gadget/udc/udc-core.c    | 2 +-
 9 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index d0895a209666..efdbed7280dd 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -42,11 +42,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
 	return addr;
 }
 
-static inline void dma_unmap_single(volatile void *vaddr, size_t len,
+static inline void dma_unmap_single(dma_addr_t addr, size_t len,
 				    enum dma_data_direction dir)
 {
-	unsigned long addr = (unsigned long)vaddr;
-
 	len = ALIGN(len, ARCH_DMA_MINALIGN);
 
 	if (dir != DMA_TO_DEVICE)
diff --git a/arch/nds32/include/asm/dma-mapping.h b/arch/nds32/include/asm/dma-mapping.h
index 9387dec34768..784f489db54e 100644
--- a/arch/nds32/include/asm/dma-mapping.h
+++ b/arch/nds32/include/asm/dma-mapping.h
@@ -34,11 +34,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
 	return addr;
 }
 
-static inline void dma_unmap_single(volatile void *vaddr, size_t len,
+static inline void dma_unmap_single(dma_addr_t addr, size_t len,
 				    enum dma_data_direction dir)
 {
-	unsigned long addr = (unsigned long)vaddr;
-
 	len = ALIGN(len, ARCH_DMA_MINALIGN);
 
 	if (dir != DMA_TO_DEVICE)
diff --git a/arch/riscv/include/asm/dma-mapping.h b/arch/riscv/include/asm/dma-mapping.h
index eac56f8fbdfa..1ac4a4fed58d 100644
--- a/arch/riscv/include/asm/dma-mapping.h
+++ b/arch/riscv/include/asm/dma-mapping.h
@@ -44,11 +44,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
 	return addr;
 }
 
-static inline void dma_unmap_single(volatile void *vaddr, size_t len,
+static inline void dma_unmap_single(dma_addr_t addr, size_t len,
 				    enum dma_data_direction dir)
 {
-	unsigned long addr = (unsigned long)vaddr;
-
 	len = ALIGN(len, ARCH_DMA_MINALIGN);
 
 	if (dir != DMA_TO_DEVICE)
diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
index 7e205e3313ac..37704da5dd4f 100644
--- a/arch/x86/include/asm/dma-mapping.h
+++ b/arch/x86/include/asm/dma-mapping.h
@@ -42,11 +42,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
 	return addr;
 }
 
-static inline void dma_unmap_single(volatile void *vaddr, size_t len,
+static inline void dma_unmap_single(dma_addr_t addr, size_t len,
 				    enum dma_data_direction dir)
 {
-	unsigned long addr = (unsigned long)vaddr;
-
 	len = ALIGN(len, ARCH_DMA_MINALIGN);
 
 	if (dir != DMA_TO_DEVICE)
diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c
index 5a8506dcb6bd..e5d9d64b9684 100644
--- a/drivers/mmc/tmio-common.c
+++ b/drivers/mmc/tmio-common.c
@@ -352,7 +352,7 @@ static int tmio_sd_dma_xfer(struct udevice *dev, struct mmc_data *data)
 	if (poll_flag == TMIO_SD_DMA_INFO1_END_RD)
 		udelay(1);
 
-	dma_unmap_single(buf, len, dir);
+	dma_unmap_single(dma_addr, len, dir);
 
 	return ret;
 }
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 8537c609fb62..8dfffad63b60 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -577,7 +577,7 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
 
 	iowrite32(0, denali->reg + DMA_ENABLE);
 
-	dma_unmap_single(buf, size, dir);
+	dma_unmap_single(dma_addr, size, dir);
 
 	if (irq_status & INTR__ERASED_PAGE)
 		memset(buf, 0xff, size);
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 0d4929bec131..7a2b1abeeb03 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -342,7 +342,7 @@ static int _macb_send(struct macb_device *macb, const char *name, void *packet,
 		udelay(1);
 	}
 
-	dma_unmap_single(packet, length, DMA_TO_DEVICE);
+	dma_unmap_single(paddr, length, DMA_TO_DEVICE);
 
 	if (i <= MACB_TX_TIMEOUT) {
 		if (ctrl & MACB_BIT(TX_UNDERRUN))
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 77c555e76924..c5907534692e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -285,8 +285,8 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
 	return 0;
 
 err1:
-	dma_unmap_single((void *)(uintptr_t)dwc->scratch_addr, dwc->nr_scratch *
-			 DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
+	dma_unmap_single(scratch_addr, dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
+			 DMA_BIDIRECTIONAL);
 
 err0:
 	return ret;
@@ -300,7 +300,7 @@ static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
 	if (!dwc->nr_scratch)
 		return;
 
-	dma_unmap_single((void *)(uintptr_t)dwc->scratch_addr, dwc->nr_scratch *
+	dma_unmap_single(dwc->scratch_addr, dwc->nr_scratch *
 			 DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
 	kfree(dwc->scratchbuf);
 }
diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index 8d1d90e3e39f..8437d9079bef 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -65,7 +65,7 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
 	if (req->length == 0)
 		return;
 
-	dma_unmap_single((void *)(uintptr_t)req->dma, req->length,
+	dma_unmap_single(req->dma, req->length,
 			 is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
 }
 EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] mmc: sdhci: put the aligned buffer pointer to struct sdhci_host
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
  2020-02-04 11:08 ` [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single() Masahiro Yamada
  2020-02-04 11:08 ` [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single() Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 4/9] mmc: sdhci: reduce code duplication for aligned buffer Masahiro Yamada
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

Using the global variable does not look nice.

Add a new field sthci::align_buffer to point to the bounce buffer.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 27 +++++++++++++--------------
 include/sdhci.h     |  1 +
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 01fa5a9d4d5b..18fbcb5f1864 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -16,12 +16,6 @@
 #include <sdhci.h>
 #include <dm.h>
 
-#if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
-void *aligned_buffer = (void *)CONFIG_FIXED_SDHCI_ALIGNED_BUFFER;
-#else
-void *aligned_buffer;
-#endif
-
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
 	unsigned long timeout;
@@ -149,9 +143,10 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 		if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) &&
 		    (host->start_addr & 0x7) != 0x0) {
 			*is_aligned = 0;
-			host->start_addr = (unsigned long)aligned_buffer;
+			host->start_addr = (unsigned long)host->align_buffer;
 			if (data->flags != MMC_DATA_READ)
-				memcpy(aligned_buffer, data->src, trans_bytes);
+				memcpy(host->align_buffer, data->src,
+				       trans_bytes);
 		}
 
 #if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
@@ -160,9 +155,9 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 		 * CONFIG_FIXED_SDHCI_ALIGNED_BUFFER is defined
 		 */
 		*is_aligned = 0;
-		host->start_addr = (unsigned long)aligned_buffer;
+		host->start_addr = (unsigned long)host->align_buffer;
 		if (data->flags != MMC_DATA_READ)
-			memcpy(aligned_buffer, data->src, trans_bytes);
+			memcpy(host->align_buffer, data->src, trans_bytes);
 #endif
 		sdhci_writel(host, host->start_addr, SDHCI_DMA_ADDRESS);
 
@@ -381,7 +376,7 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 	if (!ret) {
 		if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) &&
 				!is_aligned && (data->flags == MMC_DATA_READ))
-			memcpy(data->dest, aligned_buffer, trans_bytes);
+			memcpy(data->dest, host->align_buffer, trans_bytes);
 		return 0;
 	}
 
@@ -630,14 +625,18 @@ static int sdhci_init(struct mmc *mmc)
 
 	sdhci_reset(host, SDHCI_RESET_ALL);
 
-	if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) && !aligned_buffer) {
-		aligned_buffer = memalign(8, 512*1024);
-		if (!aligned_buffer) {
+#if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
+	host->align_buffer = (void *)CONFIG_FIXED_SDHCI_ALIGNED_BUFFER;
+#else
+	if (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) {
+		host->align_buffer = memalign(8, 512 * 1024);
+		if (!host->align_buffer) {
 			printf("%s: Aligned buffer alloc failed!!!\n",
 			       __func__);
 			return -ENOMEM;
 		}
 	}
+#endif
 
 	sdhci_set_power(host, fls(mmc->cfg->voltages) - 1);
 
diff --git a/include/sdhci.h b/include/sdhci.h
index 01addb7a6036..1358218270b8 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -321,6 +321,7 @@ struct sdhci_host {
 	uint	voltages;
 
 	struct mmc_config cfg;
+	void *align_buffer;
 	dma_addr_t start_addr;
 	int flags;
 #define USE_SDMA	(0x1 << 0)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] mmc: sdhci: reduce code duplication for aligned buffer
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (2 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 3/9] mmc: sdhci: put the aligned buffer pointer to struct sdhci_host Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 5/9] mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting adma_addr Masahiro Yamada
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

The same code is run for both SDHCI_QUIRK_32BIT_DMA_ADDR and
define(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER).

Unify the code.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 22 ++++++++--------------
 include/sdhci.h     |  2 ++
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 18fbcb5f1864..b4713e7b9bba 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -140,27 +140,16 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 
 	if (host->flags & USE_SDMA) {
-		if ((host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) &&
-		    (host->start_addr & 0x7) != 0x0) {
+		if (host->force_align_buffer ||
+		    (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR &&
+		     (host->start_addr & 0x7) != 0x0)) {
 			*is_aligned = 0;
 			host->start_addr = (unsigned long)host->align_buffer;
 			if (data->flags != MMC_DATA_READ)
 				memcpy(host->align_buffer, data->src,
 				       trans_bytes);
 		}
-
-#if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
-		/*
-		 * Always use this bounce-buffer when
-		 * CONFIG_FIXED_SDHCI_ALIGNED_BUFFER is defined
-		 */
-		*is_aligned = 0;
-		host->start_addr = (unsigned long)host->align_buffer;
-		if (data->flags != MMC_DATA_READ)
-			memcpy(host->align_buffer, data->src, trans_bytes);
-#endif
 		sdhci_writel(host, host->start_addr, SDHCI_DMA_ADDRESS);
-
 	} else if (host->flags & (USE_ADMA | USE_ADMA64)) {
 		sdhci_prepare_adma_table(host, data);
 
@@ -627,6 +616,11 @@ static int sdhci_init(struct mmc *mmc)
 
 #if defined(CONFIG_FIXED_SDHCI_ALIGNED_BUFFER)
 	host->align_buffer = (void *)CONFIG_FIXED_SDHCI_ALIGNED_BUFFER;
+	/*
+	 * Always use this bounce-buffer when CONFIG_FIXED_SDHCI_ALIGNED_BUFFER
+	 * is defined.
+	 */
+	host->force_align_buffer = true;
 #else
 	if (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR) {
 		host->align_buffer = memalign(8, 512 * 1024);
diff --git a/include/sdhci.h b/include/sdhci.h
index 1358218270b8..7f8feefa450b 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -9,6 +9,7 @@
 #ifndef __SDHCI_HW_H
 #define __SDHCI_HW_H
 
+#include <linux/types.h>
 #include <asm/io.h>
 #include <mmc.h>
 #include <asm/gpio.h>
@@ -322,6 +323,7 @@ struct sdhci_host {
 
 	struct mmc_config cfg;
 	void *align_buffer;
+	bool force_align_buffer;
 	dma_addr_t start_addr;
 	int flags;
 #define USE_SDMA	(0x1 << 0)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting adma_addr
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (3 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 4/9] mmc: sdhci: reduce code duplication for aligned buffer Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 6/9] mmc: sdhci: remove unneeded casts Masahiro Yamada
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

Use {lower,upper}_32_bits() instead of the combination of cast
and shift.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index b4713e7b9bba..fefe81016eb1 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -153,9 +153,10 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 	} else if (host->flags & (USE_ADMA | USE_ADMA64)) {
 		sdhci_prepare_adma_table(host, data);
 
-		sdhci_writel(host, (u32)host->adma_addr, SDHCI_ADMA_ADDRESS);
+		sdhci_writel(host, lower_32_bits(host->adma_addr),
+			     SDHCI_ADMA_ADDRESS);
 		if (host->flags & USE_ADMA64)
-			sdhci_writel(host, (u64)host->adma_addr >> 32,
+			sdhci_writel(host, upper_32_bits(host->adma_addr),
 				     SDHCI_ADMA_ADDRESS_HI);
 	}
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] mmc: sdhci: remove unneeded casts
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (4 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 5/9] mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting adma_addr Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 7/9] mmc: add mmc_get_dma_dir() helper Masahiro Yamada
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

host->mmc is already (struct mmc *).

memalign() returns an opaque pointer, so there is no need for casting.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index fefe81016eb1..ee54d78a28f2 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -522,7 +522,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
 
 void sdhci_set_uhs_timing(struct sdhci_host *host)
 {
-	struct mmc *mmc = (struct mmc *)host->mmc;
+	struct mmc *mmc = host->mmc;
 	u32 reg;
 
 	reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
@@ -735,8 +735,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host,
 		       __func__);
 		return -EINVAL;
 	}
-	host->adma_desc_table = (struct sdhci_adma_desc *)
-				memalign(ARCH_DMA_MINALIGN, ADMA_TABLE_SZ);
+	host->adma_desc_table = memalign(ARCH_DMA_MINALIGN, ADMA_TABLE_SZ);
 
 	host->adma_addr = (dma_addr_t)host->adma_desc_table;
 #ifdef CONFIG_DMA_ADDR_T_64BIT
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] mmc: add mmc_get_dma_dir() helper
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (5 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 6/9] mmc: sdhci: remove unneeded casts Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 8/9] mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA Masahiro Yamada
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

Copied from Linux kernel.
include/linux/mmc/host.h

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 include/mmc.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/mmc.h b/include/mmc.h
index b5cb514f57d6..71e2e1735ad5 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -12,6 +12,7 @@
 #include <linux/list.h>
 #include <linux/sizes.h>
 #include <linux/compiler.h>
+#include <linux/dma-direction.h>
 #include <part.h>
 
 #if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)
@@ -880,4 +881,9 @@ int mmc_get_env_dev(void);
  */
 struct blk_desc *mmc_get_blk_desc(struct mmc *mmc);
 
+static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
+{
+	return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+}
+
 #endif /* _MMC_H_ */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (6 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 7/9] mmc: add mmc_get_dma_dir() helper Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-04 11:09 ` [PATCH 9/9] mmc: sdhci: fix missing cache invalidation after reading by DMA Masahiro Yamada
  2020-02-14  0:50 ` [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Peng Fan
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

Currently, sdhci_prepare_dma() calls flush_cache() regardless of the
DMA direction.

Actually, cache invalidation is enough when reading data from the device.

This is correctly handled by dma_map_single(), which mimics the DMA-API
in Linux kernel. Drivers can be agnostic which cache operation occurs
behind the scene.

This commit also sanitizes the difference between the virtual address
and the dma address.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 51 ++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index ee54d78a28f2..193f402b516f 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -15,6 +15,7 @@
 #include <mmc.h>
 #include <sdhci.h>
 #include <dm.h>
+#include <asm/dma-mapping.h>
 
 static void sdhci_reset(struct sdhci_host *host, u8 mask)
 {
@@ -65,8 +66,8 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 }
 
 #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)
-static void sdhci_adma_desc(struct sdhci_host *host, char *buf, u16 len,
-			    bool end)
+static void sdhci_adma_desc(struct sdhci_host *host, dma_addr_t dma_addr,
+			    u16 len, bool end)
 {
 	struct sdhci_adma_desc *desc;
 	u8 attr;
@@ -82,9 +83,9 @@ static void sdhci_adma_desc(struct sdhci_host *host, char *buf, u16 len,
 	desc->attr = attr;
 	desc->len = len;
 	desc->reserved = 0;
-	desc->addr_lo = (dma_addr_t)buf;
+	desc->addr_lo = lower_32_bits(dma_addr);
 #ifdef CONFIG_DMA_ADDR_T_64BIT
-	desc->addr_hi = (u64)buf >> 32;
+	desc->addr_hi = upper_32_bits(dma_addr);
 #endif
 }
 
@@ -94,22 +95,17 @@ static void sdhci_prepare_adma_table(struct sdhci_host *host,
 	uint trans_bytes = data->blocksize * data->blocks;
 	uint desc_count = DIV_ROUND_UP(trans_bytes, ADMA_MAX_LEN);
 	int i = desc_count;
-	char *buf;
+	dma_addr_t dma_addr = host->start_addr;
 
 	host->desc_slot = 0;
 
-	if (data->flags & MMC_DATA_READ)
-		buf = data->dest;
-	else
-		buf = (char *)data->src;
-
 	while (--i) {
-		sdhci_adma_desc(host, buf, ADMA_MAX_LEN, false);
-		buf += ADMA_MAX_LEN;
+		sdhci_adma_desc(host, dma_addr, ADMA_MAX_LEN, false);
+		dma_addr += ADMA_MAX_LEN;
 		trans_bytes -= ADMA_MAX_LEN;
 	}
 
-	sdhci_adma_desc(host, buf, trans_bytes, true);
+	sdhci_adma_desc(host, dma_addr, trans_bytes, true);
 
 	flush_cache((dma_addr_t)host->adma_desc_table,
 		    ROUND(desc_count * sizeof(struct sdhci_adma_desc),
@@ -125,11 +121,12 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 			      int *is_aligned, int trans_bytes)
 {
 	unsigned char ctrl;
+	void *buf;
 
 	if (data->flags == MMC_DATA_READ)
-		host->start_addr = (dma_addr_t)data->dest;
+		buf = data->dest;
 	else
-		host->start_addr = (dma_addr_t)data->src;
+		buf = (void *)data->src;
 
 	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
 	ctrl &= ~SDHCI_CTRL_DMA_MASK;
@@ -139,16 +136,20 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 		ctrl |= SDHCI_CTRL_ADMA32;
 	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
 
+	if (host->flags & USE_SDMA &&
+	    (host->force_align_buffer ||
+	     (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR &&
+	      ((unsigned long)buf & 0x7) != 0x0))) {
+		*is_aligned = 0;
+		if (data->flags != MMC_DATA_READ)
+			memcpy(host->align_buffer, buf, trans_bytes);
+		buf = host->align_buffer;
+	}
+
+	host->start_addr = dma_map_single(buf, trans_bytes,
+					  mmc_get_dma_dir(data));
+
 	if (host->flags & USE_SDMA) {
-		if (host->force_align_buffer ||
-		    (host->quirks & SDHCI_QUIRK_32BIT_DMA_ADDR &&
-		     (host->start_addr & 0x7) != 0x0)) {
-			*is_aligned = 0;
-			host->start_addr = (unsigned long)host->align_buffer;
-			if (data->flags != MMC_DATA_READ)
-				memcpy(host->align_buffer, data->src,
-				       trans_bytes);
-		}
 		sdhci_writel(host, host->start_addr, SDHCI_DMA_ADDRESS);
 	} else if (host->flags & (USE_ADMA | USE_ADMA64)) {
 		sdhci_prepare_adma_table(host, data);
@@ -159,8 +160,6 @@ static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
 			sdhci_writel(host, upper_32_bits(host->adma_addr),
 				     SDHCI_ADMA_ADDRESS_HI);
 	}
-
-	flush_cache(host->start_addr, ROUND(trans_bytes, ARCH_DMA_MINALIGN));
 }
 #else
 static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data,
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] mmc: sdhci: fix missing cache invalidation after reading by DMA
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (7 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 8/9] mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA Masahiro Yamada
@ 2020-02-04 11:09 ` Masahiro Yamada
  2020-02-14  0:50 ` [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Peng Fan
  9 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-04 11:09 UTC (permalink / raw)
  To: u-boot

This driver currently performs cache operation before the DMA start,
but does nothing after the DMA completion.

When reading data by DMA, the cache invalidation is needed also after
finishing the DMA transfer. Otherwise, the CPU might read data from
the cache instead of from the main memory when speculative memory read
or memory prefetch occurs.

Instead of calling the cache operation directly, this commit adds
dma_unmap_single(), which performs cache invalidation internally,
but drivers do not need which operation is being run.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/mmc/sdhci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 193f402b516f..f31f6628870f 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -215,6 +215,10 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
 			return -ETIMEDOUT;
 		}
 	} while (!(stat & SDHCI_INT_DATA_END));
+
+	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
+			 mmc_get_dma_dir(data));
+
 	return 0;
 }
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
  2020-02-04 11:08 ` [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single() Masahiro Yamada
@ 2020-02-05  0:16   ` Simon Glass
  2020-02-11 15:18     ` Masahiro Yamada
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D0737@ATCPCS16.andestech.com>
  1 sibling, 1 reply; 19+ messages in thread
From: Simon Glass @ 2020-02-05  0:16 UTC (permalink / raw)
  To: u-boot

On Tue, 4 Feb 2020 at 04:09, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Make dma_map_single() return the dma address, and remove the
> pointless volatile.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/arm/include/asm/dma-mapping.h   | 5 +++--
>  arch/nds32/include/asm/dma-mapping.h | 5 +++--
>  arch/riscv/include/asm/dma-mapping.h | 5 +++--
>  arch/x86/include/asm/dma-mapping.h   | 5 +++--
>  4 files changed, 12 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please can you add a full function comment in the header files?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single()
  2020-02-04 11:08 ` [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single() Masahiro Yamada
@ 2020-02-05  0:16   ` Simon Glass
  2020-02-11 15:20     ` Masahiro Yamada
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D073E@ATCPCS16.andestech.com>
  1 sibling, 1 reply; 19+ messages in thread
From: Simon Glass @ 2020-02-05  0:16 UTC (permalink / raw)
  To: u-boot

On Tue, 4 Feb 2020 at 04:09, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> dma_unmap_single() takes the dma address, not virtual address.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/arm/include/asm/dma-mapping.h   | 4 +---
>  arch/nds32/include/asm/dma-mapping.h | 4 +---
>  arch/riscv/include/asm/dma-mapping.h | 4 +---
>  arch/x86/include/asm/dma-mapping.h   | 4 +---
>  drivers/mmc/tmio-common.c            | 2 +-
>  drivers/mtd/nand/raw/denali.c        | 2 +-
>  drivers/net/macb.c                   | 2 +-
>  drivers/usb/dwc3/core.c              | 6 +++---
>  drivers/usb/gadget/udc/udc-core.c    | 2 +-
>  9 files changed, 11 insertions(+), 19 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please also add a comment to the header files.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single()
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D073E@ATCPCS16.andestech.com>
@ 2020-02-06  1:13     ` Rick Chen
  0 siblings, 0 replies; 19+ messages in thread
From: Rick Chen @ 2020-02-06  1:13 UTC (permalink / raw)
  To: u-boot

> From: Masahiro Yamada [mailto:yamada.masahiro at socionext.com]
> Sent: Tuesday, February 04, 2020 7:09 PM
> To: u-boot at lists.denx.de
> Cc: Masahiro Yamada; Bin Meng; Joe Hershberger; Lukasz Majewski; Marek Vasut; Peng Fan; Rick Jian-Zhi Chen(陳建志); Simon Glass
> Subject: [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single()
>
> dma_unmap_single() takes the dma address, not virtual address.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Acked-by: Rick Chen <rick@andestech.com>

> ---
>
>  arch/arm/include/asm/dma-mapping.h   | 4 +---
>  arch/nds32/include/asm/dma-mapping.h | 4 +---  arch/riscv/include/asm/dma-mapping.h | 4 +---
>  arch/x86/include/asm/dma-mapping.h   | 4 +---
>  drivers/mmc/tmio-common.c            | 2 +-
>  drivers/mtd/nand/raw/denali.c        | 2 +-
>  drivers/net/macb.c                   | 2 +-
>  drivers/usb/dwc3/core.c              | 6 +++---
>  drivers/usb/gadget/udc/udc-core.c    | 2 +-
>  9 files changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index d0895a209666..efdbed7280dd 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -42,11 +42,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
>         return addr;
>  }
>
> -static inline void dma_unmap_single(volatile void *vaddr, size_t len,
> +static inline void dma_unmap_single(dma_addr_t addr, size_t len,
>                                     enum dma_data_direction dir)
>  {
> -       unsigned long addr = (unsigned long)vaddr;
> -
>         len = ALIGN(len, ARCH_DMA_MINALIGN);
>
>         if (dir != DMA_TO_DEVICE)
> diff --git a/arch/nds32/include/asm/dma-mapping.h b/arch/nds32/include/asm/dma-mapping.h
> index 9387dec34768..784f489db54e 100644
> --- a/arch/nds32/include/asm/dma-mapping.h
> +++ b/arch/nds32/include/asm/dma-mapping.h
> @@ -34,11 +34,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
>         return addr;
>  }
>
> -static inline void dma_unmap_single(volatile void *vaddr, size_t len,
> +static inline void dma_unmap_single(dma_addr_t addr, size_t len,
>                                     enum dma_data_direction dir)
>  {
> -       unsigned long addr = (unsigned long)vaddr;
> -
>         len = ALIGN(len, ARCH_DMA_MINALIGN);
>
>         if (dir != DMA_TO_DEVICE)
> diff --git a/arch/riscv/include/asm/dma-mapping.h b/arch/riscv/include/asm/dma-mapping.h
> index eac56f8fbdfa..1ac4a4fed58d 100644
> --- a/arch/riscv/include/asm/dma-mapping.h
> +++ b/arch/riscv/include/asm/dma-mapping.h
> @@ -44,11 +44,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
>         return addr;
>  }
>
> -static inline void dma_unmap_single(volatile void *vaddr, size_t len,
> +static inline void dma_unmap_single(dma_addr_t addr, size_t len,
>                                     enum dma_data_direction dir)
>  {
> -       unsigned long addr = (unsigned long)vaddr;
> -
>         len = ALIGN(len, ARCH_DMA_MINALIGN);
>
>         if (dir != DMA_TO_DEVICE)
> diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
> index 7e205e3313ac..37704da5dd4f 100644
> --- a/arch/x86/include/asm/dma-mapping.h
> +++ b/arch/x86/include/asm/dma-mapping.h
> @@ -42,11 +42,9 @@ static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
>         return addr;
>  }
>
> -static inline void dma_unmap_single(volatile void *vaddr, size_t len,
> +static inline void dma_unmap_single(dma_addr_t addr, size_t len,
>                                     enum dma_data_direction dir)
>  {
> -       unsigned long addr = (unsigned long)vaddr;
> -
>         len = ALIGN(len, ARCH_DMA_MINALIGN);
>
>         if (dir != DMA_TO_DEVICE)
> diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c index 5a8506dcb6bd..e5d9d64b9684 100644
> --- a/drivers/mmc/tmio-common.c
> +++ b/drivers/mmc/tmio-common.c
> @@ -352,7 +352,7 @@ static int tmio_sd_dma_xfer(struct udevice *dev, struct mmc_data *data)
>         if (poll_flag == TMIO_SD_DMA_INFO1_END_RD)
>                 udelay(1);
>
> -       dma_unmap_single(buf, len, dir);
> +       dma_unmap_single(dma_addr, len, dir);
>
>         return ret;
>  }
> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 8537c609fb62..8dfffad63b60 100644
> --- a/drivers/mtd/nand/raw/denali.c
> +++ b/drivers/mtd/nand/raw/denali.c
> @@ -577,7 +577,7 @@ static int denali_dma_xfer(struct denali_nand_info *denali, void *buf,
>
>         iowrite32(0, denali->reg + DMA_ENABLE);
>
> -       dma_unmap_single(buf, size, dir);
> +       dma_unmap_single(dma_addr, size, dir);
>
>         if (irq_status & INTR__ERASED_PAGE)
>                 memset(buf, 0xff, size);
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c index 0d4929bec131..7a2b1abeeb03 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -342,7 +342,7 @@ static int _macb_send(struct macb_device *macb, const char *name, void *packet,
>                 udelay(1);
>         }
>
> -       dma_unmap_single(packet, length, DMA_TO_DEVICE);
> +       dma_unmap_single(paddr, length, DMA_TO_DEVICE);
>
>         if (i <= MACB_TX_TIMEOUT) {
>                 if (ctrl & MACB_BIT(TX_UNDERRUN))
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 77c555e76924..c5907534692e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -285,8 +285,8 @@ static int dwc3_setup_scratch_buffers(struct dwc3 *dwc)
>         return 0;
>
>  err1:
> -       dma_unmap_single((void *)(uintptr_t)dwc->scratch_addr, dwc->nr_scratch *
> -                        DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
> +       dma_unmap_single(scratch_addr, dwc->nr_scratch * DWC3_SCRATCHBUF_SIZE,
> +                        DMA_BIDIRECTIONAL);
>
>  err0:
>         return ret;
> @@ -300,7 +300,7 @@ static void dwc3_free_scratch_buffers(struct dwc3 *dwc)
>         if (!dwc->nr_scratch)
>                 return;
>
> -       dma_unmap_single((void *)(uintptr_t)dwc->scratch_addr, dwc->nr_scratch *
> +       dma_unmap_single(dwc->scratch_addr, dwc->nr_scratch *
>                          DWC3_SCRATCHBUF_SIZE, DMA_BIDIRECTIONAL);
>         kfree(dwc->scratchbuf);
>  }
> diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
> index 8d1d90e3e39f..8437d9079bef 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -65,7 +65,7 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
>         if (req->length == 0)
>                 return;
>
> -       dma_unmap_single((void *)(uintptr_t)req->dma, req->length,
> +       dma_unmap_single(req->dma, req->length,
>                          is_in ? DMA_TO_DEVICE : DMA_FROM_DEVICE);  }  EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
       [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D0737@ATCPCS16.andestech.com>
@ 2020-02-06  1:16     ` Rick Chen
  0 siblings, 0 replies; 19+ messages in thread
From: Rick Chen @ 2020-02-06  1:16 UTC (permalink / raw)
  To: u-boot

> From: Masahiro Yamada [mailto:yamada.masahiro at socionext.com]
> Sent: Tuesday, February 04, 2020 7:09 PM
> To: u-boot at lists.denx.de
> Cc: Masahiro Yamada; Bin Meng; Rick Jian-Zhi Chen(陳建志); Simon Glass
> Subject: [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
>
> Make dma_map_single() return the dma address, and remove the pointless volatile.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Reviewed-by: Rick Chen <rick@andestech.com>

> ---
>
>  arch/arm/include/asm/dma-mapping.h   | 5 +++--
>  arch/nds32/include/asm/dma-mapping.h | 5 +++--  arch/riscv/include/asm/dma-mapping.h | 5 +++--
>  arch/x86/include/asm/dma-mapping.h   | 5 +++--
>  4 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
> index d20703739fad..d0895a209666 100644
> --- a/arch/arm/include/asm/dma-mapping.h
> +++ b/arch/arm/include/asm/dma-mapping.h
> @@ -11,6 +11,7 @@
>  #include <asm/cache.h>
>  #include <cpu_func.h>
>  #include <linux/dma-direction.h>
> +#include <linux/types.h>
>  #include <malloc.h>
>
>  #define        dma_mapping_error(x, y) 0
> @@ -26,8 +27,8 @@ static inline void dma_free_coherent(void *addr)
>         free(addr);
>  }
>
> -static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
> -                                          enum dma_data_direction dir)
> +static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
> +                                       enum dma_data_direction dir)
>  {
>         unsigned long addr = (unsigned long)vaddr;
>
> diff --git a/arch/nds32/include/asm/dma-mapping.h b/arch/nds32/include/asm/dma-mapping.h
> index c8876ceadda6..9387dec34768 100644
> --- a/arch/nds32/include/asm/dma-mapping.h
> +++ b/arch/nds32/include/asm/dma-mapping.h
> @@ -10,6 +10,7 @@
>  #include <asm/cache.h>
>  #include <cpu_func.h>
>  #include <linux/dma-direction.h>
> +#include <linux/types.h>
>  #include <malloc.h>
>
>  static void *dma_alloc_coherent(size_t len, unsigned long *handle) @@ -18,8 +19,8 @@ static void *dma_alloc_coherent(size_t len, unsigned long *handle)
>         return (void *)*handle;
>  }
>
> -static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
> -                                          enum dma_data_direction dir)
> +static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
> +                                       enum dma_data_direction dir)
>  {
>         unsigned long addr = (unsigned long)vaddr;
>
> diff --git a/arch/riscv/include/asm/dma-mapping.h b/arch/riscv/include/asm/dma-mapping.h
> index 6cc39469590a..eac56f8fbdfa 100644
> --- a/arch/riscv/include/asm/dma-mapping.h
> +++ b/arch/riscv/include/asm/dma-mapping.h
> @@ -10,6 +10,7 @@
>  #define __ASM_RISCV_DMA_MAPPING_H
>
>  #include <common.h>
> +#include <linux/types.h>
>  #include <asm/cache.h>
>  #include <cpu_func.h>
>  #include <linux/dma-direction.h>
> @@ -28,8 +29,8 @@ static inline void dma_free_coherent(void *addr)
>         free(addr);
>  }
>
> -static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
> -                                          enum dma_data_direction dir)
> +static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
> +                                       enum dma_data_direction dir)
>  {
>         unsigned long addr = (unsigned long)vaddr;
>
> diff --git a/arch/x86/include/asm/dma-mapping.h b/arch/x86/include/asm/dma-mapping.h
> index 900b99b8a69a..7e205e3313ac 100644
> --- a/arch/x86/include/asm/dma-mapping.h
> +++ b/arch/x86/include/asm/dma-mapping.h
> @@ -11,6 +11,7 @@
>  #include <asm/cache.h>
>  #include <cpu_func.h>
>  #include <linux/dma-direction.h>
> +#include <linux/types.h>
>  #include <malloc.h>
>
>  #define        dma_mapping_error(x, y) 0
> @@ -26,8 +27,8 @@ static inline void dma_free_coherent(void *addr)
>         free(addr);
>  }
>
> -static inline unsigned long dma_map_single(volatile void *vaddr, size_t len,
> -                                          enum dma_data_direction dir)
> +static inline dma_addr_t dma_map_single(void *vaddr, size_t len,
> +                                       enum dma_data_direction dir)
>  {
>         unsigned long addr = (unsigned long)vaddr;
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
  2020-02-05  0:16   ` Simon Glass
@ 2020-02-11 15:18     ` Masahiro Yamada
  2020-02-11 17:14       ` Simon Glass
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-11 15:18 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On Wed, Feb 5, 2020 at 9:17 AM Simon Glass <sjg@chromium.org> wrote:
>
> On Tue, 4 Feb 2020 at 04:09, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > Make dma_map_single() return the dma address, and remove the
> > pointless volatile.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  arch/arm/include/asm/dma-mapping.h   | 5 +++--
> >  arch/nds32/include/asm/dma-mapping.h | 5 +++--
> >  arch/riscv/include/asm/dma-mapping.h | 5 +++--
> >  arch/x86/include/asm/dma-mapping.h   | 5 +++--
> >  4 files changed, 12 insertions(+), 8 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But please can you add a full function comment in the header files?


I can comment them, but it is tedious to duplicate
exactly the same description to all of these architectures.

So, my next plan is to merge them into
include/asm-generic/dma-mapping.h,
then make it a single point of comments.

Each arch's <asm/dma-mapping.h> can simply wraps
<asm-generic/dma-mapping.h>

It would be beyond the scope of this series
(since my main motivation is fix the mmc/sdhci bug).

So, I want to just let this series go in.

After it lands, I can factoring them out,
and add some comments.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single()
  2020-02-05  0:16   ` Simon Glass
@ 2020-02-11 15:20     ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-11 15:20 UTC (permalink / raw)
  To: u-boot

On Wed, Feb 5, 2020 at 9:17 AM Simon Glass <sjg@chromium.org> wrote:
>
> On Tue, 4 Feb 2020 at 04:09, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > dma_unmap_single() takes the dma address, not virtual address.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  arch/arm/include/asm/dma-mapping.h   | 4 +---
> >  arch/nds32/include/asm/dma-mapping.h | 4 +---
> >  arch/riscv/include/asm/dma-mapping.h | 4 +---
> >  arch/x86/include/asm/dma-mapping.h   | 4 +---
> >  drivers/mmc/tmio-common.c            | 2 +-
> >  drivers/mtd/nand/raw/denali.c        | 2 +-
> >  drivers/net/macb.c                   | 2 +-
> >  drivers/usb/dwc3/core.c              | 6 +++---
> >  drivers/usb/gadget/udc/udc-core.c    | 2 +-
> >  9 files changed, 11 insertions(+), 19 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But please also add a comment to the header files.

Same here.

See my reply to 1/9.



--
Best Regards

Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single()
  2020-02-11 15:18     ` Masahiro Yamada
@ 2020-02-11 17:14       ` Simon Glass
  0 siblings, 0 replies; 19+ messages in thread
From: Simon Glass @ 2020-02-11 17:14 UTC (permalink / raw)
  To: u-boot

On Tue, 11 Feb 2020 at 08:19, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi Simon,
>
> On Wed, Feb 5, 2020 at 9:17 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > On Tue, 4 Feb 2020 at 04:09, Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > >
> > > Make dma_map_single() return the dma address, and remove the
> > > pointless volatile.
> > >
> > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > ---
> > >
> > >  arch/arm/include/asm/dma-mapping.h   | 5 +++--
> > >  arch/nds32/include/asm/dma-mapping.h | 5 +++--
> > >  arch/riscv/include/asm/dma-mapping.h | 5 +++--
> > >  arch/x86/include/asm/dma-mapping.h   | 5 +++--
> > >  4 files changed, 12 insertions(+), 8 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > But please can you add a full function comment in the header files?
>
>
> I can comment them, but it is tedious to duplicate
> exactly the same description to all of these architectures.
>
> So, my next plan is to merge them into
> include/asm-generic/dma-mapping.h,
> then make it a single point of comments.
>
> Each arch's <asm/dma-mapping.h> can simply wraps
> <asm-generic/dma-mapping.h>
>
> It would be beyond the scope of this series
> (since my main motivation is fix the mmc/sdhci bug).
>
> So, I want to just let this series go in.
>
> After it lands, I can factoring them out,
> and add some comments.

OK sounds good.

BTW while I agree that duplicating comments is annoying, it's better
than not having comments.

Regards,
Simon

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem.
  2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
                   ` (8 preceding siblings ...)
  2020-02-04 11:09 ` [PATCH 9/9] mmc: sdhci: fix missing cache invalidation after reading by DMA Masahiro Yamada
@ 2020-02-14  0:50 ` Peng Fan
  2020-02-14  5:29   ` Masahiro Yamada
  9 siblings, 1 reply; 19+ messages in thread
From: Peng Fan @ 2020-02-14  0:50 UTC (permalink / raw)
  To: u-boot

Hi Masahiro,

> Subject: [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency
> problem.

CI, build fail, please give a check.

      mips:  +   pic32mzdask
+drivers/mmc/sdhci.c:18:10: fatal error: asm/dma-mapping.h: No such file or directory
+ #include <asm/dma-mapping.h>
+          ^~~~~~~~~~~~~~~~~~~
+compilation terminated.
+make[3]: *** [drivers/mmc/sdhci.o] Error 1
+make[2]: *** [drivers/mmc] Error 2
+make[1]: *** [drivers] Error 2
+make: *** [sub-make] Error 2

Thanks,
Peng.

> 
> 
> My main motivation of this series is the last patch
> "mmc: sdhci: fix missing cache invalidation after reading by DMA".
> 
> Currently, read data are occasionally corrupted due to the missing cache
> invalidation.
> 
> To fix it nicely (adds dma_unmap_single(), which follows the Linux coding
> style), I did some cleaups first.
> 
> Patch 01 and 02 fix the prototypes of dma_{map,unmap}_single().
> 
> Patch 03-08 are code clean-ups.
> 
> Patch 09 fixes the bug.
> 
> 
> 
> Masahiro Yamada (9):
>   dma-mapping: fix the prototype of dma_map_single()
>   dma-mapping: fix the prototype of dma_unmap_single()
>   mmc: sdhci: put the aligned buffer pointer to struct sdhci_host
>   mmc: sdhci: reduce code duplication for aligned buffer
>   mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting
>     adma_addr
>   mmc: sdhci: remove unneeded casts
>   mmc: add mmc_get_dma_dir() helper
>   mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA
>   mmc: sdhci: fix missing cache invalidation after reading by DMA
> 
>  arch/arm/include/asm/dma-mapping.h   |  9 ++-
>  arch/nds32/include/asm/dma-mapping.h |  9 ++-
> arch/riscv/include/asm/dma-mapping.h |  9 ++-
>  arch/x86/include/asm/dma-mapping.h   |  9 ++-
>  drivers/mmc/sdhci.c                  | 96 +++++++++++++---------------
>  drivers/mmc/tmio-common.c            |  2 +-
>  drivers/mtd/nand/raw/denali.c        |  2 +-
>  drivers/net/macb.c                   |  2 +-
>  drivers/usb/dwc3/core.c              |  6 +-
>  drivers/usb/gadget/udc/udc-core.c    |  2 +-
>  include/mmc.h                        |  6 ++
>  include/sdhci.h                      |  3 +
>  12 files changed, 78 insertions(+), 77 deletions(-)
> 
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem.
  2020-02-14  0:50 ` [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Peng Fan
@ 2020-02-14  5:29   ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2020-02-14  5:29 UTC (permalink / raw)
  To: u-boot

Hi Peng,

On Fri, Feb 14, 2020 at 9:50 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi Masahiro,
>
> > Subject: [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency
> > problem.
>
> CI, build fail, please give a check.
>
>       mips:  +   pic32mzdask
> +drivers/mmc/sdhci.c:18:10: fatal error: asm/dma-mapping.h: No such file or directory
> + #include <asm/dma-mapping.h>
> +          ^~~~~~~~~~~~~~~~~~~
> +compilation terminated.
> +make[3]: *** [drivers/mmc/sdhci.o] Error 1
> +make[2]: *** [drivers/mmc] Error 2
> +make[1]: *** [drivers] Error 2
> +make: *** [sub-make] Error 2


Ah, right.

I will fix it soon.

Thanks.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-02-14  5:29 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-04 11:08 [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Masahiro Yamada
2020-02-04 11:08 ` [PATCH 1/9] dma-mapping: fix the prototype of dma_map_single() Masahiro Yamada
2020-02-05  0:16   ` Simon Glass
2020-02-11 15:18     ` Masahiro Yamada
2020-02-11 17:14       ` Simon Glass
     [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D0737@ATCPCS16.andestech.com>
2020-02-06  1:16     ` Rick Chen
2020-02-04 11:08 ` [PATCH 2/9] dma-mapping: fix the prototype of dma_unmap_single() Masahiro Yamada
2020-02-05  0:16   ` Simon Glass
2020-02-11 15:20     ` Masahiro Yamada
     [not found]   ` <752D002CFF5D0F4FA35C0100F1D73F3FA46D073E@ATCPCS16.andestech.com>
2020-02-06  1:13     ` Rick Chen
2020-02-04 11:09 ` [PATCH 3/9] mmc: sdhci: put the aligned buffer pointer to struct sdhci_host Masahiro Yamada
2020-02-04 11:09 ` [PATCH 4/9] mmc: sdhci: reduce code duplication for aligned buffer Masahiro Yamada
2020-02-04 11:09 ` [PATCH 5/9] mmc: sdhci: use lower_32_bit2() and upper_32_bits() for setting adma_addr Masahiro Yamada
2020-02-04 11:09 ` [PATCH 6/9] mmc: sdhci: remove unneeded casts Masahiro Yamada
2020-02-04 11:09 ` [PATCH 7/9] mmc: add mmc_get_dma_dir() helper Masahiro Yamada
2020-02-04 11:09 ` [PATCH 8/9] mmc: sdhci: use dma_map_single() instead of flush_cache() before DMA Masahiro Yamada
2020-02-04 11:09 ` [PATCH 9/9] mmc: sdhci: fix missing cache invalidation after reading by DMA Masahiro Yamada
2020-02-14  0:50 ` [PATCH 0/9] mmc: sdhci: code clean-up and fix cache coherency problem Peng Fan
2020-02-14  5:29   ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.