Patchwork [U-Boot] fsl_esdhc: Fix DMA transfer completion waiting loop

login
register
mail settings
Submitter Andrew Gabbasov
Date April 8, 2013, 9:06 a.m.
Message ID <1365411968-2282-1-git-send-email-andrew_gabbasov@mentor.com>
Download mbox | patch
Permalink /patch/234610/
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show

Comments

Andrew Gabbasov - April 8, 2013, 9:06 a.m.
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 <andrew_gabbasov@mentor.com>
---
 drivers/mmc/fsl_esdhc.c |    3 +--
 include/fsl_esdhc.h     |    4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)
Dirk Behme - April 14, 2013, 7:19 a.m.
Am 08.04.2013 11:06, schrieb 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 <andrew_gabbasov@mentor.com>
> ---
>   drivers/mmc/fsl_esdhc.c |    3 +--
>   include/fsl_esdhc.h     |    4 +++-
>   2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 54b5363..814bba4 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -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
>   	}
>
> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
> index 47d2fe4..ea0880b 100644
> --- a/include/fsl_esdhc.h
> +++ b/include/fsl_esdhc.h
> @@ -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?

Thanks

Dirk
Stefano Babic - April 14, 2013, 9:25 a.m.
On 14/04/2013 09:19, Dirk Behme wrote:
> Am 08.04.2013 11:06, schrieb 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 <andrew_gabbasov@mentor.com>
>> ---
>>   drivers/mmc/fsl_esdhc.c |    3 +--
>>   include/fsl_esdhc.h     |    4 +++-
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>> index 54b5363..814bba4 100644
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -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
>>       }
>>
>> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
>> index 47d2fe4..ea0880b 100644
>> --- a/include/fsl_esdhc.h
>> +++ b/include/fsl_esdhc.h
>> @@ -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?

I see the same. I apply both on u-boot-imx and send an update pull
request to Albert.

Best regards,
Stefano
Dirk Behme - June 10, 2013, 12:06 p.m.
On 08.04.2013 11:06, Andrew Gabbasov wrote:
> 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 <andrew_gabbasov@mentor.com>
> ---
>   drivers/mmc/fsl_esdhc.c |    3 +--
>   include/fsl_esdhc.h     |    4 +++-
>   2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 54b5363..814bba4 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -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
>   	}
>
> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
> index 47d2fe4..ea0880b 100644
> --- a/include/fsl_esdhc.h
> +++ b/include/fsl_esdhc.h
> @@ -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


[1]

mmc write 0x10800000 70ea40 1

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 54b5363..814bba4 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -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
 	}
 
diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
index 47d2fe4..ea0880b 100644
--- a/include/fsl_esdhc.h
+++ b/include/fsl_esdhc.h
@@ -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)