Patchwork [U-Boot] fsl_esdhc: Do not clear interrupt status bits until data processed

login
register
mail settings
Submitter Andrew Gabbasov
Date June 11, 2013, 3:34 p.m.
Message ID <1370964862-26947-1-git-send-email-andrew_gabbasov@mentor.com>
Download mbox | patch
Permalink /patch/250566/
State Accepted, archived
Delegated to: Andy Fleming
Headers show

Comments

Andrew Gabbasov - June 11, 2013, 3:34 p.m.
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 <dirk.behme@de.bosch.com>
Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/fsl_esdhc.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
Dirk Behme - June 12, 2013, 5:16 a.m.
On 11.06.2013 17:34, Andrew Gabbasov wrote:
> 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 <dirk.behme@de.bosch.com>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>

Acked-by: Dirk Behme <dirk.behme@de.bosch.com>

Thanks

Dirk

> ---
>   drivers/mmc/fsl_esdhc.c |    7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 861f4b9..b501b4d 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -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)) {
Andy Fleming - June 14, 2013, 6:55 p.m.
On Tue, Jun 11, 2013 at 10:34:22AM -0500, Andrew Gabbasov wrote:
> 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 <dirk.behme@de.bosch.com>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Acked-by: Dirk Behme <dirk.behme@de.bosch.com>

Applied, Thanks!

Andy

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 861f4b9..b501b4d 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -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)) {