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

login
register
mail settings
Submitter Andrew Gabbasov
Date June 10, 2013, 2:51 p.m.
Message ID <6C5EA58090A5ED459815C4D04C2B466FD933294A@EU-MBX-01.mgc.mentorg.com>
Download mbox | patch
Permalink /patch/250258/
State Not Applicable, archived
Delegated to: Andy Fleming
Headers show

Comments

Andrew Gabbasov - June 10, 2013, 2:51 p.m.
Hi Dirk,
Dirk Behme - June 11, 2013, 8:16 a.m.
On 10.06.2013 16:51, Gabbasov, Andrew wrote:
> Hi Dirk,
> ________________________________________
>> From: Behme, Dirk - Bosch
>> Sent: Monday, June 10, 2013 16:06
>> To: Gabbasov, Andrew
>> Cc: u-boot@lists.denx.de; Stefano Babic; Fleming Andy-AFLEMING
>> Subject: Re: [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
>>
>> 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
>
> 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.orig	2013-05-30 03:48:26.000000000 -0500
> +++ fsl_esdhc.c	2013-06-10 09:38:30.071905119 -0500
> @@ -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
Andrew Gabbasov - June 11, 2013, 3:24 p.m.
Hi Dirk,

Patch

diff -u fsl_esdhc.c.orig fsl_esdhc.c
--- fsl_esdhc.c.orig	2013-05-30 03:48:26.000000000 -0500
+++ fsl_esdhc.c	2013-06-10 09:38:30.071905119 -0500
@@ -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)) {