diff mbox

[U-Boot,v2] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation

Message ID 1320411972.2015.7.camel@linaro1
State Accepted
Commit 0612fcbcb183ff250c018faa6e202a15394094f0
Delegated to: Andy Fleming
Headers show

Commit Message

Jon Medhurst (Tixy) Nov. 4, 2011, 1:06 p.m. UTC 
The new IO FPGA implementation for Versatile Express contains an MMCI
(PL180) cell with the FIFO extended to 128 words. This causes the
read_bytes() function to go into an infinite loop; as it will wait for
for the half-full signal (SDI_STA_RXFIFOBR) if there are more than 8
words remaining (SDI_FIFO_BURST_SIZE), but it won't receive this signal
once there are fewer than 64 words left to transfer.

One possible fix is to add some build time configuration to change
SDI_FIFO_BURST_SIZE for the new implementation. However, the problematic
code only seems to exist as a small performance optimisation, so the
solution implemented by this patch is to simply remove it. The error
checking following the loop is also removed as this will be handled by
code further down the function.

Cc: Andy Fleming <afleming@gmail.com>
Signed-off-by: Jon Medhurst <jon.medhurst@linaro.org>
---

Changes for version 2
- Fix broken whitespace in patch

---
 drivers/mmc/arm_pl180_mmci.c |   26 --------------------------
 1 files changed, 0 insertions(+), 26 deletions(-)

Comments

Andy Fleming Nov. 8, 2011, 8:46 p.m. UTC | #1 
On Fri, Nov 4, 2011 at 8:06 AM, Jon Medhurst (Tixy)
<jon.medhurst@linaro.org> wrote:
> The new IO FPGA implementation for Versatile Express contains an MMCI
> (PL180) cell with the FIFO extended to 128 words. This causes the
> read_bytes() function to go into an infinite loop; as it will wait for
> for the half-full signal (SDI_STA_RXFIFOBR) if there are more than 8
> words remaining (SDI_FIFO_BURST_SIZE), but it won't receive this signal
> once there are fewer than 64 words left to transfer.
>
> One possible fix is to add some build time configuration to change
> SDI_FIFO_BURST_SIZE for the new implementation. However, the problematic
> code only seems to exist as a small performance optimisation, so the
> solution implemented by this patch is to simply remove it. The error
> checking following the loop is also removed as this will be handled by
> code further down the function.
>
> Cc: Andy Fleming <afleming@gmail.com>
> Signed-off-by: Jon Medhurst <jon.medhurst@linaro.org>

Applied, thx
diff mbox

Patch

diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
index ed296ee..e6467a2 100644
--- a/drivers/mmc/arm_pl180_mmci.c
+++ b/drivers/mmc/arm_pl180_mmci.c
@@ -111,7 +111,6 @@  static int do_command(struct mmc *dev, struct mmc_cmd *cmd)
 static int read_bytes(struct mmc *dev, u32 *dest, u32 blkcount, u32 blksize)
 {
 	u32 *tempbuff = dest;
-	int i;
 	u64 xfercount = blkcount * blksize;
 	struct mmc_host *host = dev->priv;
 	u32 status, status_err;
@@ -121,31 +120,6 @@  static int read_bytes(struct mmc *dev, u32 *dest, u32 blkcount, u32 blksize)
 	status = readl(&host->base->status);
 	status_err = status & (SDI_STA_DCRCFAIL | SDI_STA_DTIMEOUT |
 			       SDI_STA_RXOVERR);
-	while (!status_err &&
-	       (xfercount >= SDI_FIFO_BURST_SIZE * sizeof(u32))) {
-		if (status & SDI_STA_RXFIFOBR) {
-			for (i = 0; i < SDI_FIFO_BURST_SIZE; i++)
-				*(tempbuff + i) = readl(&host->base->fifo);
-			tempbuff += SDI_FIFO_BURST_SIZE;
-			xfercount -= SDI_FIFO_BURST_SIZE * sizeof(u32);
-		}
-		status = readl(&host->base->status);
-		status_err = status &
-			(SDI_STA_DCRCFAIL | SDI_STA_DTIMEOUT | SDI_STA_RXOVERR);
-	}
-
-	if (status & SDI_STA_DTIMEOUT) {
-		printf("Read data timed out, xfercount: %llu, status: 0x%08X\n",
-			xfercount, status);
-		return -ETIMEDOUT;
-	} else if (status & SDI_STA_DCRCFAIL) {
-		printf("Read data blk CRC error: 0x%x\n", status);
-		return -EILSEQ;
-	} else if (status & SDI_STA_RXOVERR) {
-		printf("Read data RX overflow error\n");
-		return -EIO;
-	}
-
 	while ((!status_err) && (xfercount >= sizeof(u32))) {
 		if (status & SDI_STA_RXDAVL) {
 			*(tempbuff) = readl(&host->base->fifo);