[U-Boot] [PATCH 1/4] mmc: dw_mmc: Stop bounce buffer even in case of failure

[U-Boot] [PATCH 1/4] mmc: dw_mmc: Stop bounce buffer even in case of failure

[Marek Vasut]

The driver didn't stop the bounce buffer in case a data transfer
failed. This would lead to memory leakage if the communication
between the CPU and the card is unreliable. Add the missing call
to stop the bounce buffer.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 drivers/mmc/dw_mmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 53a8aca..3fffa71 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -215,6 +215,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 			mask = dwmci_readl(host, DWMCI_RINTSTS);
 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
 				printf("%s: DATA ERROR!\n", __func__);
+				bounce_buffer_stop(&bbstate);
 				return -1;
 			}
 		} while (!(mask & DWMCI_INTMSK_DTO));
-- 
2.1.4

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Marek Vasut]

Endless timeouts are bad, since if we get stuck in one, we have no
way out. Zap this one by implementing proper timeout.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 drivers/mmc/dw_mmc.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 3fffa71..0f61f16 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -211,14 +211,29 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	}
 
 	if (data) {
-		do {
+		start = get_timer(0);
+		timeout = 1000;
+		for (;;) {
 			mask = dwmci_readl(host, DWMCI_RINTSTS);
+			/* Error during data transfer. */
 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
 				printf("%s: DATA ERROR!\n", __func__);
 				bounce_buffer_stop(&bbstate);
 				return -1;
 			}
-		} while (!(mask & DWMCI_INTMSK_DTO));
+
+			/* Data arrived correctly. */
+			if (mask & DWMCI_INTMSK_DTO)
+				break;
+
+			/* Check for timeout. */
+			if (get_timer(start) > timeout) {
+				printf("%s: Timeout waiting for data!\n",
+				       __func__);
+				bounce_buffer_stop(&bbstate);
+				return TIMEOUT;
+			}
+		}
 
 		dwmci_writel(host, DWMCI_RINTSTS, mask);
 
-- 
2.1.4

[U-Boot] [PATCH 3/4] mmc: dw_mmc: Improve handling of data transfer failure

[Marek Vasut]

In case the data transfer failure happens, instead of returning
immediatelly, make sure the DMA is disabled, status register is
cleared and the bounce buffer is stopped.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 drivers/mmc/dw_mmc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0f61f16..fcd5784 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -110,7 +110,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 	struct dwmci_host *host = mmc->priv;
 	ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
 				 data ? DIV_ROUND_UP(data->blocks, 8) : 0);
-	int flags = 0, i;
+	int ret = 0, flags = 0, i;
 	unsigned int timeout = 100000;
 	u32 retry = 10000;
 	u32 mask, ctrl;
@@ -218,20 +218,22 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 			/* Error during data transfer. */
 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
 				printf("%s: DATA ERROR!\n", __func__);
-				bounce_buffer_stop(&bbstate);
-				return -1;
+				ret = -EINVAL;
+				break;
 			}
 
 			/* Data arrived correctly. */
-			if (mask & DWMCI_INTMSK_DTO)
+			if (mask & DWMCI_INTMSK_DTO) {
+				ret = 0;
 				break;
+			}
 
 			/* Check for timeout. */
 			if (get_timer(start) > timeout) {
 				printf("%s: Timeout waiting for data!\n",
 				       __func__);
-				bounce_buffer_stop(&bbstate);
-				return TIMEOUT;
+				ret = TIMEOUT;
+				break;
 			}
 		}
 
@@ -246,7 +248,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	udelay(100);
 
-	return 0;
+	return ret;
 }
 
 static int dwmci_setup_bus(struct dwmci_host *host, u32 freq)
-- 
2.1.4

[U-Boot] [PATCH 4/4] mmc: dw_mmc: Probe the MMC from OF

[Marek Vasut]

Rework the driver to probe the MMC controller from Device Tree
and make it mandatory. There is no longer support for probing
from the ancient qts-generated header files.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 arch/arm/mach-socfpga/include/mach/dwmmc.h |  2 +-
 arch/arm/mach-socfpga/misc.c               |  3 +-
 drivers/mmc/socfpga_dw_mmc.c               | 81 +++++++++++++++++++++++++-----
 include/fdtdec.h                           |  1 +
 lib/fdtdec.c                               |  1 +
 5 files changed, 72 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-socfpga/include/mach/dwmmc.h b/arch/arm/mach-socfpga/include/mach/dwmmc.h
index 945eb64..e8ba901 100644
--- a/arch/arm/mach-socfpga/include/mach/dwmmc.h
+++ b/arch/arm/mach-socfpga/include/mach/dwmmc.h
@@ -7,6 +7,6 @@
 #ifndef	_SOCFPGA_DWMMC_H_
 #define	_SOCFPGA_DWMMC_H_
 
-extern int socfpga_dwmmc_init(u32 regbase, int bus_width, int index);
+int socfpga_dwmmc_init(const void *blob);
 
 #endif /* _SOCFPGA_SDMMC_H_ */
diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index 0f8b4d0..3ddac4c 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -92,8 +92,7 @@ int cpu_eth_init(bd_t *bis)
  */
 int cpu_mmc_init(bd_t *bis)
 {
-	return socfpga_dwmmc_init(SOCFPGA_SDMMC_ADDRESS,
-				  CONFIG_HPS_SDMMC_BUSWIDTH, 0);
+	return socfpga_dwmmc_init(gd->fdt_blob);
 }
 #endif
 
diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
index eb69aed..8076761 100644
--- a/drivers/mmc/socfpga_dw_mmc.c
+++ b/drivers/mmc/socfpga_dw_mmc.c
@@ -6,6 +6,8 @@
 
 #include <common.h>
 #include <malloc.h>
+#include <fdtdec.h>
+#include <libfdt.h>
 #include <dwmmc.h>
 #include <errno.h>
 #include <asm/arch/dwmmc.h>
@@ -42,34 +44,87 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 		CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK);
 }
 
-int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
+static int socfpga_dwmci_of_probe(const void *blob, int node, const int idx)
 {
+	/* FIXME: probe from DT eventually too/ */
+	const unsigned long clk = cm_get_mmc_controller_clk_hz();
+
 	struct dwmci_host *host;
-	unsigned long clk = cm_get_mmc_controller_clk_hz();
+	fdt_addr_t reg_base;
+	int bus_width, fifo_depth;
 
 	if (clk == 0) {
-		printf("%s: MMC clock is zero!", __func__);
+		printf("DWMMC%d: MMC clock is zero!", idx);
 		return -EINVAL;
 	}
 
-	/* calloc for zero init */
-	host = calloc(1, sizeof(struct dwmci_host));
-	if (!host) {
-		printf("%s: calloc() failed!\n", __func__);
-		return -ENOMEM;
+	/* Get the register address from the device node */
+	reg_base = fdtdec_get_addr(blob, node, "reg");
+	if (!reg_base) {
+		printf("DWMMC%d: Can't get base address\n", idx);
+		return -EINVAL;
+	}
+
+	/* Get the bus width from the device node */
+	bus_width = fdtdec_get_int(blob, node, "bus-width", 0);
+	if (bus_width <= 0) {
+		printf("DWMMC%d: Can't get bus-width\n", idx);
+		return -EINVAL;
 	}
 
+	fifo_depth = fdtdec_get_int(blob, node, "fifo-depth", 0);
+	if (fifo_depth < 0) {
+		printf("DWMMC%d: Can't get FIFO depth\n", idx);
+		return -EINVAL;
+	}
+
+	/* Allocate the host */
+	host = calloc(1, sizeof(*host));
+	if (!host)
+		return -ENOMEM;
+
 	host->name = "SOCFPGA DWMMC";
-	host->ioaddr = (void *)regbase;
+	host->ioaddr = (void *)reg_base;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;
-	host->dev_index = index;
-	/* fixed clock divide by 4 which due to the SDMMC wrapper */
+	host->dev_index = idx;
+	/* Fixed clock divide by 4 which due to the SDMMC wrapper */
 	host->bus_hz = clk;
 	host->fifoth_val = MSIZE(0x2) |
-		RX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2 - 1) |
-		TX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2);
+		RX_WMARK(fifo_depth / 2 - 1) | TX_WMARK(fifo_depth / 2);
 
 	return add_dwmci(host, host->bus_hz, 400000);
 }
 
