fix and enhancement patches for sdhci

This seris fix several issue like flush cache and build warning. And give this generic driver enhancement for timeout when transferring data and additional structure member to access in platform self driver.

Lei Wen (4):
mmc: sdhci: fix cache flush
mmc: sdhci: fix build warning
mmc: sdhci: add mmc structure for host
mmc: sdhci: add timeout for data transfer

 drivers/mmc/sdhci.c |   12 ++++++++++--
 include/sdhci.h     |    3 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

Only flush the memory range needed.

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

index 9ebd33d..4a92453 100644
@@ -196,7 +196,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	sdhci_writel(host, cmd->cmdarg, SDHCI_ARGUMENT);
 #ifdef CONFIG_MMC_SDMA
-	flush_cache(0, ~0);
+	flush_cache(start_addr, trans_bytes);
 #endif
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->cmdidx, flags), SDHCI_COMMAND);
 	do {

1.7.0.4

If CONFIG_MMC_SDHCI_IO_ACCESSORS is defined, the following warning would shows up:

include/sdhci.h:224: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:224: warning: its scope is only this definition or declaration, which is probably not what you want include/sdhci.h:225: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:226: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:227: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:228: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:229: warning: 'struct sdhci_host' declared inside parameter list

---
 include/sdhci.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

index 6d52ce9..6c6a8ea 100644
@@ -219,6 +219,7 @@
  */
 #define SDHCI_DEFAULT_BOUNDARY_SIZE	(512 * 1024)
 #define SDHCI_DEFAULT_BOUNDARY_ARG	(7)
+struct sdhci_host;
 struct sdhci_ops {
 #ifdef CONFIG_MMC_SDHCI_IO_ACCESSORS
 	u32             (*read_l)(struct sdhci_host *host, int reg);

1.7.0.4

[...]

Marek,

Got it,

Thanks,
Lei

So that sdhci host would tell in the driver that the mmc current attributes.

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

index 4a92453..31c738e 100644
@@ -377,6 +377,7 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk)
 	}
 
 	mmc->priv = host;
+	host->mmc = mmc;
 
 	sprintf(mmc->name, "%s", host->name);
 	mmc->send_cmd = sdhci_send_command;

index 6c6a8ea..2dd36a4 100644
@@ -27,6 +27,7 @@
 #define __SDHCI_HW_H
 
 #include <asm/io.h>
+#include <mmc.h>
 /*
  * Controller registers
  */

@@ -237,6 +238,7 @@ struct sdhci_host {
 	unsigned int quirks;
 	unsigned int version;
 	unsigned int clock;
+	struct mmc *mmc;
 	const struct sdhci_ops *ops;
 };
 

1.7.0.4

---
 drivers/mmc/sdhci.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

index 31c738e..77a9e70 100644
@@ -81,8 +81,9 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 				unsigned int start_addr)
 {
-	unsigned int stat, rdy, mask, block = 0;
+	unsigned int stat, rdy, mask, timeout, block = 0;
 
+	timeout = 10000;
 	rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
 	mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
 	do {

@@ -108,6 +109,12 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}
 #endif
+		if (timeout-- > 0)
+			udelay(10);
+		else {
+			printf("Transfer data timeout\n");
+			return -1;
+		}
 	} while (!(stat & SDHCI_INT_DATA_END));
 	return 0;
 }

1.7.0.4

This seris fix several issue like flush cache and build warning. And give this generic driver enhancement for timeout when transferring data and additional structure member to access in platform self driver.

Changelog:
V2: code style change.

Lei Wen (4):
mmc: sdhci: fix cache flush
mmc: sdhci: fix build warning
mmc: sdhci: add mmc structure for host
mmc: sdhci: add timeout for data transfer

 drivers/mmc/sdhci.c |   12 ++++++++++--
 include/sdhci.h     |    3 +++
 2 files changed, 13 insertions(+), 2 deletions(-)

This seris fix several issue like flush cache and build warning. And give this generic driver enhancement for timeout when transferring data and additional structure member to access in platform self driver.

Changelog:
V2: code style change.

V3: add another fix of sdma handling bug including in this series

Lei Wen (5):
mmc: sdhci: fix cache flush
mmc: sdhci: fix build warning
mmc: sdhci: add mmc structure for host
mmc: sdhci: add timeout for data transfer
mmc: sdhci: fix sdma bug for large file transfer

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

This seris fix several issue like flush cache and build warning. And give this generic driver enhancement for timeout when transferring data and additional structure member to access in platform self driver.

Changelog:
V2: code style change.
V3: add another fix of sdma handling bug including in this series
V4: minor code style change

Lei Wen (5):
mmc: sdhci: fix cache flush
mmc: sdhci: fix build warning
mmc: sdhci: add mmc structure for host
mmc: sdhci: add timeout for data transfer
mmc: sdhci: fix sdma bug for large file transfer

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

Hi Andy,

Any comment for this patch series?

Thanks,
Lei

Hi Andy,

Any comments?

Thanks,
Lei

Sorry for the delay, I am currently processing my patch backlog. I will be applying them today.

Only flush the memory range needed.

---
Changelog:
V2: no change
V3: no change
V4: no change

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

index 9ebd33d..4a92453 100644
@@ -196,7 +196,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	sdhci_writel(host, cmd->cmdarg, SDHCI_ARGUMENT);
 #ifdef CONFIG_MMC_SDMA
-	flush_cache(0, ~0);
+	flush_cache(start_addr, trans_bytes);
 #endif
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->cmdidx, flags), SDHCI_COMMAND);
 	do {

1.7.0.4

If CONFIG_MMC_SDHCI_IO_ACCESSORS is defined, the following warning would shows up:

include/sdhci.h:224: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:224: warning: its scope is only this definition or declaration, which is probably not what you want include/sdhci.h:225: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:226: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:227: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:228: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:229: warning: 'struct sdhci_host' declared inside parameter list

---
Changelog:
V2: code style change
V2.1: delete "Change-Id"
V3: no change
V4: no change

  include/sdhci.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 6d52ce9..e84d2dc 100644
@@ -214,6 +214,9 @@
  */
 #define SDHCI_QUIRK_32BIT_DMA_ADDR	(1 << 0)
 
+/* to make gcc happy */
+struct sdhci_host;
+
 /*
  * Host SDMA buffer boundary. Valid values from 4K to 512K in powers of 2.
  */

1.7.0.4

So that sdhci host would tell in the driver that the mmc current attributes.

---
Changelog:
V2: no change
V3: no change
V4: add empty line after mmc.h include

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

index 4a92453..31c738e 100644
@@ -377,6 +377,7 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk)
 	}
 
 	mmc->priv = host;
+	host->mmc = mmc;
 
 	sprintf(mmc->name, "%s", host->name);
 	mmc->send_cmd = sdhci_send_command;

index e84d2dc..0690938 100644
@@ -27,6 +27,8 @@
 #define __SDHCI_HW_H
 
 #include <asm/io.h>
+#include <mmc.h>
+
 /*
  * Controller registers
  */

@@ -239,6 +241,7 @@ struct sdhci_host {
 	unsigned int quirks;
 	unsigned int version;
 	unsigned int clock;
+	struct mmc *mmc;
 	const struct sdhci_ops *ops;
 };
 

1.7.0.4

---
Changelog:
V2: no change
V3: no change
V4: no change

 drivers/mmc/sdhci.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

index 31c738e..77a9e70 100644
@@ -81,8 +81,9 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 				unsigned int start_addr)
 {
-	unsigned int stat, rdy, mask, block = 0;
+	unsigned int stat, rdy, mask, timeout, block = 0;
 
+	timeout = 10000;
 	rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
 	mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
 	do {

@@ -108,6 +109,12 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}
 #endif
+		if (timeout-- > 0)
+			udelay(10);
+		else {
+			printf("Transfer data timeout\n");
+			return -1;
+		}
 	} while (!(stat & SDHCI_INT_DATA_END));
 	return 0;
 }

1.7.0.4

SDHCI spec need to reset the sdma base address while the software try to accorss the 512k bytes address boundary. When meet such accross behavior, sdhci controller would generate a interrupt automatically, and software need handle this.

---
Changelog:
V2: no change
V3: new added fix for sdma bug
V4: no change

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

