> From: Fleming Andy-AFLEMING [aflem...@freescale.com]
> Sent: Saturday, April 06, 2013 00:18
> To: Eric Nelson
> Cc: Gabbasov, Andrew; u-boot@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.nel...@boundarydevices.com]
> >>> Sent: Thursday, April 04, 2013 03:47
> >>> To: Gabbasov, Andrew
> >>> Cc: u-boot@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(®s->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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