+static int socfpga_dwmci_process_node(const void *blob, int nodes[],
+				      int count)
+{
+	int i, node, ret;
+
+	for (i = 0; i < count; i++) {
+		node = nodes[i];
+		if (node <= 0)
+			continue;
+
+		ret = socfpga_dwmci_of_probe(blob, node, i);
+		if (ret) {
+			printf("%s: failed to decode dev %d\n", __func__, i);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+int socfpga_dwmmc_init(const void *blob)
+{
+	int nodes[2];	/* Max. two controllers. */
+	int ret, count;
+
+	count = fdtdec_find_aliases_for_id(blob, "mmc",
+					   COMPAT_ALTERA_SOCFPGA_DWMMC,
+					   nodes, ARRAY_SIZE(nodes));
+
+	ret = socfpga_dwmci_process_node(blob, nodes, count);
+
+	return ret;
+}
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 2323603..1a39b9f 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -183,6 +183,7 @@ enum fdt_compat_id {
 	COMPAT_SOCIONEXT_XHCI,		/* Socionext UniPhier xHCI */
 	COMPAT_INTEL_PCH,		/* Intel PCH */
 	COMPAT_INTEL_IRQ_ROUTER,	/* Intel Interrupt Router */
+	COMPAT_ALTERA_SOCFPGA_DWMMC,	/* SoCFPGA DWMMC controller */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 232ca74..d0f81bc 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -76,6 +76,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(SOCIONEXT_XHCI, "socionext,uniphier-xhci"),
 	COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
 	COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
+	COMPAT(ALTERA_SOCFPGA_DWMMC, "altr,socfpga-dw-mshc"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
2.1.4

[U-Boot] [PATCH 1/4] mmc: dw_mmc: Stop bounce buffer even in case of failure

[Pantelis Antoniou]

Hi Marek,

> On Jul 27, 2015, at 23:39 , Marek Vasut <marex@denx.de> wrote:
> 
> The driver didn't stop the bounce buffer in case a data transfer
> failed. This would lead to memory leakage if the communication
> between the CPU and the card is unreliable. Add the missing call
> to stop the bounce buffer.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> drivers/mmc/dw_mmc.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 53a8aca..3fffa71 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -215,6 +215,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 			mask = dwmci_readl(host, DWMCI_RINTSTS);
> 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
> 				printf("%s: DATA ERROR!\n", __func__);
> +				bounce_buffer_stop(&bbstate);
> 				return -1;
> 			}
> 		} while (!(mask & DWMCI_INTMSK_DTO));
> -- 
> 2.1.4
> 

Applied, thanks.

? Pantelis

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Pantelis Antoniou]

Hi Marek,

> On Jul 27, 2015, at 23:39 , Marek Vasut <marex@denx.de> wrote:
> 
> Endless timeouts are bad, since if we get stuck in one, we have no
> way out. Zap this one by implementing proper timeout.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> drivers/mmc/dw_mmc.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 3fffa71..0f61f16 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -211,14 +211,29 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 	}
> 
> 	if (data) {
> -		do {
> +		start = get_timer(0);
> +		timeout = 1000;
> +		for (;;) {
> 			mask = dwmci_readl(host, DWMCI_RINTSTS);
> +			/* Error during data transfer. */
> 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
> 				printf("%s: DATA ERROR!\n", __func__);
> 				bounce_buffer_stop(&bbstate);
> 				return -1;
> 			}
> -		} while (!(mask & DWMCI_INTMSK_DTO));
> +
> +			/* Data arrived correctly. */
> +			if (mask & DWMCI_INTMSK_DTO)
> +				break;
> +
> +			/* Check for timeout. */
> +			if (get_timer(start) > timeout) {
> +				printf("%s: Timeout waiting for data!\n",
> +				       __func__);
> +				bounce_buffer_stop(&bbstate);
> +				return TIMEOUT;
> +			}
> +		}
> 
> 		dwmci_writel(host, DWMCI_RINTSTS, mask);
> 
> -- 
> 2.1.4

Applied, thanks

? Pantelis

[U-Boot] [PATCH 3/4] mmc: dw_mmc: Improve handling of data transfer failure

[Pantelis Antoniou]

Hi Marek,

> On Jul 27, 2015, at 23:39 , Marek Vasut <marex@denx.de> wrote:
> 
> In case the data transfer failure happens, instead of returning
> immediatelly, make sure the DMA is disabled, status register is
> cleared and the bounce buffer is stopped.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> drivers/mmc/dw_mmc.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 0f61f16..fcd5784 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -110,7 +110,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 	struct dwmci_host *host = mmc->priv;
> 	ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
> 				 data ? DIV_ROUND_UP(data->blocks, 8) : 0);
> -	int flags = 0, i;
> +	int ret = 0, flags = 0, i;
> 	unsigned int timeout = 100000;
> 	u32 retry = 10000;
> 	u32 mask, ctrl;
> @@ -218,20 +218,22 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 			/* Error during data transfer. */
> 			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
> 				printf("%s: DATA ERROR!\n", __func__);
> -				bounce_buffer_stop(&bbstate);
> -				return -1;
> +				ret = -EINVAL;
> +				break;
> 			}
> 
> 			/* Data arrived correctly. */
> -			if (mask & DWMCI_INTMSK_DTO)
> +			if (mask & DWMCI_INTMSK_DTO) {
> +				ret = 0;
> 				break;
> +			}
> 
> 			/* Check for timeout. */
> 			if (get_timer(start) > timeout) {
> 				printf("%s: Timeout waiting for data!\n",
> 				       __func__);
> -				bounce_buffer_stop(&bbstate);
> -				return TIMEOUT;
> +				ret = TIMEOUT;
> +				break;
> 			}
> 		}
> 
> @@ -246,7 +248,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> 
> 	udelay(100);
> 
> -	return 0;
> +	return ret;
> }
> 
> static int dwmci_setup_bus(struct dwmci_host *host, u32 freq)
> -- 
> 2.1.4

Applied, thanks

? Pantelis

[U-Boot] [PATCH 4/4] mmc: dw_mmc: Probe the MMC from OF

[Pantelis Antoniou]

Hi Marek,

> On Jul 27, 2015, at 23:39 , Marek Vasut <marex@denx.de> wrote:
> 
> Rework the driver to probe the MMC controller from Device Tree
> and make it mandatory. There is no longer support for probing
> from the ancient qts-generated header files.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
> arch/arm/mach-socfpga/include/mach/dwmmc.h |  2 +-
> arch/arm/mach-socfpga/misc.c               |  3 +-
> drivers/mmc/socfpga_dw_mmc.c               | 81 +++++++++++++++++++++++++-----
> include/fdtdec.h                           |  1 +
> lib/fdtdec.c                               |  1 +
> 5 files changed, 72 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/include/mach/dwmmc.h b/arch/arm/mach-socfpga/include/mach/dwmmc.h
> index 945eb64..e8ba901 100644
> --- a/arch/arm/mach-socfpga/include/mach/dwmmc.h
> +++ b/arch/arm/mach-socfpga/include/mach/dwmmc.h
> @@ -7,6 +7,6 @@
> #ifndef	_SOCFPGA_DWMMC_H_
> #define	_SOCFPGA_DWMMC_H_
> 
> -extern int socfpga_dwmmc_init(u32 regbase, int bus_width, int index);
> +int socfpga_dwmmc_init(const void *blob);
> 
> #endif /* _SOCFPGA_SDMMC_H_ */
> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> index 0f8b4d0..3ddac4c 100644
> --- a/arch/arm/mach-socfpga/misc.c
> +++ b/arch/arm/mach-socfpga/misc.c
> @@ -92,8 +92,7 @@ int cpu_eth_init(bd_t *bis)
>  */
> int cpu_mmc_init(bd_t *bis)
> {
> -	return socfpga_dwmmc_init(SOCFPGA_SDMMC_ADDRESS,
> -				  CONFIG_HPS_SDMMC_BUSWIDTH, 0);
> +	return socfpga_dwmmc_init(gd->fdt_blob);
> }
> #endif
> 
> diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
> index eb69aed..8076761 100644
> --- a/drivers/mmc/socfpga_dw_mmc.c
> +++ b/drivers/mmc/socfpga_dw_mmc.c
> @@ -6,6 +6,8 @@
> 
> #include <common.h>
> #include <malloc.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> #include <dwmmc.h>
> #include <errno.h>
> #include <asm/arch/dwmmc.h>
> @@ -42,34 +44,87 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
> 		CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK);
> }
> 
> -int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
> +static int socfpga_dwmci_of_probe(const void *blob, int node, const int idx)
> {
> +	/* FIXME: probe from DT eventually too/ */
> +	const unsigned long clk = cm_get_mmc_controller_clk_hz();
> +
> 	struct dwmci_host *host;
> -	unsigned long clk = cm_get_mmc_controller_clk_hz();
> +	fdt_addr_t reg_base;
> +	int bus_width, fifo_depth;
> 
> 	if (clk == 0) {
> -		printf("%s: MMC clock is zero!", __func__);
> +		printf("DWMMC%d: MMC clock is zero!", idx);
> 		return -EINVAL;
> 	}
> 
> -	/* calloc for zero init */
> -	host = calloc(1, sizeof(struct dwmci_host));
> -	if (!host) {
> -		printf("%s: calloc() failed!\n", __func__);
> -		return -ENOMEM;
> +	/* Get the register address from the device node */
> +	reg_base = fdtdec_get_addr(blob, node, "reg");
> +	if (!reg_base) {
> +		printf("DWMMC%d: Can't get base address\n", idx);
> +		return -EINVAL;
> +	}
> +
> +	/* Get the bus width from the device node */
> +	bus_width = fdtdec_get_int(blob, node, "bus-width", 0);
> +	if (bus_width <= 0) {
> +		printf("DWMMC%d: Can't get bus-width\n", idx);
> +		return -EINVAL;
> 	}
> 
> +	fifo_depth = fdtdec_get_int(blob, node, "fifo-depth", 0);
> +	if (fifo_depth < 0) {
> +		printf("DWMMC%d: Can't get FIFO depth\n", idx);
> +		return -EINVAL;
> +	}
> +
> +	/* Allocate the host */
> +	host = calloc(1, sizeof(*host));
> +	if (!host)
> +		return -ENOMEM;
> +
> 	host->name = "SOCFPGA DWMMC";
> -	host->ioaddr = (void *)regbase;
> +	host->ioaddr = (void *)reg_base;
> 	host->buswidth = bus_width;
> 	host->clksel = socfpga_dwmci_clksel;
> -	host->dev_index = index;
> -	/* fixed clock divide by 4 which due to the SDMMC wrapper */
> +	host->dev_index = idx;
> +	/* Fixed clock divide by 4 which due to the SDMMC wrapper */
> 	host->bus_hz = clk;
> 	host->fifoth_val = MSIZE(0x2) |
> -		RX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2 - 1) |
> -		TX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2);
> +		RX_WMARK(fifo_depth / 2 - 1) | TX_WMARK(fifo_depth / 2);
> 
> 	return add_dwmci(host, host->bus_hz, 400000);
> }
> 
> +static int socfpga_dwmci_process_node(const void *blob, int nodes[],
> +				      int count)
> +{
> +	int i, node, ret;
> +
> +	for (i = 0; i < count; i++) {
> +		node = nodes[i];
> +		if (node <= 0)
> +			continue;
> +
> +		ret = socfpga_dwmci_of_probe(blob, node, i);
> +		if (ret) {
> +			printf("%s: failed to decode dev %d\n", __func__, i);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +int socfpga_dwmmc_init(const void *blob)
> +{
> +	int nodes[2];	/* Max. two controllers. */
> +	int ret, count;
> +
> +	count = fdtdec_find_aliases_for_id(blob, "mmc",
> +					   COMPAT_ALTERA_SOCFPGA_DWMMC,
> +					   nodes, ARRAY_SIZE(nodes));
> +
> +	ret = socfpga_dwmci_process_node(blob, nodes, count);
> +
> +	return ret;
> +}
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 2323603..1a39b9f 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -183,6 +183,7 @@ enum fdt_compat_id {
> 	COMPAT_SOCIONEXT_XHCI,		/* Socionext UniPhier xHCI */
> 	COMPAT_INTEL_PCH,		/* Intel PCH */
> 	COMPAT_INTEL_IRQ_ROUTER,	/* Intel Interrupt Router */
> +	COMPAT_ALTERA_SOCFPGA_DWMMC,	/* SoCFPGA DWMMC controller */
> 
> 	COMPAT_COUNT,
> };
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 232ca74..d0f81bc 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -76,6 +76,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
> 	COMPAT(SOCIONEXT_XHCI, "socionext,uniphier-xhci"),
> 	COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
> 	COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
> +	COMPAT(ALTERA_SOCFPGA_DWMMC, "altr,socfpga-dw-mshc"),
> };
> 
> const char *fdtdec_get_compatible(enum fdt_compat_id id)
> -- 
> 2.1.4

Since this does not apply cleanly now, please rework and resent again.
The rest of the patchset has been applied.

Regards

? Pantelis

[U-Boot] [PATCH V2 4/4] mmc: dw_mmc: Probe the MMC from OF

[Marek Vasut]

Rework the driver to probe the MMC controller from Device Tree
and make it mandatory. There is no longer support for probing
from the ancient qts-generated header files.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
Cc: Tom Rini <trini@konsulko.com>
---
 arch/arm/mach-socfpga/include/mach/dwmmc.h |  2 +-
 arch/arm/mach-socfpga/misc.c               |  9 +---
 drivers/mmc/socfpga_dw_mmc.c               | 81 +++++++++++++++++++++++++-----
 include/fdtdec.h                           |  1 +
 lib/fdtdec.c                               |  1 +
 5 files changed, 72 insertions(+), 22 deletions(-)

V2: Rebase the patch for Panto :-)

diff --git a/arch/arm/mach-socfpga/include/mach/dwmmc.h b/arch/arm/mach-socfpga/include/mach/dwmmc.h
index 945eb64..e8ba901 100644
--- a/arch/arm/mach-socfpga/include/mach/dwmmc.h
+++ b/arch/arm/mach-socfpga/include/mach/dwmmc.h
@@ -7,6 +7,6 @@
 #ifndef	_SOCFPGA_DWMMC_H_
 #define	_SOCFPGA_DWMMC_H_
 
-extern int socfpga_dwmmc_init(u32 regbase, int bus_width, int index);
+int socfpga_dwmmc_init(const void *blob);
 
 #endif /* _SOCFPGA_SDMMC_H_ */
diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
index 6128d54..0940cc5 100644
--- a/arch/arm/mach-socfpga/misc.c
+++ b/arch/arm/mach-socfpga/misc.c
@@ -125,14 +125,7 @@ int cpu_eth_init(bd_t *bis)
  */
 int cpu_mmc_init(bd_t *bis)
 {
-/*
- * FIXME: Temporarily define CONFIG_HPS_SDMMC_BUSWIDTH to prevent breakage
- *        due to missing patches in u-boot/master . The upcoming patch will
- *        switch this to OF probing, so this whole block will go away.
- */
-#define CONFIG_HPS_SDMMC_BUSWIDTH	8
-	return socfpga_dwmmc_init(SOCFPGA_SDMMC_ADDRESS,
-				  CONFIG_HPS_SDMMC_BUSWIDTH, 0);
+	return socfpga_dwmmc_init(gd->fdt_blob);
 }
 #endif
 
diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c
index eb69aed..8076761 100644
--- a/drivers/mmc/socfpga_dw_mmc.c
+++ b/drivers/mmc/socfpga_dw_mmc.c
@@ -6,6 +6,8 @@
 
 #include <common.h>
 #include <malloc.h>
+#include <fdtdec.h>
+#include <libfdt.h>
 #include <dwmmc.h>
 #include <errno.h>
 #include <asm/arch/dwmmc.h>
@@ -42,34 +44,87 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 		CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK);
 }
 