index 77a9e70..fce0ef0 100644
@@ -104,7 +104,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 #ifdef CONFIG_MMC_SDMA
 		if (stat & SDHCI_INT_DMA_END) {
 			sdhci_writel(host, SDHCI_INT_DMA_END, SDHCI_INT_STATUS);
-			start_addr &= SDHCI_DEFAULT_BOUNDARY_SIZE - 1;
+			start_addr &= ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
 			start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}

1.7.0.4

Only flush the memory range needed.

---
Changelog:
V2: no change
V3: no change

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

index 9ebd33d..4a92453 100644
@@ -196,7 +196,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	sdhci_writel(host, cmd->cmdarg, SDHCI_ARGUMENT);
 #ifdef CONFIG_MMC_SDMA
-	flush_cache(0, ~0);
+	flush_cache(start_addr, trans_bytes);
 #endif
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->cmdidx, flags), SDHCI_COMMAND);
 	do {

1.7.0.4

If CONFIG_MMC_SDHCI_IO_ACCESSORS is defined, the following warning would shows up:

include/sdhci.h:224: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:224: warning: its scope is only this definition or declaration, which is probably not what you want include/sdhci.h:225: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:226: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:227: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:228: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:229: warning: 'struct sdhci_host' declared inside parameter list

---
Changelog:
V2: code style change
V2.1: delete "Change-Id"
V3: no change

 include/sdhci.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 6d52ce9..e84d2dc 100644
@@ -214,6 +214,9 @@
  */
 #define SDHCI_QUIRK_32BIT_DMA_ADDR	(1 << 0)
 
+/* to make gcc happy */
+struct sdhci_host;
+
 /*
  * Host SDMA buffer boundary. Valid values from 4K to 512K in powers of 2.
  */

1.7.0.4

So that sdhci host would tell in the driver that the mmc current attributes.

---
Changelog:
V2: no change
V3: no change

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

index 4a92453..31c738e 100644
@@ -377,6 +377,7 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk)
 	}
 
 	mmc->priv = host;
+	host->mmc = mmc;
 
 	sprintf(mmc->name, "%s", host->name);
 	mmc->send_cmd = sdhci_send_command;

index e84d2dc..e4e7ebe 100644
@@ -27,6 +27,7 @@
 #define __SDHCI_HW_H
 
 #include <asm/io.h>
+#include <mmc.h>
 /*
  * Controller registers
  */

@@ -239,6 +240,7 @@ struct sdhci_host {
 	unsigned int quirks;
 	unsigned int version;
 	unsigned int clock;
+	struct mmc *mmc;
 	const struct sdhci_ops *ops;
 };
 

1.7.0.4

Hello.

[...]

Hi Sergei,

Thanks for pointing that out! I would repost which keep the empty line.

Best regards,
Lei

---
Changelog:
V2: no change
V3: no change

 drivers/mmc/sdhci.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

index 31c738e..77a9e70 100644
@@ -81,8 +81,9 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 				unsigned int start_addr)
 {
-	unsigned int stat, rdy, mask, block = 0;
+	unsigned int stat, rdy, mask, timeout, block = 0;
 
+	timeout = 10000;
 	rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
 	mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
 	do {

@@ -108,6 +109,12 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}
 #endif
+		if (timeout-- > 0)
+			udelay(10);
+		else {
+			printf("Transfer data timeout\n");
+			return -1;
+		}
 	} while (!(stat & SDHCI_INT_DATA_END));
 	return 0;
 }

1.7.0.4

SDHCI spec need to reset the sdma base address while the software try to accorss the 512k bytes address boundary. When meet such accross behavior, sdhci controller would generate a interrupt automatically, and software need handle this.

---
Changelog:
V2: no change
V3: new added fix for sdma bug

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

index 77a9e70..fce0ef0 100644
@@ -104,7 +104,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 #ifdef CONFIG_MMC_SDMA
 		if (stat & SDHCI_INT_DMA_END) {
 			sdhci_writel(host, SDHCI_INT_DMA_END, SDHCI_INT_STATUS);
-			start_addr &= SDHCI_DEFAULT_BOUNDARY_SIZE - 1;
+			start_addr &= ~(SDHCI_DEFAULT_BOUNDARY_SIZE - 1);
 			start_addr += SDHCI_DEFAULT_BOUNDARY_SIZE;
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}

1.7.0.4

Only flush the memory range needed.

---
Changelog:
V2: no change

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

index 9ebd33d..4a92453 100644
@@ -196,7 +196,7 @@ int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 
 	sdhci_writel(host, cmd->cmdarg, SDHCI_ARGUMENT);
 #ifdef CONFIG_MMC_SDMA
-	flush_cache(0, ~0);
+	flush_cache(start_addr, trans_bytes);
 #endif
 	sdhci_writew(host, SDHCI_MAKE_CMD(cmd->cmdidx, flags), SDHCI_COMMAND);
 	do {

1.7.0.4

If CONFIG_MMC_SDHCI_IO_ACCESSORS is defined, the following warning would shows up:

include/sdhci.h:224: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:224: warning: its scope is only this definition or declaration, which is probably not what you want include/sdhci.h:225: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:226: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:227: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:228: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:229: warning: 'struct sdhci_host' declared inside parameter list

---
Changelog:
V2: code style change

 include/sdhci.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 6d52ce9..e84d2dc 100644
@@ -214,6 +214,9 @@
  */
 #define SDHCI_QUIRK_32BIT_DMA_ADDR	(1 << 0)
 
+/* to make gcc happy */
+struct sdhci_host;
+
 /*
  * Host SDMA buffer boundary. Valid values from 4K to 512K in powers of 2.
  */

1.7.0.4

If CONFIG_MMC_SDHCI_IO_ACCESSORS is defined, the following warning would shows up:

include/sdhci.h:224: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:224: warning: its scope is only this definition or declaration, which is probably not what you want include/sdhci.h:225: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:226: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:227: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:228: warning: 'struct sdhci_host' declared inside parameter list
include/sdhci.h:229: warning: 'struct sdhci_host' declared inside parameter list

---
Changelog:
V2: code style change
V2.1: delete "Change-Id"

 include/sdhci.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 6d52ce9..e84d2dc 100644
@@ -214,6 +214,9 @@
  */
 #define SDHCI_QUIRK_32BIT_DMA_ADDR	(1 << 0)
 
+/* to make gcc happy */
+struct sdhci_host;
+
 /*
  * Host SDMA buffer boundary. Valid values from 4K to 512K in powers of 2.
  */

1.7.0.4

So that sdhci host would tell in the driver that the mmc current attributes.

---
Changelog:
V2: no change

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

index 4a92453..31c738e 100644
@@ -377,6 +377,7 @@ int add_sdhci(struct sdhci_host *host, u32 max_clk, u32 min_clk)
 	}
 
 	mmc->priv = host;
+	host->mmc = mmc;
 
 	sprintf(mmc->name, "%s", host->name);
 	mmc->send_cmd = sdhci_send_command;

index 6c6a8ea..2dd36a4 100644
@@ -27,6 +27,7 @@
 #define __SDHCI_HW_H
 
 #include <asm/io.h>
+#include <mmc.h>
 /*
  * Controller registers
  */

@@ -237,6 +238,7 @@ struct sdhci_host {
 	unsigned int quirks;
 	unsigned int version;
 	unsigned int clock;
+	struct mmc *mmc;
 	const struct sdhci_ops *ops;
 };
 

1.7.0.4

---
Changelog:
V2: no change

 drivers/mmc/sdhci.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

index 31c738e..77a9e70 100644
@@ -81,8 +81,9 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data)
 static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 				unsigned int start_addr)
 {
-	unsigned int stat, rdy, mask, block = 0;
+	unsigned int stat, rdy, mask, timeout, block = 0;
 
+	timeout = 10000;
 	rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
 	mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
 	do {

@@ -108,6 +109,12 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 			sdhci_writel(host, start_addr, SDHCI_DMA_ADDRESS);
 		}
 #endif
+		if (timeout-- > 0)
+			udelay(10);
+		else {
+			printf("Transfer data timeout\n");
+			return -1;
+		}
 	} while (!(stat & SDHCI_INT_DATA_END));
 	return 0;
 }

1.7.0.4