On Mon, Mar 08, 2021 at 09:29:54PM +0100, Arnd Bergmann wrote:

On Mon, Mar 8, 2021 at 4:56 PM Rob Herring [off-list ref] wrote:

quoted

On Fri, Mar 5, 2021 at 2:17 PM Arnd Bergmann [off-list ref] wrote:

quoted

On Fri, Mar 5, 2021 at 7:18 PM Hector Martin [off-list ref] wrote:

quoted

quoted

quoted

What's the code path using these functions on the M1 where we need to
return 'posted'? It's just downstream PCI mappings (PCI memory space),
right? Those would never hit these paths because they don't have a DT
node or if they do the memory space is not part of it. So can't the
check just be:

bool of_mmio_is_nonposted(struct device_node *np)
{
     return np && of_machine_is_compatible("apple,arm-platform");
}

Yes; the implementation was trying to be generic, but AIUI we don't need
this on M1 because the PCI mappings don't go through this codepath, and
nothing else needs posted mode. My first hack was something not too
unlike this, then I was going to get rid of apple,arm-platform and just
have this be a generic mechanism with the properties, but then we added
the optimization to not do the lookups on other platforms, and now we're
coming full circle... :-)

I never liked the idea of having a list of platforms that need a
special hack, please let's not go back to that.

I'm a fan of generic solutions as much as anyone, but not when there's
a single user. Yes, there could be more, but we haven't seen any yet
and Apple seems to have a knack for doing special things. I'm pretty
sure posted vs. non-posted has been a possibility with AXI buses from
the start, so it's not like this is a new thing we're going to see
frequently on new platforms.

Ok, but if we make it a platform specific bit, I would prefer not
to do the IORESOURCE_MEM_NONPOSTED flag either but
instead keep the logic in the device drivers that call ioremap().

That seems like an orthogonal decision to me.

This is obviously more work for the drivers, but at least it keeps
the common code free of the hack while also allowing drivers to
use ioremap_np() intentionally on other platforms.

I don't agree. The problem is within the interconnect. The device and 
its driver are unaware of this.

The other idea I had was doing a compatible other than 'simple-bus' for 
the bus node which could imply non-posted io and any other quirks in 
Apple's bus implementation. However, something different there means 
updates in lots of places (schemas, dtc checks, etc.) unless we kept 
'simple-bus' as a fallback.

Let's just stick with 'nonposted-mmio', but drop 'posted-mmio'. I'd 
rather know if and when we need 'posted-mmio'. It does need to be added 
to the DT spec[1] and schema[2] though (GH PRs are fine for both).

quoted

The other situation I worry about here is another arch has implicitly
defaulted to non-posted instead of posted. It could just be non-posted
was what worked everywhere and Linux couldn't distinguish. Now someone
sees we have this new posted vs. non-posted handling and can optimize
some mappings on their platform and we have to have per arch defaults
(like 'dma-coherent' now).

I think one of the dark secrets of MMIO is that a lot of drivers
get the posted behavior wrong by assuming that a writel() before
a spin_unlock() is protected by that unlock. This may in fact work
on many architectures but is broken on PCI and on local devices
for ARM.

Having a properly working (on non-PCI) ioremap_np() interface
would be nice here, as it could be used to document when drivers
rely on non-posted behavior, and cause the ioremap to fail when
running on architectures that don't support nonposted maps.

Good to know.

Rob

[1] https://github.com/devicetree-org/devicetree-specification
[2] https://github.com/devicetree-org/dt-schema