* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
@ 2013-04-08 9:06 Andrew Gabbasov
2013-04-14 7:19 ` Dirk Behme
2013-06-10 12:06 ` Dirk Behme
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Gabbasov @ 2013-04-08 9:06 UTC (permalink / raw)
To: u-boot
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(®s->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)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-04-08 9:06 [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop Andrew Gabbasov
@ 2013-04-14 7:19 ` Dirk Behme
2013-04-14 9:25 ` Stefano Babic
2013-06-10 12:06 ` Dirk Behme
1 sibling, 1 reply; 10+ messages in thread
From: Dirk Behme @ 2013-04-14 7:19 UTC (permalink / raw)
To: u-boot
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(®s->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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-04-14 7:19 ` Dirk Behme
@ 2013-04-14 9:25 ` Stefano Babic
0 siblings, 0 replies; 10+ messages in thread
From: Stefano Babic @ 2013-04-14 9:25 UTC (permalink / raw)
To: u-boot
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(®s->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
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-04-08 9:06 [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop Andrew Gabbasov
2013-04-14 7:19 ` Dirk Behme
@ 2013-06-10 12:06 ` Dirk Behme
2013-06-10 14:51 ` Gabbasov, Andrew
1 sibling, 1 reply; 10+ messages in thread
From: Dirk Behme @ 2013-06-10 12:06 UTC (permalink / raw)
To: u-boot
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(®s->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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-06-10 12:06 ` Dirk Behme
@ 2013-06-10 14:51 ` Gabbasov, Andrew
2013-06-11 8:16 ` Dirk Behme
0 siblings, 1 reply; 10+ messages in thread
From: Gabbasov, Andrew @ 2013-06-10 14:51 UTC (permalink / raw)
To: u-boot
Hi Dirk,
________________________________________
> 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
>
> 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(®s->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(®s->irqstat);
esdhc_write32(®s->irqstat, irqstat);
+ printf("fsl_esdhc: irqstat = 0x%08x\n", irqstat);
/* Reset CMD and DATA portions on error */
if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-06-10 14:51 ` Gabbasov, Andrew
@ 2013-06-11 8:16 ` Dirk Behme
2013-06-11 15:24 ` Gabbasov, Andrew
0 siblings, 1 reply; 10+ messages in thread
From: Dirk Behme @ 2013-06-11 8:16 UTC (permalink / raw)
To: u-boot
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 at 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(®s->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(®s->irqstat);
> esdhc_write32(®s->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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop
2013-06-11 8:16 ` Dirk Behme
@ 2013-06-11 15:24 ` Gabbasov, Andrew
2013-06-11 15:34 ` [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed Andrew Gabbasov
0 siblings, 1 reply; 10+ messages in thread
From: Gabbasov, Andrew @ 2013-06-11 15:24 UTC (permalink / raw)
To: u-boot
Hi Dirk,
________________________________________
> 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
>
> On 10.06.2013 16:51, Gabbasov, Andrew wrote:
> > Hi Dirk,
[ skipped ]
> >>
> >> 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(®s->irqstat);
> > esdhc_write32(®s->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
Indeed, the DMA End interrupt bit appears in the register already by the time
of Command Complete event. Then, it gets cleared together with command-related
bits. So, the data completion waiting loop, that starts later, never gets it and waits forever.
One of solutions could be to explicitly clear only the command related bits (command complete
and may be command errors). However, it is easier just not clear the bits at all,
leave them all until the data part processing completes too, and clear the whole
register at once at the very end (which is being done anyway).
I'm submitting the patch removing clearing of interrupt status bits at that point. See Subject line
"fsl_esdhc: Do not clear interrupt status bits until data processed".
Thanks.
Best regards,
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed
2013-06-11 15:24 ` Gabbasov, Andrew
@ 2013-06-11 15:34 ` Andrew Gabbasov
2013-06-12 5:16 ` Dirk Behme
2013-06-14 18:55 ` [U-Boot] " Andy Fleming
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Gabbasov @ 2013-06-11 15:34 UTC (permalink / raw)
To: u-boot
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(-)
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(®s->irqsigen, 0);
+
/* Send the command */
esdhc_write32(®s->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(®s->xfertyp, xfertyp);
#endif
- /* Mask all irqs */
- esdhc_write32(®s->irqsigen, 0);
-
/* Wait for the command to complete */
while (!(esdhc_read32(®s->irqstat) & (IRQSTAT_CC | IRQSTAT_CTOE)))
;
irqstat = esdhc_read32(®s->irqstat);
- esdhc_write32(®s->irqstat, irqstat);
/* Reset CMD and DATA portions on error */
if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed
2013-06-11 15:34 ` [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed Andrew Gabbasov
@ 2013-06-12 5:16 ` Dirk Behme
2013-06-14 18:55 ` [U-Boot] " Andy Fleming
1 sibling, 0 replies; 10+ messages in thread
From: Dirk Behme @ 2013-06-12 5:16 UTC (permalink / raw)
To: u-boot
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(®s->irqsigen, 0);
> +
> /* Send the command */
> esdhc_write32(®s->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(®s->xfertyp, xfertyp);
> #endif
>
> - /* Mask all irqs */
> - esdhc_write32(®s->irqsigen, 0);
> -
> /* Wait for the command to complete */
> while (!(esdhc_read32(®s->irqstat) & (IRQSTAT_CC | IRQSTAT_CTOE)))
> ;
>
> irqstat = esdhc_read32(®s->irqstat);
> - esdhc_write32(®s->irqstat, irqstat);
>
> /* Reset CMD and DATA portions on error */
> if (irqstat & (CMD_ERR | IRQSTAT_CTOE)) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] fsl_esdhc: Do not clear interrupt status bits until data processed
2013-06-11 15:34 ` [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed Andrew Gabbasov
2013-06-12 5:16 ` Dirk Behme
@ 2013-06-14 18:55 ` Andy Fleming
1 sibling, 0 replies; 10+ messages in thread
From: Andy Fleming @ 2013-06-14 18:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-06-14 18:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-08 9:06 [U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop Andrew Gabbasov
2013-04-14 7:19 ` Dirk Behme
2013-04-14 9:25 ` Stefano Babic
2013-06-10 12:06 ` Dirk Behme
2013-06-10 14:51 ` Gabbasov, Andrew
2013-06-11 8:16 ` Dirk Behme
2013-06-11 15:24 ` Gabbasov, Andrew
2013-06-11 15:34 ` [U-Boot] [PATCH] fsl_esdhc: Do not clear interrupt status bits until data processed Andrew Gabbasov
2013-06-12 5:16 ` Dirk Behme
2013-06-14 18:55 ` [U-Boot] " Andy Fleming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox