fsl_esdhc: Fix DMA transfer completion waiting loop

Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop.

Signed-off-by Andrew Gabbasov <[email protected]>

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

drivers/mmc/fsl_esdhc.c +1 -2

index 54b5363..814bba4 100644
@@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
  			if (irqstat & DATA_ERR)
  				return COMM_ERR;
-		} while (!(irqstat & IRQSTAT_TC) &&
-				(esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
  #endif
  	}
 

include/fsl_esdhc.h +0 -0

index 47d2fe4..ea0880b 100644

Am 08.04.2013 11:06, schrieb Andrew Gabbasov:

Quoted 33 lines

Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop.

Signed-off-by Andrew Gabbasov <[email protected]>

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

drivers/mmc/fsl_esdhc.c +1 -2

index 54b5363..814bba4 100644
@@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd
 *cmd, struct mmc_data *data)
 
              if (irqstat & DATA_ERR)
                  return COMM_ERR;
-        } while (!(irqstat & IRQSTAT_TC) &&
-                (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+        } while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
  #endif
      }

include/fsl_esdhc.h +0 -0

index 47d2fe4..ea0880b 100644

include/fsl_esdhc.h line 71

Quoted 10 lines

include/fsl_esdhc.h +3 -1

@@ -63,7 +63,9 @@
  #define IRQSTAT_CC        (0x00000001)
 
  #define CMD_ERR        (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE)
-#define DATA_ERR    (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE)
+#define DATA_ERR    (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \
+                IRQSTAT_DMAE)
+#define DATA_COMPLETE    (IRQSTAT_TC | IRQSTAT_DINT)
 
  #define IRQSTATEN        0x0002e034
  #define IRQSTATEN_DMAE        (0x10000000)

I can't say anything to the content of the patches ;)

But are this patch and the patch from Eric

http://patchwork.ozlabs.org/patch/233595/

fine and should be applied together, now?

Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop.

Signed-off-by Andrew Gabbasov <[email protected]>

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

drivers/mmc/fsl_esdhc.c +1 -2

index 54b5363..814bba4 100644
@@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
  			if (irqstat & DATA_ERR)
  				return COMM_ERR;
-		} while (!(irqstat & IRQSTAT_TC) &&
-				(esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+		} while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
  #endif
  	}
 

include/fsl_esdhc.h +0 -0

index 47d2fe4..ea0880b 100644

From: Behme, Dirk - Bosch
Sent: Monday, June 10, 2013 16:06
To: Gabbasov, Andrew
Cc: u-boot at lists.denx.de; Stefano Babic; Fleming Andy-AFLEMING
Subject: Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop

Quoted 32 lines · Andrew Gabbasov

Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop.

Signed-off-by Andrew Gabbasov <[email protected]>

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

drivers/mmc/fsl_esdhc.c +1 -2

index 54b5363..814bba4 100644
@@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
                      if (irqstat & DATA_ERR)
                              return COMM_ERR;
-             } while (!(irqstat & IRQSTAT_TC) &&
-                             (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+             } while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
  #endif
      }
 

include/fsl_esdhc.h +0 -0

index 47d2fe4..ea0880b 100644

include/fsl_esdhc.h line 71

Quoted 10 lines

include/fsl_esdhc.h +3 -1

@@ -63,7 +63,9 @@
  #define IRQSTAT_CC          (0x00000001)
 
  #define CMD_ERR             (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE)
-#define DATA_ERR     (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE)
+#define DATA_ERR     (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \
+                             IRQSTAT_DMAE)
+#define DATA_COMPLETE        (IRQSTAT_TC | IRQSTAT_DINT)
 
  #define IRQSTATEN           0x0002e034
  #define IRQSTATEN_DMAE              (0x10000000)

I haven't tested this myself, but I got the following issue report regarding this patch:

Using a SANDISK ULTRA II 8GB card (or alternatively Transcend 16GB or 32GB cards) and trying an mmc write [1] into the upper area of the 8GB card makes the write hang in 9 of 10 cases. Sometimes even more. Reverting this patch make these writes work again.

mmc read does work fine, though. Even newer SANDISK Extreme III or several micro SD cards are working fine.

Any idea?

Best regards

Dirk

Hi Dirk,
________________________________________

Quoted 58 lines

From: Behme, Dirk - Bosch
Sent: Monday, June 10, 2013 16:06
To: Gabbasov, Andrew
Cc: u-boot at lists.denx.de; Stefano Babic; Fleming Andy-AFLEMING
Subject: Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop

Quoted 32 lines · Andrew Gabbasov

