Re: [PATCH v4 12/18] of/address: Add infrastructure to declare MMIO as non-posted

From: Rob Herring <robh@kernel.org>
To: Hector Martin <marcan@marcan.st>
Cc: linux-arm-kernel@lists.infradead.org,
	Marc Zyngier <maz@kernel.org>, Arnd Bergmann <arnd@kernel.org>,
	Olof Johansson <olof@lixom.net>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Mark Kettenis <mark.kettenis@xs4all.nl>,
	Tony Lindgren <tony@atomide.com>,
	Mohamed Mediouni <mohamed.mediouni@caramail.com>,
	Stan Skowronek <stan@corellium.com>,
	Alexander Graf <graf@amazon.com>, Will Deacon <will@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Christoph Hellwig <hch@infradead.org>,
	"David S. Miller" <davem@davemloft.net>,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 12/18] of/address: Add infrastructure to declare MMIO as non-posted
Date: Tue, 6 Apr 2021 11:47:48 -0500	[thread overview]
Message-ID: <20210406164748.GA1937719@robh.at.kernel.org> (raw)
In-Reply-To: <20210402090542.131194-13-marcan@marcan.st>

On Fri, Apr 02, 2021 at 06:05:36PM +0900, Hector Martin wrote:
> This implements the 'nonposted-mmio' boolean property. Placing this
> property in a bus marks all direct child devices as requiring
> non-posted MMIO mappings. If no such property is found, the default
> is posted MMIO.
> 
> of_mmio_is_nonposted() performs this check to determine if a given
> device has requested non-posted MMIO.
> 
> of_address_to_resource() uses this to set the IORESOURCE_MEM_NONPOSTED
> flag on resources that require non-posted MMIO.
> 
> of_iomap() and of_io_request_and_map() then use this flag to pick the
> correct ioremap() variant.
> 
> This mechanism is currently restricted to builds that support Apple ARM
> platforms, as an optimization.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/of/address.c       | 43 ++++++++++++++++++++++++++++++++++++--
>  include/linux/of_address.h |  1 +
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 73ddf2540f3f..6485cc536e81 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -847,6 +847,9 @@ static int __of_address_to_resource(struct device_node *dev,
>  		return -EINVAL;
>  	memset(r, 0, sizeof(struct resource));
>  
> +	if (of_mmio_is_nonposted(dev))
> +		flags |= IORESOURCE_MEM_NONPOSTED;
> +
>  	r->start = taddr;
>  	r->end = taddr + size - 1;
>  	r->flags = flags;
> @@ -896,7 +899,10 @@ void __iomem *of_iomap(struct device_node *np, int index)
>  	if (of_address_to_resource(np, index, &res))
>  		return NULL;
>  
> -	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));
>  }
>  EXPORT_SYMBOL(of_iomap);
>  
> @@ -928,7 +934,11 @@ void __iomem *of_io_request_and_map(struct device_node *np, int index,
>  	if (!request_mem_region(res.start, resource_size(&res), name))
>  		return IOMEM_ERR_PTR(-EBUSY);
>  
> -	mem = ioremap(res.start, resource_size(&res));
> +	if (res.flags & IORESOURCE_MEM_NONPOSTED)
> +		mem = ioremap_np(res.start, resource_size(&res));
> +	else
> +		mem = ioremap(res.start, resource_size(&res));
> +
>  	if (!mem) {
>  		release_mem_region(res.start, resource_size(&res));
>  		return IOMEM_ERR_PTR(-ENOMEM);
> @@ -1094,3 +1104,32 @@ bool of_dma_is_coherent(struct device_node *np)
>  	return false;
>  }
>  EXPORT_SYMBOL_GPL(of_dma_is_coherent);
> +
> +/**
> + * of_mmio_is_nonposted - Check if device uses non-posted MMIO
> + * @np:	device node
> + *
> + * Returns true if the "nonposted-mmio" property was found for
> + * the device's bus.
> + *
> + * This is currently only enabled on builds that support Apple ARM devices, as
> + * an optimization.
> + */
> +bool of_mmio_is_nonposted(struct device_node *np)
> +{
> +	struct device_node *parent;
> +	bool nonposted;
> +
> +	if (!IS_ENABLED(CONFIG_ARCH_APPLE))
> +		return false;
> +
> +	parent = of_get_parent(np);
> +	if (!parent)
> +		return false;
> +
> +	nonposted = of_property_read_bool(parent, "nonposted-mmio");
> +
> +	of_node_put(parent);
> +	return nonposted;
> +}
> +EXPORT_SYMBOL_GPL(of_mmio_is_nonposted);

Is this needed outside of of/address.c? If not, please make it static 
and don't export.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel