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

[Eric Nelson <eric.nelson@boundarydevices.com>]

Hi Andrew,

On 04/03/2013 10:30 AM, Gabbasov, Andrew wrote:
>>
>>> 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.
>>>
>> I agree wholeheartedly.
>>
>> I do wonder if the previous loop should be re-worked though.
>> It seems that we should be waiting for TC & (DINT|DMAE) on
>> all processor variants and the previous loop has tests for
>> timeout and data errors.
>>
>> Regards,
>>
>> Eric
>
> Hi Eric,
>
> Do you mean making it something like
>
>     do {
>        irqstat = esdhc_read32(&regs->irqstat);
>
>        if (irqstat & IRQSTAT_DTOE)
>           return TIMEOUT;
>
>        if (irqstat & (DATA_ERR | IRQSTAT_DMAE))
>           return COMM_ERR;
>
>     } while (!((irqstat & IRQSTAT_TC) && (irqstat & IRQSTAT_DINT)) &&
>           (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
>

Yes. That's what I was thinking.

> The check for DMAE (DMA Error) can be combined with other data errors
> and cause exit from the loop. And DINT can be checked with TC in the loop
> condition.
>
Makes sense.

> Actually, I'm a little confused by PRSSTAT_DLA checking: currently the loop exits
> when either IRQSTAT_TC occurs _or_ PRSSTAT_DLA flag comes to 0. Is that correct?
> I'm not quite familiar with using this flag, but should the loop exit when both
> IRQSTAT_TC occurs _and_ PRSSTAT_DLA flag comes to 0 (i.e. in current code '&&'
> should be replaced by '||')? And then the modified loop condition (with DMA check)
> would be
>
>     } while (!(irqstat & IRQSTAT_TC) || !(irqstat & IRQSTAT_DINT) ||
>           (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
>
> Can you advise anything on using this flag?
>

That is weird, and suspect. The reference manual indicates that this
bit (Data line active) will go low when the data lines are done with
the transaction, but that will happen before the DMA completes, so
it seems like a bad way to short-circuit the loop.

Fabio, can you comment on this?

The code appears to have been like this since the beginning
of main-line support for i.MX so there's no history to go on.

The same goes in Freescale's git tree.

Regards,


Eric