On Mon, Mar 08, 2021 at 09:29:54PM +0100, Arnd Bergmann wrote: > On Mon, Mar 8, 2021 at 4:56 PM Rob Herring <r...@kernel.org> wrote: > > On Fri, Mar 5, 2021 at 2:17 PM Arnd Bergmann <a...@kernel.org> wrote: > > > On Fri, Mar 5, 2021 at 7:18 PM Hector Martin <mar...@marcan.st> wrote: > > > > > > > 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). > > 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