On 05/03/2021 23.45, Andy Shevchenko wrote:
On Thu, Mar 4, 2021 at 11:40 PM Hector Martin <marcan@xxxxxxxxx> wrote:
ARM64 currently defaults to posted MMIO (nGnRnE), but some devices
require the use of non-posted MMIO (nGnRE). Introduce a new ioremap()
variant to handle this case. ioremap_np() is aliased to ioremap() by
default on arches that do not implement this variant.
Hmm... But isn't it basically a requirement to those device drivers to
use readX()/writeX() instead of readX_relaxed() / writeX_relaxed()?
No, the write ops used do not matter. It's just that on these Apple SoCs
the fabric requires the mappings to be nGnRnE, else it just throws
SErrors on all writes and ignores them.
The difference between _relaxed and not is barrier behavior with regards
to DMA/memory accesses; this applies regardless of whether the writes
are E or nE. You can have relaxed accesses with nGnRnE and then you
would still have race conditions if you do not have a barrier between
the MMIO and accessing DMA memory. What nGnRnE buys you (on
platforms/buses where it works properly) is that you do need a dummy
read after a write to ensure completion.
All of this is to some extent moot on these SoCs; it's not that we need
the drivers to use nGnRnE for some correctness reason, it's that the
SoCs force us to use it or else everything breaks, which was the
motivation for this change. But since on most other SoCs both are valid
options, this does allow some other drivers/platforms to opt into nGnRnE
if they have a good reason to do so.
Though you just made me notice two mistakes in the commit description:
first, it describes the old v2 version, for v3 I made ioremap_np() just
return NULL on arches that don't implement it. Second, nGnRnE and nGnRE
are backwards. Oops. I'll fix it for the next version.
#define IORESOURCE_MEM_32BIT (3<<3)
#define IORESOURCE_MEM_SHADOWABLE (1<<5) /* dup: IORESOURCE_SHADOWABLE */
#define IORESOURCE_MEM_EXPANSIONROM (1<<6)
+#define IORESOURCE_MEM_NONPOSTED (1<<7)
Not sure it's the right location (in a bit field) for this flag.
Do you have a better suggestion? It seemed logical to put it here, as a
flag on memory-type I/O resources.
--
Hector Martin (marcan@xxxxxxxxx)
Public Key: https://mrcn.st/pub