Rework the waiting for transfer completion loop condition to continue waiting until both Transfer Complete and DMA End interrupts occur. Checking of DLA bit in Present State register looks not needed in addition to interrupts status checking, so it can be removed from the condition. Also, DMA Error condition is added to the list of data errors, checked in the loop.

Signed-off-by Andrew Gabbasov <[email protected]>

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

drivers/mmc/fsl_esdhc.c +1 -2

index 54b5363..814bba4 100644
@@ -400,8 +400,7 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 
                       if (irqstat & DATA_ERR)
                               return COMM_ERR;
-             } while (!(irqstat & IRQSTAT_TC) &&
-                             (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+             } while ((irqstat & DATA_COMPLETE) != DATA_COMPLETE);
   #endif
       }
 

include/fsl_esdhc.h +0 -0

index 47d2fe4..ea0880b 100644

include/fsl_esdhc.h line 71

Quoted 10 lines

include/fsl_esdhc.h +3 -1

@@ -63,7 +63,9 @@
   #define IRQSTAT_CC          (0x00000001)
 
   #define CMD_ERR             (IRQSTAT_CIE | IRQSTAT_CEBE | IRQSTAT_CCE)
-#define DATA_ERR     (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE)
+#define DATA_ERR     (IRQSTAT_DEBE | IRQSTAT_DCE | IRQSTAT_DTOE | \
+                             IRQSTAT_DMAE)
+#define DATA_COMPLETE        (IRQSTAT_TC | IRQSTAT_DINT)
 
   #define IRQSTATEN           0x0002e034
   #define IRQSTATEN_DMAE              (0x10000000)

I haven't tested this myself, but I got the following issue report regarding this patch:

Using a SANDISK ULTRA II 8GB card (or alternatively Transcend 16GB or 32GB cards) and trying an mmc write [1] into the upper area of the 8GB card makes the write hang in 9 of 10 cases. Sometimes even more. Reverting this patch make these writes work again.

Quoted 8 lines

mmc read does work fine, though. Even newer SANDISK Extreme III or several micro SD cards are working fine.

Any idea?

Best regards

Dirk

So far the only idea that comes into my mind is that DMA for some reason completes its part of work too early so that the corresponding interrupt status bit appears and has already been cleared even before entering this loop.

I will be trying to reproduce the issue.

Meanwhile, is it possible to ask the reporter (who obviously can reproduce it) to try to add the debug print from the diff below and show what it prints when the write command hangs and when it succeeds?

Thanks.

Best regards,
Andrew

diff -u fsl_esdhc.c.orig fsl_esdhc.c

From: Behme, Dirk - Bosch
Sent: Tuesday, June 11, 2013 12:16
To: Gabbasov, Andrew
Cc: u-boot at lists.denx.de; Stefano Babic; Fleming Andy-AFLEMING
Subject: Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop

Quoted 1 line · Andrew

Hi Dirk,

Quoted 37 lines

Quoted 9 lines

I haven't tested this myself, but I got the following issue report regarding this patch:

Using a SANDISK ULTRA II 8GB card (or alternatively Transcend 16GB or 32GB cards) and trying an mmc write [1] into the upper area of the 8GB card makes the write hang in 9 of 10 cases. Sometimes even more. Reverting this patch make these writes work again.

Quoted 8 lines

mmc read does work fine, though. Even newer SANDISK Extreme III or several micro SD cards are working fine.

Any idea?

Best regards

Dirk

So far the only idea that comes into my mind is that DMA for some reason completes its part of work too early so that the corresponding interrupt status bit appears and has already been cleared even before entering this loop.

I will be trying to reproduce the issue.

Meanwhile, is it possible to ask the reporter (who obviously can reproduce it) to try to add the debug print from the diff below and show what it prints when the write command hangs and when it succeeds?

Thanks.

Best regards,
Andrew

diff -u fsl_esdhc.c.orig fsl_esdhc.c

fsl_esdhc.c 2013-06-10 09:38:30.071905119 -0500 line 335

Quoted 7 lines

fsl_esdhc.c 2013-06-10 09:38:30.071905119 -0500 renamed from fsl_esdhc.c.orig 2013-05-30 03:48:26.000000000 -0500 +1 -0

@@ -329,6 +329,7 @@
 
      irqstat = esdhc_read32(&regs->irqstat);
      esdhc_write32(&regs->irqstat, irqstat);
+     printf("fsl_esdhc: irqstat = 0x%08x\n", irqstat);
 
      /* Reset CMD and DATA portions on error */
      if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {

1. Sandisk 8GB Ultra 2 class 4 SDHC

[   23.967081] MMC write: dev # 0, block # 7400000, count 1 ...
fsl_esdhc: irqstat = 0x00000001
[   23.977473] fsl_esdhc: irqstat = 0x00000009
  => hang

With "fsl_esdhc: Fix DMA transfer completion waiting loop" reverted:

[   41.769231] MMC write: dev # 0, block # 7400000, count 1 ...
fsl_esdhc: irqstat = 0x00000001
[   41.779622] fsl_esdhc: irqstat = 0x00000009
[   41.798490] fsl_esdhc: irqstat = 0x00000001
[   41.802593] 1 blocks write: OK

=> work

2. Sandisk 16GB Extreme class 10 (30MB/s)

[   45.871140] MMC write: dev # 0, block # 7400000, count 1 ...
fsl_esdhc: irqstat = 0x00000001
[   45.881528] fsl_esdhc: irqstat = 0x00000001
[   46.054409] fsl_esdhc: irqstat = 0x00000001
[   46.058513] 1 blocks write: OK

=> work

With "fsl_esdhc: Fix DMA transfer completion waiting loop" reverted:

[   17.901514] MMC write: dev # 0, block # 7400000, count 1 ...
fsl_esdhc: irqstat = 0x00000001
[   17.911901] fsl_esdhc: irqstat = 0x00000001
[   18.081153] fsl_esdhc: irqstat = 0x00000001
[   18.085256] 1 blocks write: OK

=> work

Best regards

Dirk

After waiting for the command completion event, the interrupt status bits, that occured to be set by that time, are cleared by writing them back. It is supposed, that it should be command related bits (command complete and may be command errors).

However, in some cases the DMA already completes by that time before the full transaction completes. The corresponding DINT bit gets set and then cleared before even entering the loop, waiting for data part completion. That waiting loop never gets this bit set, causing the operation to hang. This is reported to happen, for example, for write operation of 1 sector to upper area (block #7400000) of SanDisk Ultra II 8GB card.

The solution could be to explicitly clear only command related interrupt status bits. However, since subsequent processing does not rely on any command bits state, it could be easier just to remove clearing of any bits at that point, leaving them all until all data processing completes. After that the whole register will be cleared at once.

Also, on occasion, interrupts masking moved to before writing the command, just for the case there should be no chance of interrupt between the first command and interrupts masking.

Reported-by Dirk Behme <[email protected]>

Signed-off-by Andrew Gabbasov <[email protected]>

---
  drivers/mmc/fsl_esdhc.c |    7 +++----
  1 file changed, 3 insertions(+), 4 deletions(-)

drivers/mmc/fsl_esdhc.c +3 -4

index 861f4b9..b501b4d 100644
@@ -310,6 +310,9 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
  	/* Figure out the transfer arguments */
  	xfertyp = esdhc_xfertyp(cmd, data);
 
+	/* Mask all irqs */
+	esdhc_write32(&regs->irqsigen, 0);
+
  	/* Send the command */
  	esdhc_write32(&regs->cmdarg, cmd->cmdarg);
  #if defined(CONFIG_FSL_USDHC)

@@ -320,15 +323,11 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
  	esdhc_write32(&regs->xfertyp, xfertyp);
  #endif
 
-	/* Mask all irqs */
-	esdhc_write32(&regs->irqsigen, 0);
-
  	/* Wait for the command to complete */
  	while (!(esdhc_read32(&regs->irqstat) & (IRQSTAT_CC | IRQSTAT_CTOE)))
  		;
 
  	irqstat = esdhc_read32(&regs->irqstat);
-	esdhc_write32(&regs->irqstat, irqstat);
 
  	/* Reset CMD and DATA portions on error */
  	if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {

After waiting for the command completion event, the interrupt status bits, that occured to be set by that time, are cleared by writing them back. It is supposed, that it should be command related bits (command complete and may be command errors).

However, in some cases the DMA already completes by that time before the full transaction completes. The corresponding DINT bit gets set and then cleared before even entering the loop, waiting for data part completion. That waiting loop never gets this bit set, causing the operation to hang. This is reported to happen, for example, for write operation of 1 sector to upper area (block #7400000) of SanDisk Ultra II 8GB card.

The solution could be to explicitly clear only command related interrupt status bits. However, since subsequent processing does not rely on any command bits state, it could be easier just to remove clearing of any bits at that point, leaving them all until all data processing completes. After that the whole register will be cleared at once.

Also, on occasion, interrupts masking moved to before writing the command, just for the case there should be no chance of interrupt between the first command and interrupts masking.

Reported-by Dirk Behme <[email protected]>

Signed-off-by Andrew Gabbasov <[email protected]>

Acked-by Dirk Behme <[email protected]>