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

[Eric Nelson]

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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot