From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Sat, 06 Apr 2013 14:31:17 -0700 Subject: [U-Boot] [PATCH] mx6: fsl_esdhc: Fix waiting for DMA operation completion In-Reply-To: References: <1364897095-28227-1-git-send-email-andrew_gabbasov@mentor.com>, <515AFE1E.801@boundarydevices.com> <6C5EA58090A5ED459815C4D04C2B466FD931DF69@EU-MBX-03.mgc.mentorg.com>, <515B4FDB.9020902@boundarydevices.com> <6C5EA58090A5ED459815C4D04C2B466FD931DFA2@EU-MBX-03.mgc.mentorg.com>, <515C30C4.1030802@boundarydevices.com> <6C5EA58090A5ED459815C4D04C2B466FD931E08D@EU-MBX-03.mgc.mentorg.com> <515CB88C.3030909@boundarydevices.com>, <515CBF97.30701@boundarydevices.com> <6C5EA58090A5ED459815C4D04C2B466FD931E121@EU-MBX-03.mgc.mentorg.com> <515DC945.20306@boundarydevices.com> Message-ID: <51609425.3080702@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Thanks for the review Andy. On 04/05/2013 01:18 PM, Fleming Andy-AFLEMING wrote: > > 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(®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. :/ > Cool. It's always hard to tell when IP like this is used for multiple processors. So many data sheets, so little time... Regards, Eric