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