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?
Could you please try and use readl() here instead of readl_relaxed()?
That will place a memory barrier after the read to enforce ordering.
But if this is a problem, many other parts of that driver should be
equally affected.
On 28/09/18 19:43, Daniel Mack wrote:
>
> Also, does my .EALREADY approach (v1) make any difference?
>
The v1 of this patch doesn't show the problem.
That's also very strange because the condition it triggers on is exactly
the same.
Thanks,
Daniel
