diff mbox

[U-Boot] mx6: fsl_esdhc: Fix waiting for DMA operation completion

Message ID 1364897095-28227-1-git-send-email-andrew_gabbasov@mentor.com
State Superseded
Delegated to: Stefano Babic
Headers show

Commit Message

Gabbasov, Andrew April 2, 2013, 10:04 a.m. UTC
On iMX6 sometimes the Transfer Complete interrupt occurs earlier
than the DMA part completes its operation. If immediately after that
the read data is used for some data verification, those obtained data
may be incomplete, which causes intermittent verification failures.

For example, when the default environment command tries to load and run
boot script from FAT partition on SD/MMC card, it sometimes fails,
reporting invalid partition table, or unknown partition type, or
something else of that kind. Such errors disappear if the build
configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
is added after transfer completion.

Adding extra waiting for DMA completion after Transfer Complete
event fixes this issue.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mmc/fsl_esdhc.c |    6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Nelson April 2, 2013, 3:49 p.m. UTC | #1
Thanks Andrew,

On 04/02/2013 03:04 AM, Andrew Gabbasov wrote:
> On iMX6 sometimes the Transfer Complete interrupt occurs earlier
> than the DMA part completes its operation. If immediately after that
> the read data is used for some data verification, those obtained data
> may be incomplete, which causes intermittent verification failures.
>

Can you describe how to repeat this?

> For example, when the default environment command tries to load and run
> boot script from FAT partition on SD/MMC card, it sometimes fails,
> reporting invalid partition table, or unknown partition type, or
> something else of that kind. Such errors disappear if the build
> configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
> is added after transfer completion.
>
We do this on every boot on SABRE Lite and Nitrogen6x boards,
and haven't seen an issue.

What board are you testing on?

Do you have cache enabled?

Is this with an SD card or eMMC?

> Adding extra waiting for DMA completion after Transfer Complete
> event fixes this issue.
>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> ---
>   drivers/mmc/fsl_esdhc.c |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index d2a505e..806c6dd 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -402,6 +402,12 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>   				return COMM_ERR;
>   		} while (!(irqstat & IRQSTAT_TC) &&
>   				(esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
> +#ifdef CONFIG_MX6
> +		/* In imx6 TC (data end) interrupt sometimes occur earlier
> +		   than DMA completes. In this case just wait a little more. */
> +		while (!(irqstat & (IRQSTAT_DINT | IRQSTAT_DMAE)))
> +			irqstat = esdhc_read32(&regs->irqstat);
> +#endif
>   #endif
>   	}
>
>
Dirk Behme April 2, 2013, 6:10 p.m. UTC | #2
Am 02.04.2013 17:49, schrieb Eric Nelson:
> Thanks Andrew,
>
> On 04/02/2013 03:04 AM, Andrew Gabbasov wrote:
>> On iMX6 sometimes the Transfer Complete interrupt occurs earlier
>> than the DMA part completes its operation. If immediately after that
>> the read data is used for some data verification, those obtained data
>> may be incomplete, which causes intermittent verification failures.
>>
>
> Can you describe how to repeat this?
>
>> For example, when the default environment command tries to load and run
>> boot script from FAT partition on SD/MMC card, it sometimes fails,
>> reporting invalid partition table, or unknown partition type, or
>> something else of that kind. Such errors disappear if the build
>> configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
>> is added after transfer completion.
>>
> We do this on every boot on SABRE Lite and Nitrogen6x boards,
> and haven't seen an issue.
>
> What board are you testing on?
>
> Do you have cache enabled?
>
> Is this with an SD card or eMMC?

Andrew will have the details, but to my understanding this implements 
in U-Boot what the Freescale kernel has in

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/mmc/host/sdhci-esdhc-imx.c?h=imx_3.0.35_12.09.01#n234

for TO 1.0.

Best regards

Dirk

>> Adding extra waiting for DMA completion after Transfer Complete
>> event fixes this issue.
>>
>> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
>> ---
>>   drivers/mmc/fsl_esdhc.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>> index d2a505e..806c6dd 100644
>> --- a/drivers/mmc/fsl_esdhc.c
>> +++ b/drivers/mmc/fsl_esdhc.c
>> @@ -402,6 +402,12 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd
>> *cmd, struct mmc_data *data)
>>                   return COMM_ERR;
>>           } while (!(irqstat & IRQSTAT_TC) &&
>>                   (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
>> +#ifdef CONFIG_MX6
>> +        /* In imx6 TC (data end) interrupt sometimes occur earlier
>> +           than DMA completes. In this case just wait a little
>> more. */
>> +        while (!(irqstat & (IRQSTAT_DINT | IRQSTAT_DMAE)))
>> +            irqstat = esdhc_read32(&regs->irqstat);
>> +#endif
>>   #endif
>>       }
>>
>>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Gabbasov, Andrew April 2, 2013, 6:21 p.m. UTC | #3
> From: Eric Nelson [eric.nelson@boundarydevices.com]
> Sent: Tuesday, April 02, 2013 19:49
> To: Gabbasov, Andrew
> Cc: u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
> 
> Thanks Andrew,
> 
> On 04/02/2013 03:04 AM, Andrew Gabbasov wrote:
> > On iMX6 sometimes the Transfer Complete interrupt occurs earlier
> > than the DMA part completes its operation. If immediately after that
> > the read data is used for some data verification, those obtained data
> > may be incomplete, which causes intermittent verification failures.
> >
> 
> Can you describe how to repeat this?
> 
> > For example, when the default environment command tries to load and run
> > boot script from FAT partition on SD/MMC card, it sometimes fails,
> > reporting invalid partition table, or unknown partition type, or
> > something else of that kind. Such errors disappear if the build
> > configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
> > is added after transfer completion.
> >
> We do this on every boot on SABRE Lite and Nitrogen6x boards,
> and haven't seen an issue.
> 
> What board are you testing on?
> 
> Do you have cache enabled?
> 
> Is this with an SD card or eMMC?

Hi Eric,

Thank you for the response.

I'm using SabreLite with SD card.

For reproducing a problem I took the latest version of U-Boot from master branch and made
a couple of changes in include/configs/mx6qsabrelite.h:
- removed duplication of "mmc dev ${mmcdev};" in CONFIG_BOOTCOMMAND;
- and changed CONFIG_ENV_IS_IN_MMC to CONFIG_ENV_IS_NOWHERE
(thus disabling to try reading saved environment from mmc).
The boot.scr contains a single command "printenv". When I'm doing resets with this
configuration, I'm getting errors in MMC access approximately once per 4-5 boots.
The errors can be "Invalid partition 2" or "No partition table".

Indeed, I was unable to reproduce it with plain master version, but 2 simple changes in configuration, described above, allowed me to reproduce it.

Thanks.

Best regards,
Andrew

> 
> > Adding extra waiting for DMA completion after Transfer Complete
> > event fixes this issue.
> >
> > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > ---
> >   drivers/mmc/fsl_esdhc.c |    6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> > index d2a505e..806c6dd 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -402,6 +402,12 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
> >                               return COMM_ERR;
> >               } while (!(irqstat & IRQSTAT_TC) &&
> >                               (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
> > +#ifdef CONFIG_MX6
> > +             /* In imx6 TC (data end) interrupt sometimes occur earlier
> > +                than DMA completes. In this case just wait a little more. */
> > +             while (!(irqstat & (IRQSTAT_DINT | IRQSTAT_DMAE)))
> > +                     irqstat = esdhc_read32(&regs->irqstat);
> > +#endif
> >   #endif
> >       }
> >
> >
Eric Nelson April 2, 2013, 9:50 p.m. UTC | #4
Thanks Dirk,

On 04/02/2013 11:10 AM, Dirk Behme wrote:
> Am 02.04.2013 17:49, schrieb Eric Nelson:
>> Thanks Andrew,
>>
>> On 04/02/2013 03:04 AM, Andrew Gabbasov wrote:
>>> On iMX6 sometimes the Transfer Complete interrupt occurs earlier
>>> than the DMA part completes its operation. If immediately after that
>>> the read data is used for some data verification, those obtained data
>>> may be incomplete, which causes intermittent verification failures.
>>>
>>
>> Can you describe how to repeat this?
>>
>>> For example, when the default environment command tries to load and run
>>> boot script from FAT partition on SD/MMC card, it sometimes fails,
>>> reporting invalid partition table, or unknown partition type, or
>>> something else of that kind. Such errors disappear if the build
>>> configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
>>> is added after transfer completion.
>>>
>> We do this on every boot on SABRE Lite and Nitrogen6x boards,
>> and haven't seen an issue.
>>
>> What board are you testing on?
>>
>> Do you have cache enabled?
>>
>> Is this with an SD card or eMMC?
>
> Andrew will have the details, but to my understanding this implements in
> U-Boot what the Freescale kernel has in
>
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/mmc/host/sdhci-esdhc-imx.c?h=imx_3.0.35_12.09.01#n234
>

Andrew, did you trap one of these?
Gabbasov, Andrew April 3, 2013, 7:33 a.m. UTC | #5
> From: Eric Nelson [eric.nelson@boundarydevices.com]
> Sent: Wednesday, April 03, 2013 01:50
> To: Dirk Behme
> Cc: Gabbasov, Andrew; u-boot@lists.denx.de
> Subject: Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
> 
> Thanks Dirk,
> 
> On 04/02/2013 11:10 AM, Dirk Behme wrote:
> > Am 02.04.2013 17:49, schrieb Eric Nelson:
> >> Thanks Andrew,
> >>
> >> On 04/02/2013 03:04 AM, Andrew Gabbasov wrote:
> >>> On iMX6 sometimes the Transfer Complete interrupt occurs earlier
> >>> than the DMA part completes its operation. If immediately after that
> >>> the read data is used for some data verification, those obtained data
> >>> may be incomplete, which causes intermittent verification failures.
> >>>
> >>
> >> Can you describe how to repeat this?
> >>
> >>> For example, when the default environment command tries to load and run
> >>> boot script from FAT partition on SD/MMC card, it sometimes fails,
> >>> reporting invalid partition table, or unknown partition type, or
> >>> something else of that kind. Such errors disappear if the build
> >>> configuration has CONFIG_SYS_FSL_ESDHC_USE_PIO, or if some delay
> >>> is added after transfer completion.
> >>>
> >> We do this on every boot on SABRE Lite and Nitrogen6x boards,
> >> and haven't seen an issue.
> >>
> >> What board are you testing on?
> >>
> >> Do you have cache enabled?
> >>
> >> Is this with an SD card or eMMC?
> >
> > Andrew will have the details, but to my understanding this implements in
> > U-Boot what the Freescale kernel has in
> >
> > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/mmc/host/sdhci-esdhc-imx.c?h=imx_3.0.35_12.09.01#n234
> >
> 
> Andrew, did you trap one of these?

Hi Eric,

Yes, I saw that part of code in the kernel and that led me to check if the events can come out of order in my case, and after finding that they indeed do, to make that patch. So, this is the similar workaround but on U-Boot level.

Thanks.

Best regards,
Andrew
diff mbox

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index d2a505e..806c6dd 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -402,6 +402,12 @@  esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 				return COMM_ERR;
 		} while (!(irqstat & IRQSTAT_TC) &&
 				(esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
+#ifdef CONFIG_MX6
+		/* In imx6 TC (data end) interrupt sometimes occur earlier
+		   than DMA completes. In this case just wait a little more. */ 
+		while (!(irqstat & (IRQSTAT_DINT | IRQSTAT_DMAE)))
+			irqstat = esdhc_read32(&regs->irqstat);
+#endif
 #endif
 	}