Re: [RFT PATCH v3 12/27] of/address: Add infrastructure to declare MMIO as non-posted

From: Arnd Bergmann
Date: Fri Mar 05 2021 - 16:29:32 EST


On Fri, Mar 5, 2021 at 7:18 PM Hector Martin <marcan@xxxxxxxxx> wrote:
>
> On 06/03/2021 02.39, Rob Herring wrote:
> >> - return ioremap(res.start, resource_size(&res));
> >> + if (res.flags & IORESOURCE_MEM_NONPOSTED)
> >> + return ioremap_np(res.start, resource_size(&res));
> >> + else
> >> + return ioremap(res.start, resource_size(&res));
> >
> > This and the devm variants all scream for a ioremap_extended()
> > function. IOW, it would be better if the ioremap flavor was a
> > parameter. Unless we could implement that just for arm64 first, that's
> > a lot of refactoring...
>
> I agree, but yeah... that's one big refactor to try to do now...

FWIW, there is ioremap_prot() that Christoph introduced in 2019
for a few architectures. I suppose it would be nice to lift
that out architecture specific code and completely replace the
unusual variants, leaving only ioremap(), ioremap_prot() and
memremap() but dropping the _nc, _cached, _wc, _wt and _np
versions in favor of an extensible set of flags.

Then again, I would not make that a prerequisite for the merge
of the M1 support.

> > 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.

Arnd