-int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
+static int socfpga_dwmci_of_probe(const void *blob, int node, const int idx)
 {
+	/* FIXME: probe from DT eventually too/ */
+	const unsigned long clk = cm_get_mmc_controller_clk_hz();
+
 	struct dwmci_host *host;
-	unsigned long clk = cm_get_mmc_controller_clk_hz();
+	fdt_addr_t reg_base;
+	int bus_width, fifo_depth;
 
 	if (clk == 0) {
-		printf("%s: MMC clock is zero!", __func__);
+		printf("DWMMC%d: MMC clock is zero!", idx);
 		return -EINVAL;
 	}
 
-	/* calloc for zero init */
-	host = calloc(1, sizeof(struct dwmci_host));
-	if (!host) {
-		printf("%s: calloc() failed!\n", __func__);
-		return -ENOMEM;
+	/* Get the register address from the device node */
+	reg_base = fdtdec_get_addr(blob, node, "reg");
+	if (!reg_base) {
+		printf("DWMMC%d: Can't get base address\n", idx);
+		return -EINVAL;
+	}
+
+	/* Get the bus width from the device node */
+	bus_width = fdtdec_get_int(blob, node, "bus-width", 0);
+	if (bus_width <= 0) {
+		printf("DWMMC%d: Can't get bus-width\n", idx);
+		return -EINVAL;
 	}
 
+	fifo_depth = fdtdec_get_int(blob, node, "fifo-depth", 0);
+	if (fifo_depth < 0) {
+		printf("DWMMC%d: Can't get FIFO depth\n", idx);
+		return -EINVAL;
+	}
+
+	/* Allocate the host */
+	host = calloc(1, sizeof(*host));
+	if (!host)
+		return -ENOMEM;
+
 	host->name = "SOCFPGA DWMMC";
-	host->ioaddr = (void *)regbase;
+	host->ioaddr = (void *)reg_base;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;
-	host->dev_index = index;
-	/* fixed clock divide by 4 which due to the SDMMC wrapper */
+	host->dev_index = idx;
+	/* Fixed clock divide by 4 which due to the SDMMC wrapper */
 	host->bus_hz = clk;
 	host->fifoth_val = MSIZE(0x2) |
-		RX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2 - 1) |
-		TX_WMARK(CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH / 2);
+		RX_WMARK(fifo_depth / 2 - 1) | TX_WMARK(fifo_depth / 2);
 
 	return add_dwmci(host, host->bus_hz, 400000);
 }
 
+static int socfpga_dwmci_process_node(const void *blob, int nodes[],
+				      int count)
+{
+	int i, node, ret;
+
+	for (i = 0; i < count; i++) {
+		node = nodes[i];
+		if (node <= 0)
+			continue;
+
+		ret = socfpga_dwmci_of_probe(blob, node, i);
+		if (ret) {
+			printf("%s: failed to decode dev %d\n", __func__, i);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+int socfpga_dwmmc_init(const void *blob)
+{
+	int nodes[2];	/* Max. two controllers. */
+	int ret, count;
+
+	count = fdtdec_find_aliases_for_id(blob, "mmc",
+					   COMPAT_ALTERA_SOCFPGA_DWMMC,
+					   nodes, ARRAY_SIZE(nodes));
+
+	ret = socfpga_dwmci_process_node(blob, nodes, count);
+
+	return ret;
+}
diff --git a/include/fdtdec.h b/include/fdtdec.h
index eac679e..cd2c926 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -182,6 +182,7 @@ enum fdt_compat_id {
 	COMPAT_INTEL_PCH,		/* Intel PCH */
 	COMPAT_INTEL_IRQ_ROUTER,	/* Intel Interrupt Router */
 	COMPAT_ALTERA_SOCFPGA_DWMAC,	/* SoCFPGA Ethernet controller */
+	COMPAT_ALTERA_SOCFPGA_DWMMC,	/* SoCFPGA DWMMC controller */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index b201787..7023e55 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -76,6 +76,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(COMPAT_INTEL_PCH, "intel,bd82x6x"),
 	COMPAT(COMPAT_INTEL_IRQ_ROUTER, "intel,irq-router"),
 	COMPAT(ALTERA_SOCFPGA_DWMAC, "altr,socfpga-stmmac"),
+	COMPAT(ALTERA_SOCFPGA_DWMMC, "altr,socfpga-dw-mshc"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
2.1.4

[U-Boot] [PATCH 4/4] mmc: dw_mmc: Probe the MMC from OF

[Marek Vasut]

On Wednesday, August 12, 2015 at 09:35:22 AM, Pantelis Antoniou wrote:
> Hi Marek,

Hi!

> > On Jul 27, 2015, at 23:39 , Marek Vasut <marex@denx.de> wrote:
> > 
> > Rework the driver to probe the MMC controller from Device Tree
> > and make it mandatory. There is no longer support for probing
> > from the ancient qts-generated header files.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> > Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> > Cc: Tom Rini <trini@konsulko.com>

Done :)

Best regards,
Marek Vasut

[U-Boot] [PATCH V2 4/4] mmc: dw_mmc: Probe the MMC from OF

[Marek Vasut]

On Wednesday, August 12, 2015 at 10:43:26 PM, Marek Vasut wrote:
> Rework the driver to probe the MMC controller from Device Tree
> and make it mandatory. There is no longer support for probing
> from the ancient qts-generated header files.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>

Bump?

Best regards,
Marek Vasut

[U-Boot] [PATCH V2 4/4] mmc: dw_mmc: Probe the MMC from OF

[Pantelis Antoniou]

Plumbers :)

???????? ??? ?? iPhone ???

19 ??? 2015, 14:58, ?/? Marek Vasut <marex@denx.de> ??????:

>> On Wednesday, August 12, 2015 at 10:43:26 PM, Marek Vasut wrote:
>> Rework the driver to probe the MMC controller from Device Tree
>> and make it mandatory. There is no longer support for probing
>> from the ancient qts-generated header files.
>> 
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Cc: Tom Rini <trini@konsulko.com>
> 
> Bump?
> 
> Best regards,
> Marek Vasut

[U-Boot] [PATCH V2 4/4] mmc: dw_mmc: Probe the MMC from OF

[Marek Vasut]

On Thursday, August 20, 2015 at 12:55:41 AM, Pantelis Antoniou wrote:
> Plumbers :)

Right, your office assistant already mentioned this ;-)

> ???????? ??? ?? iPhone ???
> 
> 19 ??? 2015, 14:58, ?/? Marek Vasut <marex@denx.de> ??????:
> >> On Wednesday, August 12, 2015 at 10:43:26 PM, Marek Vasut wrote:
> >> Rework the driver to probe the MMC controller from Device Tree
> >> and make it mandatory. There is no longer support for probing
> >> from the ancient qts-generated header files.
> >> 
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> >> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> Cc: Tom Rini <trini@konsulko.com>
> > 
> > Bump?
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Alexey Brodkin]

Hi Marek,

On Mon, 2015-07-27 at 22:39 +0200, Marek Vasut wrote:
> Endless timeouts are bad, since if we get stuck in one, we have no
> way out. Zap this one by implementing proper timeout.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>  drivers/mmc/dw_mmc.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 3fffa71..0f61f16 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -211,14 +211,29 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>  	}
>  
>  	if (data) {
> -		do {
> +		start = get_timer(0);
> +		timeout = 1000;
> +		for (;;) {
>  			mask = dwmci_readl(host, DWMCI_RINTSTS);
> +			/* Error during data transfer. */
>  			if (mask & (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
>  				printf("%s: DATA ERROR!\n", __func__);
>  				bounce_buffer_stop(&bbstate);
>  				return -1;
>  			}
> -		} while (!(mask & DWMCI_INTMSK_DTO));
> +
> +			/* Data arrived correctly. */
> +			if (mask & DWMCI_INTMSK_DTO)
> +				break;
> +
> +			/* Check for timeout. */
> +			if (get_timer(start) > timeout) {
> +				printf("%s: Timeout waiting for data!\n",
> +				       __func__);
> +				bounce_buffer_stop(&bbstate);
> +				return TIMEOUT;
> +			}
> +		}
>  
>  		dwmci_writel(host, DWMCI_RINTSTS, mask);
>  

It turned out that patch breaks functionality in some cases.
For me on every attempt to download something significant (at least I see it on
5/7 Mb files) from SD I'm seeing timeout firing too early.

I added a bit of extra instrumentation to see where time is spent and why.

So my diff is:
----------------------------------->8--------------------------------
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 77b87e0..2da77a7 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -213,7 +213,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 
        if (data) {
                start = get_timer(0);
-               timeout = 1000;
+               timeout = 10000; // That's required to get to the end of the transfer
                for (;;) {
                        mask = dwmci_readl(host, DWMCI_RINTSTS);
                        /* Error during data transfer. */
@@ -226,6 +226,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
                        /* Data arrived correctly. */
                        if (mask & DWMCI_INTMSK_DTO) {
                                ret = 0;
+                               printf(" * time spent: %d, data size: %d, blocks: %d\n", (int)get_timer(start), data
->blocksize * data->blocks, data->blocks);
                                break;
                        }
----------------------------------->8--------------------------------

And that's what I see then:
----------------------------------->8--------------------------------
AXS# fatload mmc 0
 * time spent: 0, data size: 8, blocks: 1
 * time spent: 0, data size: 512, blocks: 1
 * time spent: 0, data size: 512, blocks: 1
 * time spent: 0, data size: 512, blocks: 1
reading uImage
 * time spent: 1, data size: 512, blocks: 1
 * time spent: 0, data size: 1024, blocks: 2
 * time spent: 1, data size: 3072, blocks: 6
 * time spent: 1, data size: 3072, blocks: 6
 * time spent: 1, data size: 3072, blocks: 6
 * time spent: 0, data size: 3072, blocks: 6
 * time spent: 0, data size: 3072, blocks: 6
 * time spent: 1599, data size: 13338112, blocks: 26051
 * time spent: 0, data size: 512, blocks: 1
13338188 bytes read in 1651 ms (7.7 MiB/s)
----------------------------------->8--------------------------------

So you see real data transfer takes  ~1.7 seconds when getting 26k blocks.

In other words timeout check has to be a bit smarter, for example
taking into account number of blocks to be transferred.

Any thoughts?

-Alexey

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Marek Vasut]

On Friday, September 11, 2015 at 09:59:32 AM, Alexey Brodkin wrote:
> Hi Marek,

Hi!

> On Mon, 2015-07-27 at 22:39 +-0200, Marek Vasut wrote:
> +AD4- Endless timeouts are bad, since if we get stuck in one, we have no
> +AD4- way out. Zap this one by implementing proper timeout.
> +AD4-
> +AD4- Signed-off-by: Marek Vasut +ADw-marex+AEA-denx.de+AD4-
> +AD4- Cc: Dinh Nguyen +ADw-dinguyen+AEA-opensource.altera.com+AD4-
> +AD4- Cc: Pantelis Antoniou +ADw-panto+AEA-antoniou-consulting.com+AD4-
> +AD4- Cc: Tom Rini +ADw-trini+AEA-konsulko.com+AD4-
> +AD4- ---
> +AD4-  drivers/mmc/dw+AF8-mmc.c +AHw- 19
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+--- +AD4-  1 file changed, 17
> insertions(+-), 2 deletions(-)
> +AD4-
> +AD4- diff --git a/drivers/mmc/dw+AF8-mmc.c b/drivers/mmc/dw+AF8-mmc.c
> +AD4- index 3fffa71..0f61f16 100644
> +AD4- --- a/drivers/mmc/dw+AF8-mmc.c
> +AD4- +-+-+- b/drivers/mmc/dw+AF8-mmc.c
> +AD4- +AEAAQA- -211,14 +-211,29 +AEAAQA- static int
> dwmci+AF8-send+AF8-cmd(struct mmc +ACo-mmc, struct mmc+AF8-cmd +ACo-cmd,
> +AD4-  	+AH0-
> +AD4-
> +AD4-  	if (data) +AHs-
> +AD4- -		do +AHs-
> +AD4- +-		start +AD0- get+AF8-timer(0)+ADs-
> +AD4- +-		timeout +AD0- 1000+ADs-
> +AD4- +-		for (+ADsAOw-) +AHs-
> +AD4-  			mask +AD0- dwmci+AF8-readl(host, DWMCI+AF8-
RINTSTS)+ADs-
> +AD4- +-			/+ACo- Error during data transfer. +ACo-/
> +AD4-  			if (mask +ACY- (DWMCI+AF8-DATA+AF8-ERR +AHw-
> DWMCI+AF8-DATA+AF8-TOUT)) +AHs- +AD4-  				
printf(+ACIAJQ-s: DATA
> ERROR+ACEAXA-n+ACI-, +AF8AXw-func+AF8AXw-)+ADs- +AD4- 
> 				bounce+AF8-buffer+AF8-stop(+ACY-bbstate)+ADs-
> +AD4-  				return -1+ADs-
> +AD4-  			+AH0-
> +AD4- -		+AH0- while (+ACE-(mask +ACY- DWMCI+AF8-INTMSK+AF8-
DTO))+ADs-
> +AD4- +-
> +AD4- +-			/+ACo- Data arrived correctly. +ACo-/
> +AD4- +-			if (mask +ACY- DWMCI+AF8-INTMSK+AF8-DTO)
> +AD4- +-				break+ADs-
> +AD4- +-
> +AD4- +-			/+ACo- Check for timeout. +ACo-/
> +AD4- +-			if (get+AF8-timer(start) +AD4- timeout) +AHs-
> +AD4- +-				printf(+ACIAJQ-s: Timeout waiting for 
data+ACEAXA-n+ACI-,
> +AD4- +-				       +AF8AXw-func+AF8AXw-)+ADs-
> +AD4- +-				bounce+AF8-buffer+AF8-stop(+ACY-
bbstate)+ADs-
> +AD4- +-				return TIMEOUT+ADs-
> +AD4- +-			+AH0-
> +AD4- +-		+AH0-
> +AD4-
> +AD4-  		dwmci+AF8-writel(host, DWMCI+AF8-RINTSTS, mask)+ADs-
> +AD4-

btw Is your mailer totally broken by any chance ?

> It turned out that patch breaks functionality in some cases.
> For me on every attempt to download something significant (at least I see
> it on 5/7 Mb files) from SD I'm seeing timeout firing too early.
> 
> I added a bit of extra instrumentation to see where time is spent and why.

Check this patch:

[PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds

https://patchwork.ozlabs.org/patch/511899/

Does it fix things for you ?

[...]

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Alexey Brodkin]

Hi Marek,

On Fri, 2015-09-11 at 13:49 +0200, Marek Vasut wrote:
> On Friday, September 11, 2015 at 09:59:32 AM, Alexey Brodkin wrote:
> > Hi Marek,
> 
> Hi!

> btw Is your mailer totally broken by any chance ?

Hm, I'm not sure what happened but as I may see here
https://patchwork.ozlabs.org/patch/516618/ my message looks good :)

> > It turned out that patch breaks functionality in some cases.
> > For me on every attempt to download something significant (at least I see
> > it on 5/7 Mb files) from SD I'm seeing timeout firing too early.
> > 
> > I added a bit of extra instrumentation to see where time is spent and why.
> 
> Check this patch:
> 
> [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds
> 
> https://patchwork.ozlabs.org/patch/511899/
> 
> Does it fix things for you ?

Well this might fix my particular test-case, but are you sure there's
no chance for this timeout to be not long enough?

And vice versa why wait 20 seconds if problem has happened on short
transfer? Really wait 20 seconds on boot of say TV-set just because
USB-drive is broken?

So I would say that we need to rely on amount of data to be transferred
instead of having any random number of seconds for all.

-Alexey

[U-Boot] [PATCH 2/4] mmc: dw_mmc: Zap endless timeout

[Marek Vasut]

On Friday, September 11, 2015 at 07:04:42 PM, Alexey Brodkin wrote:
> Hi Marek,

Hi,

> On Fri, 2015-09-11 at 13:49 +-0200, Marek Vasut wrote:
> +AD4- On Friday, September 11, 2015 at 09:59:32 AM, Alexey Brodkin wrote:
> +AD4- +AD4- Hi Marek,
> +AD4-
> +AD4- Hi+ACE-
> 
> +AD4- btw Is your mailer totally broken by any chance ?
> 
> Hm, I'm not sure what happened but as I may see here
> https://patchwork.ozlabs.org/patch/516618/ my message looks good :)

That's great.

> +AD4- +AD4- It turned out that patch breaks functionality in some cases.
> +AD4- +AD4- For me on every attempt to download something significant (at
> least I see +AD4- +AD4- it on 5/7 Mb files) from SD I'm seeing timeout
> firing too early. +AD4- +AD4-
> +AD4- +AD4- I added a bit of extra instrumentation to see where time is
> spent and why. +AD4-
> +AD4- Check this patch:
> +AD4-
> +AD4- +AFs-PATCH 1/2+AF0- mmc: dw+AF8-mmc: Increase timeout to 20 seconds
> +AD4-
> +AD4- https://patchwork.ozlabs.org/patch/511899/
> +AD4-
> +AD4- Does it fix things for you ?
> 
> Well this might fix my particular test-case, but are you sure there's
> no chance for this timeout to be not long enough?

There is, the crappier the card, the higher the possibility.

> And vice versa why wait 20 seconds if problem has happened on short
> transfer? Really wait 20 seconds on boot of say TV-set just because
> USB-drive is broken?

It's still better to wait 20 seconds instead of waiting indefinitelly, right?
That is the reason for my patch, which removed the unbounded loop.

> So I would say that we need to rely on amount of data to be transferred
> instead of having any random number of seconds for all.

Let's move this discussion into the dwmmc thread by Lukasz. There is more
to it than just the amount of data transferred.

Best regards,
Marek Vasut