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

[Gabbasov, Andrew]

> From: Fleming Andy-AFLEMING [afleming at freescale.com]
> Sent: Saturday, April 06, 2013 00:18
> To: Eric Nelson
> Cc: Gabbasov, Andrew; u-boot at lists.denx.de; Behme, Dirk - Bosch; Estevam Fabio-R49496
> Subject: Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
> 
> On Apr 4, 2013, at 1:41 PM, Eric Nelson wrote:
> 
> > Hi Andrew,
> >
> > On 04/04/2013 11:03 AM, Gabbasov, Andrew wrote:
> >> Hi Eric,
> >>
> >>> From: Eric Nelson [eric.nelson at boundarydevices.com]
> >>> Sent: Thursday, April 04, 2013 03:47
> >>> To: Gabbasov, Andrew
> >>> Cc: u-boot at lists.denx.de; Behme, Dirk - Bosch; Fabio Estevam
> >>> Subject: Re: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion
> >>>
> >>
> >> So, do you think the latter (modified) loop condition
> >>
> >>      } while (!(irqstat & IRQSTAT_TC) || !(irqstat & IRQSTAT_DINT) ||
> >>            (esdhc_read32(&regs->prsstat) & PRSSTAT_DLA));
> >>
> >> will be correct?
> >>
> >
> > I think the right thing to do is eliminate the DLA test entirely,
> > so the loop condition can be simplified to something like this:
> >
> > #define TRANSFER_COMPLETE (IRQSTAT_TC|IRQSTAT_DINT)
> >
> >       do {
> >               ...
> >       } while (TRANSFER_COMPLETE != (irqstat&TRANSFER_COMPLETE));
> 
> 
> That looks right to me. I have been known to mistakenly write loops that are supposed to wait for two conditions to only wait for one of those. Apparently I need remedial boolean logic lessons.
> 
> 
> >
> > If there is another part that needs to bail out on PRSSTAT_DLA,
> > it seems that the affected part should be the one with the #ifdef
> 
> 
> I don't think we need a special case. Just correct logic. :/
> 
> Andy

Hi Andy, Eric,

Thanks for the review!

I've submitted a new patch ("[U-Boot] [Patch] fsl_esdhc: Fix DMA transfer completion waiting loop"),
that reworks this loop condition. I've changed the patch subject to better reflect the fix contents,
and since it no longer relates mx6 only. So, it becomes not the V2 of this patch, but a new one.

Please, let me know if there are any problems with the new patch.

Thanks.

Best regards,
Andrew