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

[Eric Nelson]

On 04/03/2013 04:17 PM, Eric Nelson wrote:

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.

I just put a test in to see if PRSSTAT_DLA clears before IRQSTAT_TC
and was able to see it, but only with cache enabled (when presumably
the loop runs faster).

I pulled it out of the while loop test so I've seen at one
read with both TC and DINT bits clear in irqstat after a
read of prsstat with DLA high.

IOW, we are sometimes short-circuiting the loop based on DLA
clear, which seems faulty.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot