On Mon, 1 Oct 2018 19:59:11 +0000 Chris Packham <Chris.Packham@xxxxxxxxxxxxxxxxxxx> wrote: > On 01/10/18 18:31, Daniel Mack wrote: > > On 30/9/2018 11:10 PM, Chris Packham wrote: > >>>> With this in mind, I don't see why this > >>>> > >>>> + st = readl_relaxed(nfc->regs + NDSR); > >>>> + if (st & (NDSR_RDY(0) | NDSR_RDY(1))) > >>>> + complete(&nfc->complete); > >>> Yeah, me neither. Chris, are you absolutely sure this is the reason? I'm > >>> asking because it took me several tries sometimes to trigger the bug, so > >>> is there a chance that you see an error at all times, regardless of > >>> whether my patch is applied? > >> It seems pretty consistent. Without this patch there seems to be no > >> problem. With this patch it triggers pretty much straight away. I can't > >> discount that there might be something wrong with my dts (the R/B > >> configuration was missing initially). > >> > >> I've also been able to run this on the DB-88F6820-AMC board with the > >> same result (the dts for this is in the for-next branch of > >> git://git.infradead.org/linux-mvebu.git). > >> > >> The really odd thing is the following seems to avoid the problem > >> > >> + st = readl_relaxed(nfc->regs + NDSR); > >> + udelay(1000); > >> + if (st & (NDSR_RDY(0) | NDSR_RDY(1))) > >> + complete(&nfc->complete); > >> > >> Which is weird because the st value has already been read so the udelay > >> should have no effect. > > > > Erm, yes. That's totally weird. Which gcc are you using for this? > > arm-softfloat-linux-gnueabi-gcc (crosstool-NG crosstool-ng-1.22.0) 4.9.3 > > > Could you please try and use readl() here instead of readl_relaxed()? > > That will place a memory barrier after the read to enforce ordering. > > I'd previously tried readl() based on the same hunch. No change. > > I think my snippet above might be misleading. While a delay between > readl_relaxed() and the if should not change the outcome, this is also a > delay between marvell_nfc_enable_int() and marvell_nfc_disable_int() > which is probably more significant. Sure enough if I move the delay to > just before the marvell_nfc_disable_int() the error is not seen. AFAICT, your timeout always happens when waiting for RDREQ, not RDYM. So maybe disabling MRDY too early has a side-effect on the RDREQ event.
