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

[Gabbasov, Andrew]

> From: Eric Nelson [eric.nelson at boundarydevices.com]
> Sent: Wednesday, April 03, 2013 01:38
> To: Gabbasov, Andrew
> Cc: u-boot at lists.denx.de; Behme, Dirk - Bosch
> Subject: Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
> 
> Thanks Andrew,
> 
> On 04/02/2013 11:21 AM, Gabbasov, Andrew wrote:
> >> From: Eric Nelson [eric.nelson at boundarydevices.com]
> >> Sent: Tuesday, April 02, 2013 19:49
> >> To: Gabbasov, Andrew
> >> Cc: u-boot at 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.
> >
> 
> I think you exposed another bug here.
> 
> I was able to to reproduce the symptom (failure reading from
> filesystem) on master by simply setting my bootcmd to this:
>         mmc dev 0
>         && fatload mmc 0 10800000 uImage
>         && fatload mmc 0 12800000 uramdisk.img
>         && bootm 10800000 12800000
> 
> The failures differed somewhat from boot to boot,
> but this was typical:
> 
>         reading uImage
>         4350916 bytes read in 216 ms (19.2 MiB/s)
>         bad MBR sector signature 0x0000
>         ** Invalid partition 1 **
> 
> I've seen weirdness in the past with cache enabled (as is the
> case in the mx6qsabrelite_config), so I re-tested with
> a 'dcache off' in bootcmd:
> 
>         dcache off ;
>         mmc dev 0
>         && fatload mmc 0 10800000 uImage
>         && fatload mmc 0 12800000 uramdisk.img
>         && bootm 10800000 12800000
> 
> This appeared to fix the issue (through 100 boots).
> 
> Looking at the code with fresh eyes, it appears that
> the cache invalidate is in the wrong place (after
> "command complete" but before "transfer complete"
> is checked).
> 
> Dirk, this was on a TO1.2 device, so I don't think
> this was caused by the out-of-order completion
> interrupts.
> 
> Andrew, can you test with this patch to see if
> it also addresses the issue?
> 
> By inspection, it seems needed.
> 
> Regards,
> 
> Eric

Hi Eric,

Yes, this patch seems to fix the issue too.

I think, it would be useful to have both patches. Although invalidating cache
(by adding some delay) indirectly helps with waiting for DMA End event,
it is probably worth having explicit DMA completion waiting patch too.

Thanks.

Best regards,
Andrew