[RFC/PATCH 0/9] IOMMU probe deferral support

[RFC/PATCH 0/9] IOMMU probe deferral support

[Laurent Pinchart]

Hello,

This patch series attempts to implement support for deferring probe of both
IOMMU drivers and bus master drivers.

The relationship between bus masters and IOMMUs creates a strong ordering
during initialization of devices. As in the general case IOMMUs are hidden
behind the DMA mapping API, IOMMU support relies on the automatic setup of DMA
operations without any direct intervention of bus master drivers.

DMA operations are set up when platform devices are added to the system. This
requires IOMMUs to be available at that time. On systems where ordering of
device add and probe can't be guaranteed (such as, but not limited to,
DT-based systems) this caused incorrect DMA operation setup. This has been
addressed by a patches series [1] that introduced a DT-based early
registration mechanism for IOMMUs.

However, that mechanism fails to address all issues. Various dependencies
exist between IOMMU devices and other devices, in particular on clocks and on
power domains (as mentioned by Marek in [2]). While there are mechanisms to
handle some of them without probe deferral (for instance by using the
OF_DECLARE macros to register clock drivers), generalizing those mechanisms
would essentially recreate a probe ordering mechanism similar to link order
probe ordering and couldn't really scale.

Additionally, IOMMUs could also be present hot-pluggable devices and depend on 
resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case. 
For all those reasons probe deferral for IOMMUs has been considered as desired
if it can be implemented cleanly. For more in-depth information see [3].

This RFC series is a first attempt at implementing IOMMU probe deferral
support cleanly.

The core idea is to move setup of DMA operations from device add time to
device probe time, implemented in patch 6/9. It could be possible to move
setup of other DMA parameters (namely masks and offset) to probe time as well,
but that change would be more intrusive and has a higher risk of introducing
regressions. For that reason I've decided to keep DMA masks and offset setup
at device add time and thus split DMA configuration in masks and operations
(patch 5/9). This can be revisited if we decide that the DMA mapping API
shouldn't require masks and offset to be set before probe time.

Patch 8/9 then defers probe of bus master drivers when required IOMMUs are not
available yet. This requires knowing when a failed IOMMU lookup should be
considered as permanent or temporary. I've reused the OF_DECLARE_IOMMU for
this purpose, considering that the presence of a driver compatible with the
IOMMU DT node indicates that the failure is temporary and probing of the bus
master device should be deferred.

Note that only IOMMU drivers using the recent .of_xlate() mechanism for
DT-based IOMMU reference can cause probe deferral of bus master devices. The
.add_device() mechanism isn't supported in this case.

As an example I've converted the ipmmu-vmsa driver to the new API in patch 9/9.

At this point many enhancements are possible, but I'd like to receive feedback
on the proposed approach before basing more patches on this series. One
particular point I would like to address (or see being addressed) in the
future is the use of struct iommu_ops with of_iommu_get_ops() and
of_iommu_set_ops(). I believe we should introduct a struct iommu and register
IOMMU instances instead of IOMMU operations. That should bring us one step
closer to removing bus_set_iommu().

[1] http://www.spinics.net/lists/arm-kernel/msg382787.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html
[3] https://lkml.org/lkml/2015/2/16/345

Laurent Pinchart (9):
  arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
  arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits
    range
  of: dma: Move range size workaround to of_dma_get_range()
  of: dma: Make of_dma_deconfigure() public
  of: dma: Split of_configure_dma() into mask and ops configuration
  drivers: platform: Configure dma operations at probe time
  iommu: of: Document the of_iommu_configure() function
  iommu: of: Handle IOMMU lookup failure with deferred probing or error
  iommu/ipmmu-vmsa: Use DT-based instantiation

 arch/arm/include/asm/dma-iommu.h |   2 +-
 arch/arm/mm/dma-mapping.c        |  21 +++--
 drivers/base/platform.c          |   9 ++
 drivers/iommu/ipmmu-vmsa.c       | 189 +++++++++++++--------------------------
 drivers/iommu/of_iommu.c         |  29 +++++-
 drivers/of/address.c             |  20 ++++-
 drivers/of/device.c              |  77 ++++++++++------
 drivers/of/of_pci.c              |   3 +-
 drivers/of/platform.c            |  16 ++--
 include/linux/of_device.h        |  14 ++-
 10 files changed, 195 insertions(+), 185 deletions(-)

-- 
Regards,

Laurent Pinchart

[RFC/PATCH 1/9] arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

[Laurent Pinchart]

The arch_setup_dma_ops() function is in charge of setting dma_ops with a
call to set_dma_ops(). set_dma_ops() is also called from

- highbank and mvebu bus notifiers
- dmabounce (to be replaced with swiotlb)
- arm_iommu_attach_device

(arm_iommu_attach_device is itself called from IOMMU and bus master
device drivers)

To allow the arch_setup_dma_ops() call to be moved from device add time
to device probe time we must ensure that dma_ops already setup by any of
the above callers will not be overriden.

Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to
of_xlate and taking care of highbank and mvebu, the workaround should be
removed.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/mm/dma-mapping.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 09c5fe3d30c2..7aa5e339a596 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2117,6 +2117,15 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
 	struct dma_map_ops *dma_ops;
 
 	dev->archdata.dma_coherent = coherent;
+
+	/*
+	 * Don't override the dma_ops if they have already been set. Ideally
+	 * this should be the only location where dma_ops are set, remove this
+	 * check when all other callers of set_dma_ops will have disappeared.
+	 */
+	if (dev->archdata.dma_ops)
+		return;
+
 	if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu))
 		dma_ops = arm_get_iommu_dma_map_ops(coherent);
 	else
-- 
2.3.6

Re: [RFC/PATCH 1/9] arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

[Will Deacon]

On Fri, May 15, 2015 at 12:00:02AM +0100, Laurent Pinchart wrote:
> The arch_setup_dma_ops() function is in charge of setting dma_ops with a
> call to set_dma_ops(). set_dma_ops() is also called from
> 
> - highbank and mvebu bus notifiers
> - dmabounce (to be replaced with swiotlb)
> - arm_iommu_attach_device
> 
> (arm_iommu_attach_device is itself called from IOMMU and bus master
> device drivers)
> 
> To allow the arch_setup_dma_ops() call to be moved from device add time
> to device probe time we must ensure that dma_ops already setup by any of
> the above callers will not be overriden.
> 
> Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to

s/Aftering/After/

> of_xlate and taking care of highbank and mvebu, the workaround should be
> removed.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  arch/arm/mm/dma-mapping.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 09c5fe3d30c2..7aa5e339a596 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2117,6 +2117,15 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
>  	struct dma_map_ops *dma_ops;
>  
>  	dev->archdata.dma_coherent = coherent;
> +
> +	/*
> +	 * Don't override the dma_ops if they have already been set. Ideally
> +	 * this should be the only location where dma_ops are set, remove this
> +	 * check when all other callers of set_dma_ops will have disappeared.
> +	 */
> +	if (dev->archdata.dma_ops)
> +		return;

Maybe printing a warning at the same time will encourage people to migrate
away from the existing behaviour?

Will

[RFC/PATCH 2/9] arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits range

[Laurent Pinchart]

The arm_iommu_create_mapping() function takes the IOMMU mapping size as
a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
bus masters would support the full 32 bits range. Fix this by passing
the size as a 64 bits integer, and limiting it to 4GB for now.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/include/asm/dma-iommu.h |  2 +-
 arch/arm/mm/dma-mapping.c        | 12 +++---------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 8e3fcb924db6..2ef282f96651 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -25,7 +25,7 @@ struct dma_iommu_mapping {
 };
 
 struct dma_iommu_mapping *
-arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
+arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size);
 
 void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
 
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7aa5e339a596..c69e216a0cf9 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
  * arm_iommu_attach_device function.
  */
 struct dma_iommu_mapping *
-arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
+arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size)
 {
 	unsigned int bits = size >> PAGE_SHIFT;
 	unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
@@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
 	int extensions = 1;
 	int err = -ENOMEM;
 
-	if (!bitmap_size)
+	/* Limit the size to 4GB for now. */
+	if (!size || size > (1ULL << 32))
 		return ERR_PTR(-EINVAL);
 
 	if (bitmap_size > PAGE_SIZE) {
@@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
 	if (!iommu)
 		return false;
 
-	/*
-	 * currently arm_iommu_create_mapping() takes a max of size_t
-	 * for size param. So check this limit for now.
-	 */
-	if (size > SIZE_MAX)
-		return false;
-
 	mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
 	if (IS_ERR(mapping)) {
 		pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
-- 
2.3.6

Re: [RFC/PATCH 2/9] arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits range

[Robin Murphy]

Hi Laurent,

On 15/05/15 00:00, Laurent Pinchart wrote:
> The arm_iommu_create_mapping() function takes the IOMMU mapping size as
> a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
> bus masters would support the full 32 bits range. Fix this by passing
> the size as a 64 bits integer, and limiting it to 4GB for now.

FYI there's already an almost-identical patch in -rc3 from Marek fixing 
this: 1424532b2163 ("ARM: 8347/1: dma-mapping: fix off-by-one check in 
arm_setup_iommu_dma_ops").

Robin.

>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   arch/arm/include/asm/dma-iommu.h |  2 +-
>   arch/arm/mm/dma-mapping.c        | 12 +++---------
>   2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
> index 8e3fcb924db6..2ef282f96651 100644
> --- a/arch/arm/include/asm/dma-iommu.h
> +++ b/arch/arm/include/asm/dma-iommu.h
> @@ -25,7 +25,7 @@ struct dma_iommu_mapping {
>   };
>
>   struct dma_iommu_mapping *
> -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
> +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size);
>
>   void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7aa5e339a596..c69e216a0cf9 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
>    * arm_iommu_attach_device function.
>    */
>   struct dma_iommu_mapping *
> -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
> +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64 size)
>   {
>   	unsigned int bits = size >> PAGE_SHIFT;
>   	unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
> @@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
>   	int extensions = 1;
>   	int err = -ENOMEM;
>
> -	if (!bitmap_size)
> +	/* Limit the size to 4GB for now. */
> +	if (!size || size > (1ULL << 32))
>   		return ERR_PTR(-EINVAL);
>
>   	if (bitmap_size > PAGE_SIZE) {
> @@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
>   	if (!iommu)
>   		return false;
>
> -	/*
> -	 * currently arm_iommu_create_mapping() takes a max of size_t
> -	 * for size param. So check this limit for now.
> -	 */
> -	if (size > SIZE_MAX)
> -		return false;
> -
>   	mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
>   	if (IS_ERR(mapping)) {
>   		pr_warn("Failed to create %llu-byte IOMMU mapping for device %s\n",
>

Re: [RFC/PATCH 2/9] arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits range

[Laurent Pinchart]

Hi Robin,

On Tuesday 19 May 2015 11:17:32 Robin Murphy wrote:
> On 15/05/15 00:00, Laurent Pinchart wrote:
> > The arm_iommu_create_mapping() function takes the IOMMU mapping size as
> > a size_t, limiting the size to 4GB - 1 on 32 bit platforms while most
> > bus masters would support the full 32 bits range. Fix this by passing
> > the size as a 64 bits integer, and limiting it to 4GB for now.
> 
> FYI there's already an almost-identical patch in -rc3 from Marek fixing
> this: 1424532b2163 ("ARM: 8347/1: dma-mapping: fix off-by-one check in
> arm_setup_iommu_dma_ops").

Indeed, I should have rebased on top of -rc3. I suppose it's a good indication 
that I was on the right track :-) I'll drop this patch from my series.

> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >   arch/arm/include/asm/dma-iommu.h |  2 +-
> >   arch/arm/mm/dma-mapping.c        | 12 +++---------
> >   2 files changed, 4 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/dma-iommu.h
> > b/arch/arm/include/asm/dma-iommu.h index 8e3fcb924db6..2ef282f96651
> > 100644
> > --- a/arch/arm/include/asm/dma-iommu.h
> > +++ b/arch/arm/include/asm/dma-iommu.h
> > @@ -25,7 +25,7 @@ struct dma_iommu_mapping {
> >   };
> >   
> >   struct dma_iommu_mapping *
> > -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> > size);
> > +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, u64
> > size);
> >   void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping);
> > 
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 7aa5e339a596..c69e216a0cf9 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -1878,7 +1878,7 @@ struct dma_map_ops iommu_coherent_ops = {
> > 
> >    * arm_iommu_attach_device function.
> >    */
> >   
> >   struct dma_iommu_mapping *
> > 
> > -arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t
> > size) +arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base,
> > u64 size)
> >   {
> >   	unsigned int bits = size >> PAGE_SHIFT;
> >   	unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
> > @@ -1886,7 +1886,8 @@ arm_iommu_create_mapping(struct bus_type *bus,
> > dma_addr_t base, size_t size)
> >   	int extensions = 1;
> >   	int err = -ENOMEM;
> > 
> > -	if (!bitmap_size)
> > +	/* Limit the size to 4GB for now. */
> > +	if (!size || size > (1ULL << 32))
> >   		return ERR_PTR(-EINVAL);
> >   	
> >   	if (bitmap_size > PAGE_SIZE) {
> > @@ -2057,13 +2058,6 @@ static bool arm_setup_iommu_dma_ops(struct device
> > *dev, u64 dma_base, u64 size,
> >   	if (!iommu)
> >   		return false;
> > 
> > -	/*
> > -	 * currently arm_iommu_create_mapping() takes a max of size_t
> > -	 * for size param. So check this limit for now.
> > -	 */
> > -	if (size > SIZE_MAX)
> > -		return false;
> > -
> >   	mapping = arm_iommu_create_mapping(dev->bus, dma_base, size);
> >   	if (IS_ERR(mapping)) {
> >   		pr_warn("Failed to create %llu-byte IOMMU mapping for device 
%s\n",

-- 
Regards,

Laurent Pinchart

[RFC/PATCH 3/9] of: dma: Move range size workaround to of_dma_get_range()

[Laurent Pinchart]

Invalid dma-ranges values should be worked around when retrieving the
DMA range in of_dma_get_range(), not by all callers of the function.
This isn't much of a problem now that we have a single caller, but that
situation will change when moving DMA configuration to device probe
time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/address.c | 20 ++++++++++++++++++--
 drivers/of/device.c  | 15 ---------------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 78a7dcbec7d8..75eebd19ebfa 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
  *	CPU addr (phys_addr_t)	: pna cells
  *	size			: nsize cells
  *
- * It returns -ENODEV if "dma-ranges" property was not found
- * for this device in DT.
+ * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
+ * this device in DT, or -EINVAL if the CPU address or size is invalid.
  */
 int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
 {
@@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
 	*dma_addr = dmaaddr;
 
 	*size = of_read_number(ranges + naddr + pna, nsize);
+	/*
+	 * DT nodes sometimes incorrectly set the size as a mask. Work around
+	 * those incorrect DT by computing the size as mask + 1.
+	 */
+	if (*size & 1) {
+		pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
+			__func__, *size, np->full_name);
+		*size = *size + 1;
+	}
+
+	if (!*size) {
+		pr_err("%s: invalid size zero for dma-range in node(%s)\n",
+		       __func__, np->full_name);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
 		 *dma_addr, *paddr, *size);
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 20c1332a0018..530aa1ed3e1b 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 		size = dev->coherent_dma_mask + 1;
 	} else {
 		offset = PFN_DOWN(paddr - dma_addr);
-
-		/*
-		 * Add a work around to treat the size as mask + 1 in case
-		 * it is defined in DT as a mask.
-		 */
-		if (size & 1) {
-			dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
-				 size);
-			size = size + 1;
-		}
-
-		if (!size) {
-			dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
-			return;
-		}
 		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
 	}
 
-- 
2.3.6

Re: [RFC/PATCH 3/9] of: dma: Move range size workaround to of_dma_get_range()

[Will Deacon]

On Fri, May 15, 2015 at 12:00:04AM +0100, Laurent Pinchart wrote:
> Invalid dma-ranges values should be worked around when retrieving the
> DMA range in of_dma_get_range(), not by all callers of the function.
> This isn't much of a problem now that we have a single caller, but that
> situation will change when moving DMA configuration to device probe
> time.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/address.c | 20 ++++++++++++++++++--
>  drivers/of/device.c  | 15 ---------------
>  2 files changed, 18 insertions(+), 17 deletions(-)

Looks good to me:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 78a7dcbec7d8..75eebd19ebfa 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
>   *	CPU addr (phys_addr_t)	: pna cells
>   *	size			: nsize cells
>   *
> - * It returns -ENODEV if "dma-ranges" property was not found
> - * for this device in DT.
> + * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
> + * this device in DT, or -EINVAL if the CPU address or size is invalid.
>   */
>  int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
>  {
> @@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
>  	*dma_addr = dmaaddr;
>  
>  	*size = of_read_number(ranges + naddr + pna, nsize);
> +	/*
> +	 * DT nodes sometimes incorrectly set the size as a mask. Work around
> +	 * those incorrect DT by computing the size as mask + 1.
> +	 */
> +	if (*size & 1) {
> +		pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
> +			__func__, *size, np->full_name);
> +		*size = *size + 1;
> +	}
> +
> +	if (!*size) {
> +		pr_err("%s: invalid size zero for dma-range in node(%s)\n",
> +		       __func__, np->full_name);
> +		ret = -EINVAL;
> +		goto out;
> +	}
>  
>  	pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
>  		 *dma_addr, *paddr, *size);
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 20c1332a0018..530aa1ed3e1b 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>  		size = dev->coherent_dma_mask + 1;
>  	} else {
>  		offset = PFN_DOWN(paddr - dma_addr);
> -
> -		/*
> -		 * Add a work around to treat the size as mask + 1 in case
> -		 * it is defined in DT as a mask.
> -		 */
> -		if (size & 1) {
> -			dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> -				 size);
> -			size = size + 1;
> -		}
> -
> -		if (!size) {
> -			dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> -			return;
> -		}
>  		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>  	}
>  
> -- 
> 2.3.6
> 

Re: [RFC/PATCH 3/9] of: dma: Move range size workaround to of_dma_get_range()

[Rob Herring]

On Thu, May 14, 2015 at 6:00 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Invalid dma-ranges values should be worked around when retrieving the
> DMA range in of_dma_get_range(), not by all callers of the function.
> This isn't much of a problem now that we have a single caller, but that
> situation will change when moving DMA configuration to device probe
> time.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/address.c | 20 ++++++++++++++++++--
>  drivers/of/device.c  | 15 ---------------

Please use get_maintainers.pl. Anyway, looks fine to me:

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

Rob

>  2 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 78a7dcbec7d8..75eebd19ebfa 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -924,8 +924,8 @@ EXPORT_SYMBOL(of_io_request_and_map);
>   *     CPU addr (phys_addr_t)  : pna cells
>   *     size                    : nsize cells
>   *
> - * It returns -ENODEV if "dma-ranges" property was not found
> - * for this device in DT.
> + * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
> + * this device in DT, or -EINVAL if the CPU address or size is invalid.
>   */
>  int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
>  {
> @@ -986,6 +986,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
>         *dma_addr = dmaaddr;
>
>         *size = of_read_number(ranges + naddr + pna, nsize);
> +       /*
> +        * DT nodes sometimes incorrectly set the size as a mask. Work around
> +        * those incorrect DT by computing the size as mask + 1.
> +        */
> +       if (*size & 1) {
> +               pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
> +                       __func__, *size, np->full_name);
> +               *size = *size + 1;
> +       }
> +
> +       if (!*size) {
> +               pr_err("%s: invalid size zero for dma-range in node(%s)\n",
> +                      __func__, np->full_name);
> +               ret = -EINVAL;
> +               goto out;
> +       }
>
>         pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
>                  *dma_addr, *paddr, *size);
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 20c1332a0018..530aa1ed3e1b 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -109,21 +109,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>                 size = dev->coherent_dma_mask + 1;
>         } else {
>                 offset = PFN_DOWN(paddr - dma_addr);
> -
> -               /*
> -                * Add a work around to treat the size as mask + 1 in case
> -                * it is defined in DT as a mask.
> -                */
> -               if (size & 1) {
> -                       dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
> -                                size);
> -                       size = size + 1;
> -               }
> -
> -               if (!size) {
> -                       dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
> -                       return;
> -               }
>                 dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>         }
>
> --
> 2.3.6
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC/PATCH 4/9] of: dma: Make of_dma_deconfigure() public

[Laurent Pinchart]

As part of moving DMA initializing to probe time the
of_dma_deconfigure() function will need to be called from different
source files. Make it public and move it to drivers/of/device.c where
the of_dma_configure() function is.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/device.c       | 12 ++++++++++++
 drivers/of/platform.c     |  5 -----
 include/linux/of_device.h |  3 +++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index 530aa1ed3e1b..f1b84f464fe1 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -135,6 +135,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_dma_configure);
 
+/**
+ * of_dma_deconfigure - Clean up DMA configuration
+ * @dev:	Device for which to clean up DMA configuration
+ *
+ * Clean up all configuration performed by of_dma_configure_ops() and free all
+ * resources that have been allocated.
+ */
+void of_dma_deconfigure(struct device *dev)
+{
+	arch_teardown_dma_ops(dev);
+}
+
 int of_device_register(struct platform_device *pdev)
 {
 	device_initialize(&pdev->dev);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index a01f57c9e34e..7a660c79ff84 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -149,11 +149,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
 }
 EXPORT_SYMBOL(of_device_alloc);
 
-static void of_dma_deconfigure(struct device *dev)
-{
-	arch_teardown_dma_ops(dev);
-}
-
 /**
  * of_platform_device_create_pdata - Alloc, initialize and register an of_device
  * @np: pointer to node to create device for
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 22801b10cef5..6710807b0653 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -54,6 +54,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 }
 
 void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_deconfigure(struct device *dev);
 #else /* CONFIG_OF */
 
 static inline int of_driver_match_device(struct device *dev,
@@ -93,6 +94,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 }
 static inline void of_dma_configure(struct device *dev, struct device_node *np)
 {}
+static inline void of_dma_deconfigure(struct device *dev)
+{}
 #endif /* CONFIG_OF */
 
 #endif /* _LINUX_OF_DEVICE_H */
-- 
2.3.6

Re: [RFC/PATCH 4/9] of: dma: Make of_dma_deconfigure() public

[Will Deacon]

(sorry for the mid-review delay, I got side-tracked yesterday evening)

On Fri, May 15, 2015 at 12:00:05AM +0100, Laurent Pinchart wrote:
> As part of moving DMA initializing to probe time the
> of_dma_deconfigure() function will need to be called from different
> source files. Make it public and move it to drivers/of/device.c where
> the of_dma_configure() function is.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/device.c       | 12 ++++++++++++
>  drivers/of/platform.c     |  5 -----
>  include/linux/of_device.h |  3 +++
>  3 files changed, 15 insertions(+), 5 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will

> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 530aa1ed3e1b..f1b84f464fe1 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -135,6 +135,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_dma_configure);
>  
> +/**
> + * of_dma_deconfigure - Clean up DMA configuration
> + * @dev:	Device for which to clean up DMA configuration
> + *
> + * Clean up all configuration performed by of_dma_configure_ops() and free all
> + * resources that have been allocated.
> + */
> +void of_dma_deconfigure(struct device *dev)
> +{
> +	arch_teardown_dma_ops(dev);
> +}
> +
>  int of_device_register(struct platform_device *pdev)
>  {
>  	device_initialize(&pdev->dev);
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index a01f57c9e34e..7a660c79ff84 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -149,11 +149,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
>  }
>  EXPORT_SYMBOL(of_device_alloc);
>  
> -static void of_dma_deconfigure(struct device *dev)
> -{
> -	arch_teardown_dma_ops(dev);
> -}
> -
>  /**
>   * of_platform_device_create_pdata - Alloc, initialize and register an of_device
>   * @np: pointer to node to create device for
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index 22801b10cef5..6710807b0653 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -54,6 +54,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>  }
>  
>  void of_dma_configure(struct device *dev, struct device_node *np);
> +void of_dma_deconfigure(struct device *dev);
>  #else /* CONFIG_OF */
>  
>  static inline int of_driver_match_device(struct device *dev,
> @@ -93,6 +94,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>  }
>  static inline void of_dma_configure(struct device *dev, struct device_node *np)
>  {}
> +static inline void of_dma_deconfigure(struct device *dev)
> +{}
>  #endif /* CONFIG_OF */
>  
>  #endif /* _LINUX_OF_DEVICE_H */
> -- 
> 2.3.6
> 

Re: [RFC/PATCH 4/9] of: dma: Make of_dma_deconfigure() public

[Rob Herring]

On Thu, May 14, 2015 at 6:00 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> As part of moving DMA initializing to probe time the
> of_dma_deconfigure() function will need to be called from different
> source files. Make it public and move it to drivers/of/device.c where
> the of_dma_configure() function is.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

One minor fix below, but otherwise:

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

> ---
>  drivers/of/device.c       | 12 ++++++++++++
>  drivers/of/platform.c     |  5 -----
>  include/linux/of_device.h |  3 +++
>  3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 530aa1ed3e1b..f1b84f464fe1 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -135,6 +135,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>  }
>  EXPORT_SYMBOL_GPL(of_dma_configure);
>
> +/**
> + * of_dma_deconfigure - Clean up DMA configuration
> + * @dev:       Device for which to clean up DMA configuration
> + *
> + * Clean up all configuration performed by of_dma_configure_ops() and free all
> + * resources that have been allocated.
> + */
> +void of_dma_deconfigure(struct device *dev)
> +{
> +       arch_teardown_dma_ops(dev);
> +}

You need an EXPORT_SYMBOL_GPL

[RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration

[Laurent Pinchart]

The of_configure_dma() function configures both the DMA masks and ops.
Moving DMA ops configuration to probe time would thus also delay
configuration of the DMA masks, which might not be safe. To avoid issues
split the configuration in two to allow keeping masks configuration at
device add time and move ops configuration to device probe time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/of/device.c       | 48 ++++++++++++++++++++++++++++++++++-------------
 drivers/of/of_pci.c       |  3 ++-
 drivers/of/platform.c     |  6 ++++--
 include/linux/of_device.h | 11 +++++++++--
 4 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index f1b84f464fe1..3cb3f78a6d13 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
 }
 
 /**
- * of_dma_configure - Setup DMA configuration
+ * of_dma_configure - Setup DMA masks and offset
  * @dev:	Device to apply DMA configuration
  * @np:		Pointer to OF node having DMA configuration
  *
@@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
  * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
  * to fix up DMA configuration.
  */
-void of_dma_configure(struct device *dev, struct device_node *np)
+void of_dma_configure_masks(struct device *dev, struct device_node *np)
 {
-	u64 dma_addr, paddr, size;
-	int ret;
-	bool coherent;
+	u64 dma_addr, paddr, size, range_mask;
 	unsigned long offset;
-	struct iommu_ops *iommu;
+	int ret;
 
 	/*
 	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
@@ -105,9 +103,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 
 	ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
 	if (ret < 0) {
-		dma_addr = offset = 0;
-		size = dev->coherent_dma_mask + 1;
+		range_mask = dev->coherent_dma_mask + 1;
+		offset = 0;
 	} else {
+		range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
 		offset = PFN_DOWN(paddr - dma_addr);
 		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
 	}
@@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 	 * Limit coherent and dma mask based on size and default mask
 	 * set by the driver.
 	 */
-	dev->coherent_dma_mask = min(dev->coherent_dma_mask,
-				     DMA_BIT_MASK(ilog2(dma_addr + size)));
-	*dev->dma_mask = min((*dev->dma_mask),
-			     DMA_BIT_MASK(ilog2(dma_addr + size)));
+	dev->coherent_dma_mask = min(dev->coherent_dma_mask, range_mask);
+	*dev->dma_mask = min((*dev->dma_mask), range_mask);
+}
+EXPORT_SYMBOL_GPL(of_dma_configure_masks);
+
+/**
+ * of_dma_configure_ops - Setup DMA operations
+ * @dev:	Device to apply DMA configuration
+ * @np:		Pointer to OF node having DMA configuration
+ *
+ * Try to get devices's DMA configuration from DT and update it
+ * accordingly.
+ */
+int of_dma_configure_ops(struct device *dev, struct device_node *np)
+{
+	u64 dma_addr, paddr, size;
+	struct iommu_ops *iommu;
+	bool coherent;
+	int ret;
+
+	ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
+	if (ret < 0) {
+		dma_addr = 0;
+		size = dev->coherent_dma_mask + 1;
+	}
 
 	coherent = of_dma_is_coherent(np);
 	dev_dbg(dev, "device is%sdma coherent\n",
@@ -132,8 +152,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
 		iommu ? " " : " not ");
 
 	arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+	return 0;
 }
-EXPORT_SYMBOL_GPL(of_dma_configure);
+EXPORT_SYMBOL_GPL(of_dma_configure_ops);
 
 /**
  * of_dma_deconfigure - Clean up DMA configuration
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5b6494..c65803da2067 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -132,7 +132,8 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
 	if (!bridge->parent)
 		return;
 
-	of_dma_configure(dev, bridge->parent->of_node);
+	of_dma_configure_masks(dev, bridge->parent->of_node);
+	of_dma_configure_ops(dev, bridge->parent->of_node);
 	pci_put_host_bridge_device(bridge);
 }
 EXPORT_SYMBOL_GPL(of_pci_dma_configure);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 7a660c79ff84..9a29f09b7723 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -177,7 +177,8 @@ static struct platform_device *of_platform_device_create_pdata(
 
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
-	of_dma_configure(&dev->dev, dev->dev.of_node);
+	of_dma_configure_masks(&dev->dev, dev->dev.of_node);
+	of_dma_configure_ops(&dev->dev, dev->dev.of_node);
 
 	if (of_device_add(dev) != 0) {
 		of_dma_deconfigure(&dev->dev);
@@ -240,7 +241,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 		dev_set_name(&dev->dev, "%s", bus_id);
 	else
 		of_device_make_bus_id(&dev->dev);
-	of_dma_configure(&dev->dev, dev->dev.of_node);
+	of_dma_configure_masks(&dev->dev, dev->dev.of_node);
+	of_dma_configure_ops(&dev->dev, dev->dev.of_node);
 
 	/* Allow the HW Peripheral ID to be overridden */
 	prop = of_get_property(node, "arm,primecell-periphid", NULL);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 6710807b0653..794f234c7c7d 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -53,7 +53,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 	return of_node_get(cpu_dev->of_node);
 }
 
-void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_configure_masks(struct device *dev, struct device_node *np);
+int of_dma_configure_ops(struct device *dev, struct device_node *np);
 void of_dma_deconfigure(struct device *dev);
 #else /* CONFIG_OF */
 
@@ -92,8 +93,14 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
 {
 	return NULL;
 }
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
+static inline void of_dma_configure_masks(struct device *dev,
+					  struct device_node *np)
 {}
+static inline int of_dma_configure_ops(struct device *dev,
+				       struct device_node *np)
+{
+	return 0;
+}
 static inline void of_dma_deconfigure(struct device *dev)
 {}
 #endif /* CONFIG_OF */
-- 
2.3.6

Re: [RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration

[Will Deacon]

On Fri, May 15, 2015 at 12:00:06AM +0100, Laurent Pinchart wrote:
> The of_configure_dma() function configures both the DMA masks and ops.
> Moving DMA ops configuration to probe time would thus also delay
> configuration of the DMA masks, which might not be safe. To avoid issues
> split the configuration in two to allow keeping masks configuration at
> device add time and move ops configuration to device probe time.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/device.c       | 48 ++++++++++++++++++++++++++++++++++-------------
>  drivers/of/of_pci.c       |  3 ++-
>  drivers/of/platform.c     |  6 ++++--
>  include/linux/of_device.h | 11 +++++++++--
>  4 files changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index f1b84f464fe1..3cb3f78a6d13 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
>  }
>  
>  /**
> - * of_dma_configure - Setup DMA configuration
> + * of_dma_configure - Setup DMA masks and offset
>   * @dev:	Device to apply DMA configuration
>   * @np:		Pointer to OF node having DMA configuration
>   *
> @@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
>   * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
>   * to fix up DMA configuration.
>   */
> -void of_dma_configure(struct device *dev, struct device_node *np)
> +void of_dma_configure_masks(struct device *dev, struct device_node *np)
>  {
> -	u64 dma_addr, paddr, size;
> -	int ret;
> -	bool coherent;
> +	u64 dma_addr, paddr, size, range_mask;
>  	unsigned long offset;
> -	struct iommu_ops *iommu;
> +	int ret;
>  
>  	/*
>  	 * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
> @@ -105,9 +103,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>  
>  	ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>  	if (ret < 0) {
> -		dma_addr = offset = 0;
> -		size = dev->coherent_dma_mask + 1;
> +		range_mask = dev->coherent_dma_mask + 1;

Hmm, do we still want this '+ 1'...

> +		offset = 0;
>  	} else {
> +		range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
>  		offset = PFN_DOWN(paddr - dma_addr);
>  		dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>  	}
> @@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>  	 * Limit coherent and dma mask based on size and default mask
>  	 * set by the driver.
>  	 */
> -	dev->coherent_dma_mask = min(dev->coherent_dma_mask,
> -				     DMA_BIT_MASK(ilog2(dma_addr + size)));
> -	*dev->dma_mask = min((*dev->dma_mask),
> -			     DMA_BIT_MASK(ilog2(dma_addr + size)));
> +	dev->coherent_dma_mask = min(dev->coherent_dma_mask, range_mask);

... if we now compare directly with the mask here? In fact, this setting
of the coherent_dma_mask could just belong in the else clause, no?

> +	*dev->dma_mask = min((*dev->dma_mask), range_mask);

Similar comment here, but we need to do this for both cases I guess.

> +}
> +EXPORT_SYMBOL_GPL(of_dma_configure_masks);
> +
> +/**
> + * of_dma_configure_ops - Setup DMA operations
> + * @dev:	Device to apply DMA configuration
> + * @np:		Pointer to OF node having DMA configuration
> + *
> + * Try to get devices's DMA configuration from DT and update it

Weird spelling.

Will

Re: [RFC/PATCH 5/9] of: dma: Split of_configure_dma() into mask and ops configuration

[Rob Herring]

On Thu, May 14, 2015 at 6:00 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The of_configure_dma() function configures both the DMA masks and ops.
> Moving DMA ops configuration to probe time would thus also delay
> configuration of the DMA masks, which might not be safe. To avoid issues
> split the configuration in two to allow keeping masks configuration at
> device add time and move ops configuration to device probe time.

Why is it not safe? The probe is deferred before drivers' probe happens.

Rob

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/of/device.c       | 48 ++++++++++++++++++++++++++++++++++-------------
>  drivers/of/of_pci.c       |  3 ++-
>  drivers/of/platform.c     |  6 ++++--
>  include/linux/of_device.h | 11 +++++++++--
>  4 files changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index f1b84f464fe1..3cb3f78a6d13 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -70,7 +70,7 @@ int of_device_add(struct platform_device *ofdev)
>  }
>
>  /**
> - * of_dma_configure - Setup DMA configuration
> + * of_dma_configure - Setup DMA masks and offset
>   * @dev:       Device to apply DMA configuration
>   * @np:                Pointer to OF node having DMA configuration
>   *
> @@ -81,13 +81,11 @@ int of_device_add(struct platform_device *ofdev)
>   * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
>   * to fix up DMA configuration.
>   */
> -void of_dma_configure(struct device *dev, struct device_node *np)
> +void of_dma_configure_masks(struct device *dev, struct device_node *np)
>  {
> -       u64 dma_addr, paddr, size;
> -       int ret;
> -       bool coherent;
> +       u64 dma_addr, paddr, size, range_mask;
>         unsigned long offset;
> -       struct iommu_ops *iommu;
> +       int ret;
>
>         /*
>          * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
> @@ -105,9 +103,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>
>         ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>         if (ret < 0) {
> -               dma_addr = offset = 0;
> -               size = dev->coherent_dma_mask + 1;
> +               range_mask = dev->coherent_dma_mask + 1;
> +               offset = 0;
>         } else {
> +               range_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
>                 offset = PFN_DOWN(paddr - dma_addr);
>                 dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
>         }
> @@ -118,10 +117,31 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>          * Limit coherent and dma mask based on size and default mask
>          * set by the driver.
>          */
> -       dev->coherent_dma_mask = min(dev->coherent_dma_mask,
> -                                    DMA_BIT_MASK(ilog2(dma_addr + size)));
> -       *dev->dma_mask = min((*dev->dma_mask),
> -                            DMA_BIT_MASK(ilog2(dma_addr + size)));
> +       dev->coherent_dma_mask = min(dev->coherent_dma_mask, range_mask);
> +       *dev->dma_mask = min((*dev->dma_mask), range_mask);
> +}
> +EXPORT_SYMBOL_GPL(of_dma_configure_masks);
> +
> +/**
> + * of_dma_configure_ops - Setup DMA operations
> + * @dev:       Device to apply DMA configuration
> + * @np:                Pointer to OF node having DMA configuration
> + *
> + * Try to get devices's DMA configuration from DT and update it
> + * accordingly.
> + */
> +int of_dma_configure_ops(struct device *dev, struct device_node *np)
> +{
> +       u64 dma_addr, paddr, size;
> +       struct iommu_ops *iommu;
> +       bool coherent;
> +       int ret;
> +
> +       ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> +       if (ret < 0) {
> +               dma_addr = 0;
> +               size = dev->coherent_dma_mask + 1;
> +       }
>
>         coherent = of_dma_is_coherent(np);
>         dev_dbg(dev, "device is%sdma coherent\n",
> @@ -132,8 +152,10 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>                 iommu ? " " : " not ");
>
>         arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> +
> +       return 0;
>  }
> -EXPORT_SYMBOL_GPL(of_dma_configure);
> +EXPORT_SYMBOL_GPL(of_dma_configure_ops);
>
>  /**
>   * of_dma_deconfigure - Clean up DMA configuration
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 5751dc5b6494..c65803da2067 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -132,7 +132,8 @@ void of_pci_dma_configure(struct pci_dev *pci_dev)
>         if (!bridge->parent)
>                 return;
>
> -       of_dma_configure(dev, bridge->parent->of_node);
> +       of_dma_configure_masks(dev, bridge->parent->of_node);
> +       of_dma_configure_ops(dev, bridge->parent->of_node);
>         pci_put_host_bridge_device(bridge);
>  }
>  EXPORT_SYMBOL_GPL(of_pci_dma_configure);
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 7a660c79ff84..9a29f09b7723 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -177,7 +177,8 @@ static struct platform_device *of_platform_device_create_pdata(
>
>         dev->dev.bus = &platform_bus_type;
>         dev->dev.platform_data = platform_data;
> -       of_dma_configure(&dev->dev, dev->dev.of_node);
> +       of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> +       of_dma_configure_ops(&dev->dev, dev->dev.of_node);
>
>         if (of_device_add(dev) != 0) {
>                 of_dma_deconfigure(&dev->dev);
> @@ -240,7 +241,8 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
>                 dev_set_name(&dev->dev, "%s", bus_id);
>         else
>                 of_device_make_bus_id(&dev->dev);
> -       of_dma_configure(&dev->dev, dev->dev.of_node);
> +       of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> +       of_dma_configure_ops(&dev->dev, dev->dev.of_node);
>
>         /* Allow the HW Peripheral ID to be overridden */
>         prop = of_get_property(node, "arm,primecell-periphid", NULL);
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index 6710807b0653..794f234c7c7d 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -53,7 +53,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>         return of_node_get(cpu_dev->of_node);
>  }
>
> -void of_dma_configure(struct device *dev, struct device_node *np);
> +void of_dma_configure_masks(struct device *dev, struct device_node *np);
> +int of_dma_configure_ops(struct device *dev, struct device_node *np);
>  void of_dma_deconfigure(struct device *dev);
>  #else /* CONFIG_OF */
>
> @@ -92,8 +93,14 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>  {
>         return NULL;
>  }
> -static inline void of_dma_configure(struct device *dev, struct device_node *np)
> +static inline void of_dma_configure_masks(struct device *dev,
> +                                         struct device_node *np)
>  {}
> +static inline int of_dma_configure_ops(struct device *dev,
> +                                      struct device_node *np)
> +{
> +       return 0;
> +}
>  static inline void of_dma_deconfigure(struct device *dev)
>  {}
>  #endif /* CONFIG_OF */
> --
> 2.3.6
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC/PATCH 6/9] drivers: platform: Configure dma operations at probe time

[Laurent Pinchart]

Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/base/platform.c | 9 +++++++++
 drivers/of/platform.c   | 7 +++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index ebf034b97278..508a866859dc 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
 	if (ret < 0)
 		return ret;
 
+	ret = of_dma_configure_ops(_dev, _dev->of_node);
+	if (ret < 0)
+		goto done;
+
 	ret = dev_pm_domain_attach(_dev, true);
 	if (ret != -EPROBE_DEFER) {
 		ret = drv->probe(dev);
@@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
 			dev_pm_domain_detach(_dev, true);
 	}
 
+	if (ret)
+		of_dma_deconfigure(_dev);
+
+done:
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
 		dev_warn(_dev, "probe deferral not supported\n");
 		ret = -ENXIO;
@@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
 
 	ret = drv->remove(dev);
 	dev_pm_domain_detach(_dev, true);
+	of_dma_deconfigure(_dev);
 
 	return ret;
 }
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 9a29f09b7723..fc939bec799e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -178,10 +178,8 @@ static struct platform_device *of_platform_device_create_pdata(
 	dev->dev.bus = &platform_bus_type;
 	dev->dev.platform_data = platform_data;
 	of_dma_configure_masks(&dev->dev, dev->dev.of_node);
-	of_dma_configure_ops(&dev->dev, dev->dev.of_node);
 
 	if (of_device_add(dev) != 0) {
-		of_dma_deconfigure(&dev->dev);
 		platform_device_put(dev);
 		goto err_clear_flag;
 	}
@@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device *dev, void *data)
 	if (dev->bus == &platform_bus_type)
 		platform_device_unregister(to_platform_device(dev));
 #ifdef CONFIG_ARM_AMBA
-	else if (dev->bus == &amba_bustype)
+	else if (dev->bus == &amba_bustype) {
 		amba_device_unregister(to_amba_device(dev));
+		of_dma_deconfigure(dev);
+	}
 #endif
 
-	of_dma_deconfigure(dev);
 	of_node_clear_flag(dev->of_node, OF_POPULATED);
 	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
 	return 0;
-- 
2.3.6

Re: [RFC/PATCH 6/9] drivers: platform: Configure dma operations at probe time

[Greg Kroah-Hartman]

On Fri, May 15, 2015 at 02:00:07AM +0300, Laurent Pinchart wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/base/platform.c | 9 +++++++++
>  drivers/of/platform.c   | 7 +++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [RFC/PATCH 6/9] drivers: platform: Configure dma operations at probe time

[Robin Murphy]

Hi Laurent,

On 15/05/15 00:00, Laurent Pinchart wrote:
> Configuring DMA ops at probe time will allow deferring device probe when
> the IOMMU isn't available yet.

This is great, as I think it also subtly solves the ordering problem the 
current domain allocation has with platform devices. WRT to your comment 
on the other thread, this actually makes things slightly saner for IOMMU 
groups - the group assignment has to happen after device creation or 
else some sysfs stuff blows up, so of_xlate is far too early and the 
add_device callback is a reasonable place for it to be (until we can 
move it out of every driver and into bus code). However, we're currently 
attaching the device to the automatic domain long before that, so things 
happen logically backwards and drivers like the ARM SMMU which actually 
use the group to store relevant data get all confused.

With this change, the existing attach_device call in arch_setup_dma_ops 
will actually work far more reliably, and I might be able to revive my 
attempt to port the ARM SMMU driver over to of_xlate :D

Robin.

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   drivers/base/platform.c | 9 +++++++++
>   drivers/of/platform.c   | 7 +++----
>   2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index ebf034b97278..508a866859dc 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
>   	if (ret < 0)
>   		return ret;
>
> +	ret = of_dma_configure_ops(_dev, _dev->of_node);
> +	if (ret < 0)
> +		goto done;
> +
>   	ret = dev_pm_domain_attach(_dev, true);
>   	if (ret != -EPROBE_DEFER) {
>   		ret = drv->probe(dev);
> @@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
>   			dev_pm_domain_detach(_dev, true);
>   	}
>
> +	if (ret)
> +		of_dma_deconfigure(_dev);
> +
> +done:
>   	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
>   		dev_warn(_dev, "probe deferral not supported\n");
>   		ret = -ENXIO;
> @@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
>
>   	ret = drv->remove(dev);
>   	dev_pm_domain_detach(_dev, true);
> +	of_dma_deconfigure(_dev);
>
>   	return ret;
>   }
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 9a29f09b7723..fc939bec799e 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -178,10 +178,8 @@ static struct platform_device *of_platform_device_create_pdata(
>   	dev->dev.bus = &platform_bus_type;
>   	dev->dev.platform_data = platform_data;
>   	of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> -	of_dma_configure_ops(&dev->dev, dev->dev.of_node);
>
>   	if (of_device_add(dev) != 0) {
> -		of_dma_deconfigure(&dev->dev);
>   		platform_device_put(dev);
>   		goto err_clear_flag;
>   	}
> @@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device *dev, void *data)
>   	if (dev->bus == &platform_bus_type)
>   		platform_device_unregister(to_platform_device(dev));
>   #ifdef CONFIG_ARM_AMBA
> -	else if (dev->bus == &amba_bustype)
> +	else if (dev->bus == &amba_bustype) {
>   		amba_device_unregister(to_amba_device(dev));
> +		of_dma_deconfigure(dev);
> +	}
>   #endif
>
> -	of_dma_deconfigure(dev);
>   	of_node_clear_flag(dev->of_node, OF_POPULATED);
>   	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
>   	return 0;
>

Re: [RFC/PATCH 6/9] drivers: platform: Configure dma operations at probe time

[Laurent Pinchart]

Hi Robin,

On Tuesday 19 May 2015 11:39:17 Robin Murphy wrote:
> On 15/05/15 00:00, Laurent Pinchart wrote:
> > Configuring DMA ops at probe time will allow deferring device probe when
> > the IOMMU isn't available yet.
> 
> This is great, as I think it also subtly solves the ordering problem the
> current domain allocation has with platform devices. WRT to your comment
> on the other thread, this actually makes things slightly saner for IOMMU
> groups - the group assignment has to happen after device creation or
> else some sysfs stuff blows up, so of_xlate is far too early and the
> add_device callback is a reasonable place for it to be (until we can
> move it out of every driver and into bus code). However, we're currently
> attaching the device to the automatic domain long before that, so things
> happen logically backwards and drivers like the ARM SMMU which actually
> use the group to store relevant data get all confused.
> 
> With this change, the existing attach_device call in arch_setup_dma_ops
> will actually work far more reliably, and I might be able to revive my
> attempt to port the ARM SMMU driver over to of_xlate :D

Please do :-) I second IOMMU driver using the of_xlate + probe deferral 
mechanism would help validate this patch series.

> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >   drivers/base/platform.c | 9 +++++++++
> >   drivers/of/platform.c   | 7 +++----
> >   2 files changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> > index ebf034b97278..508a866859dc 100644
> > --- a/drivers/base/platform.c
> > +++ b/drivers/base/platform.c
> > @@ -516,6 +516,10 @@ static int platform_drv_probe(struct device *_dev)
> >   	if (ret < 0)
> >   		return ret;
> > 
> > +	ret = of_dma_configure_ops(_dev, _dev->of_node);
> > +	if (ret < 0)
> > +		goto done;
> > +
> >   	ret = dev_pm_domain_attach(_dev, true);
> >   	if (ret != -EPROBE_DEFER) {
> >   		ret = drv->probe(dev);
> > @@ -523,6 +527,10 @@ static int platform_drv_probe(struct device *_dev)
> >   			dev_pm_domain_detach(_dev, true);
> >   	}
> > 
> > +	if (ret)
> > +		of_dma_deconfigure(_dev);
> > +
> > 
> > +done:
> >   	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
> >   		dev_warn(_dev, "probe deferral not supported\n");
> >   		ret = -ENXIO;
> > @@ -544,6 +552,7 @@ static int platform_drv_remove(struct device *_dev)
> > 
> >   	ret = drv->remove(dev);
> >   	dev_pm_domain_detach(_dev, true);
> > +	of_dma_deconfigure(_dev);
> >   	return ret;
> >   }
> > 
> > diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> > index 9a29f09b7723..fc939bec799e 100644
> > --- a/drivers/of/platform.c
> > +++ b/drivers/of/platform.c
> > @@ -178,10 +178,8 @@ static struct platform_device
> > *of_platform_device_create_pdata(
> >   	dev->dev.bus = &platform_bus_type;
> >   	dev->dev.platform_data = platform_data;
> >   	of_dma_configure_masks(&dev->dev, dev->dev.of_node);
> > -	of_dma_configure_ops(&dev->dev, dev->dev.of_node);
> > 
> >   	if (of_device_add(dev) != 0) {
> > -		of_dma_deconfigure(&dev->dev);
> >   		platform_device_put(dev);
> >   		goto err_clear_flag;
> >   	}
> > @@ -465,11 +463,12 @@ static int of_platform_device_destroy(struct device
> > *dev, void *data)
> >   	if (dev->bus == &platform_bus_type)
> >   		platform_device_unregister(to_platform_device(dev));
> >   
> >   #ifdef CONFIG_ARM_AMBA
> > -	else if (dev->bus == &amba_bustype)
> > +	else if (dev->bus == &amba_bustype) {
> >   		amba_device_unregister(to_amba_device(dev));
> > +		of_dma_deconfigure(dev);
> > +	}
> >   #endif
> > 
> > -	of_dma_deconfigure(dev);
> >   	of_node_clear_flag(dev->of_node, OF_POPULATED);
> >   	of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
> >   	return 0;

-- 
Regards,

Laurent Pinchart

[RFC/PATCH 7/9] iommu: of: Document the of_iommu_configure() function

[Laurent Pinchart]

The function isn't trivial, document its behaviour.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/of_iommu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 43429ab62228..b922ed4f9fb3 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,6 +133,19 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
 	return ops;
 }
 
+/**
+ * of_iommu_configure - Configure and return the IOMMU for a device
+ * @dev: device for which to configure the IOMMU
+ * @master_np: device node of the bus master connected to the IOMMU
+ *
+ * The master_np parameter specifies the device node of the bus master seen by
+ * the IOMMU. This is usually the device node of the dev device, but can be the
+ * device node of a bridge when the device is dynamically discovered and
+ * instantiated and thus has no device node (such as PCI devices for instance).
+ *
+ * Return a pointer to the iommu_ops for the device, NULL if the device isn't
+ * connected to an IOMMU, or a negative value if an error occurs.
+ */
 struct iommu_ops *of_iommu_configure(struct device *dev,
 				     struct device_node *master_np)
 {
-- 
2.3.6

Re: [RFC/PATCH 7/9] iommu: of: Document the of_iommu_configure() function

[Will Deacon]

On Fri, May 15, 2015 at 12:00:08AM +0100, Laurent Pinchart wrote:
> The function isn't trivial, document its behaviour.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/iommu/of_iommu.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Thanks for doing this.

Acked-by: Will Deacon <will.deacon@arm.com>

Will

> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 43429ab62228..b922ed4f9fb3 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -133,6 +133,19 @@ struct iommu_ops *of_iommu_get_ops(struct device_node *np)
>  	return ops;
>  }
>  
> +/**
> + * of_iommu_configure - Configure and return the IOMMU for a device
> + * @dev: device for which to configure the IOMMU
> + * @master_np: device node of the bus master connected to the IOMMU
> + *
> + * The master_np parameter specifies the device node of the bus master seen by
> + * the IOMMU. This is usually the device node of the dev device, but can be the
> + * device node of a bridge when the device is dynamically discovered and
> + * instantiated and thus has no device node (such as PCI devices for instance).
> + *
> + * Return a pointer to the iommu_ops for the device, NULL if the device isn't
> + * connected to an IOMMU, or a negative value if an error occurs.
> + */
>  struct iommu_ops *of_iommu_configure(struct device *dev,
>  				     struct device_node *master_np)
>  {
> -- 
> 2.3.6
> 

[RFC/PATCH 8/9] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Laurent Pinchart]

Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.

The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.

The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.

The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.

The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/of_iommu.c | 16 +++++++++++++---
 drivers/of/device.c      |  2 ++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index b922ed4f9fb3..e98f19a3b740 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -170,8 +170,18 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
 		np = iommu_spec.np;
 		ops = of_iommu_get_ops(np);
 
-		if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
+		if (!ops) {
+			const struct of_device_id *oid;
+
+			oid = of_match_node(&__iommu_of_table, np);
+			ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL;
+			goto err_put_node;
+		}
+
+		if (!ops->of_xlate || ops->of_xlate(dev, &iommu_spec)) {
+			ops = NULL;
 			goto err_put_node;
+		}
 
 		of_node_put(np);
 		idx++;
@@ -181,7 +191,7 @@ struct iommu_ops *of_iommu_configure(struct device *dev,
 
 err_put_node:
 	of_node_put(np);
-	return NULL;
+	return ops;
 }
 
 void __init of_iommu_init(void)
@@ -192,7 +202,7 @@ void __init of_iommu_init(void)
 	for_each_matching_node_and_match(np, matches, &match) {
 		const of_iommu_init_fn init_fn = match->data;
 
-		if (init_fn(np))
+		if (init_fn && init_fn(np))
 			pr_err("Failed to initialise IOMMU %s\n",
 				of_node_full_name(np));
 	}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 3cb3f78a6d13..f4964ab21026 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -148,6 +148,8 @@ int of_dma_configure_ops(struct device *dev, struct device_node *np)
 		coherent ? " " : " not ");
 
 	iommu = of_iommu_configure(dev, np);
+	if (IS_ERR(iommu))
+		return PTR_ERR(iommu);
 	dev_dbg(dev, "device is%sbehind an iommu\n",
 		iommu ? " " : " not ");
 
-- 
2.3.6

Re: [RFC/PATCH 8/9] iommu: of: Handle IOMMU lookup failure with deferred probing or error

[Will Deacon]

Hi Laurent,

On Fri, May 15, 2015 at 12:00:09AM +0100, Laurent Pinchart wrote:
> Failures to look up an IOMMU when parsing the DT iommus property need to
> be handled separately from the .of_xlate() failures to support deferred
> probing.
> 
> The lack of a registered IOMMU can be caused by the lack of a driver for
> the IOMMU, the IOMMU device probe not having been performed yet, having
> been deferred, or having failed.
> 
> The first case occurs when the device tree describes the bus master and
> IOMMU topology correctly but no device driver exists for the IOMMU yet
> or the device driver has not been compiled in. Return NULL, the caller
> will configure the device without an IOMMU.
> 
> The second and third cases are handled by deferring the probe of the bus
> master device which will eventually get reprobed after the IOMMU.
> 
> The last case is currently handled by deferring the probe of the bus
> master device as well. A mechanism to either configure the bus master
> device without an IOMMU or to fail the bus master device probe depending
> on whether the IOMMU is optional or mandatory would be a good
> enhancement.

I appreciate that you're just looking to handle early initialisation
failures here, but do you have any thoughts on how to deal with failures
later on when e.g. the DMA-mapping API is trying to create IOMMU domains.

One potential problem I foresee is if we try to add all devices to a common
DMA domain, we may get -ENOSPC-style failures due to limited resources on
the IOMMU. In this case, we'd probably want to fall-back to non-IOMMU DMA
ops, but that in-turn could have consequences on things like dma-coherent.

It's all a bit murky, so I'd be glad to hear any thoughts you might have
around this.

Anyway, this patch looks fine:

  Acked-by: Will Deacon <will.deacon@arm.com>

but we should consider how all of this will get used too.

Will

[RFC/PATCH 9/9] iommu/ipmmu-vmsa: Use DT-based instantiation

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 189 ++++++++++++++-------------------------------
 1 file changed, 60 insertions(+), 129 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 24a950091458..7fa2afb5d7d1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -18,6 +18,8 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
@@ -30,7 +32,6 @@
 struct ipmmu_vmsa_device {
 	struct device *dev;
 	void __iomem *base;
-	struct list_head list;
 
 	unsigned int num_utlbs;
 
@@ -54,9 +55,6 @@ struct ipmmu_vmsa_archdata {
 	unsigned int num_utlbs;
 };
 
-static DEFINE_SPINLOCK(ipmmu_devices_lock);
-static LIST_HEAD(ipmmu_devices);
-
 static struct ipmmu_vmsa_domain *to_vmsa_domain(struct iommu_domain *dom)
 {
 	return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
@@ -578,110 +576,81 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 	return domain->iop->iova_to_phys(domain->iop, iova);
 }
 
-static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device *dev,
-			    unsigned int *utlbs, unsigned int num_utlbs)
+static void ipmmu_remove_device(struct device *dev)
 {
-	unsigned int i;
-
-	for (i = 0; i < num_utlbs; ++i) {
-		struct of_phandle_args args;
-		int ret;
-
-		ret = of_parse_phandle_with_args(dev->of_node, "iommus",
-						 "#iommu-cells", i, &args);
-		if (ret < 0)
-			return ret;
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 
-		of_node_put(args.np);
+	if (!archdata)
+		return;
 
-		if (args.np != mmu->dev->of_node || args.args_count != 1)
-			return -EINVAL;
+	arm_iommu_detach_device(dev);
+	iommu_group_remove_device(dev);
 
-		utlbs[i] = args.args[0];
-	}
+	kfree(archdata->utlbs);
+	kfree(archdata);
 
-	return 0;
+	dev->archdata.iommu = NULL;
 }
 
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_of_xlate(struct device *dev, struct of_phandle_args *args)
 {
 	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
-	struct iommu_group *group = NULL;
+	struct platform_device *pdev;
+	unsigned int num_utlbs;
 	unsigned int *utlbs;
-	unsigned int i;
-	int num_utlbs;
-	int ret = -ENODEV;
-
-	if (dev->archdata.iommu) {
-		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
-			 dev_name(dev));
-		return -EINVAL;
-	}
+	unsigned int utlb;
+	int ret;
 
 	/* Find the master corresponding to the device. */
-
-	num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
-					       "#iommu-cells");
-	if (num_utlbs < 0)
+	pdev = of_find_device_by_node(args->np);
+	if (!pdev) {
+		dev_dbg(dev, "%s: ipmmu pdev not found\n", __func__);
 		return -ENODEV;
-
-	utlbs = kcalloc(num_utlbs, sizeof(*utlbs), GFP_KERNEL);
-	if (!utlbs)
-		return -ENOMEM;
-
-	spin_lock(&ipmmu_devices_lock);
-
-	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		ret = ipmmu_find_utlbs(mmu, dev, utlbs, num_utlbs);
-		if (!ret) {
-			/*
-			 * TODO Take a reference to the MMU to protect
-			 * against device removal.
-			 */
-			break;
-		}
 	}
 
-	spin_unlock(&ipmmu_devices_lock);
-
-	if (ret < 0)
+	mmu = platform_get_drvdata(pdev);
+	if (!mmu) {
+		dev_dbg(dev, "%s: ipmmu not found\n", __func__);
 		return -ENODEV;
-
-	for (i = 0; i < num_utlbs; ++i) {
-		if (utlbs[i] >= mmu->num_utlbs) {
-			ret = -EINVAL;
-			goto error;
-		}
 	}
 
-	/* Create a device group and add the device to it. */
-	group = iommu_group_alloc();
-	if (IS_ERR(group)) {
-		dev_err(dev, "Failed to allocate IOMMU group\n");
-		ret = PTR_ERR(group);
-		goto error;
+	/* Allocate arch data if not already done. */
+	if (!dev->archdata.iommu) {
+		dev->archdata.iommu = kzalloc(sizeof(*archdata), GFP_KERNEL);
+		if (!dev->archdata.iommu)
+			return -ENOMEM;
 	}
 
-	ret = iommu_group_add_device(group, dev);
-	iommu_group_put(group);
+	archdata = dev->archdata.iommu;
+	archdata->mmu = mmu;
 
-	if (ret < 0) {
-		dev_err(dev, "Failed to add device to IPMMU group\n");
-		group = NULL;
-		goto error;
+	/*
+	 * We don't support handling a device through different IOMMU
+	 * instances.
+	 */
+	if (archdata->mmu && archdata->mmu != mmu) {
+		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
+			 dev_name(dev));
+		return -EINVAL;
 	}
 
-	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
-	if (!archdata) {
-		ret = -ENOMEM;
-		goto error;
+	/* Validate and store the microTLB number. */
+	utlb = args->args[0];
+	if (utlb >= mmu->num_utlbs) {
+		dev_dbg(dev, "%s: invalid utlb %u\n", __func__, utlb);
+		return -EINVAL;
 	}
 
-	archdata->mmu = mmu;
+	num_utlbs = archdata->num_utlbs + 1;
+	utlbs = krealloc(archdata->utlbs, num_utlbs * sizeof(*utlbs),
+			 GFP_KERNEL);
+	if (utlbs == NULL)
+		return -ENOMEM;
+	utlbs[archdata->num_utlbs] = utlb;
+
 	archdata->utlbs = utlbs;
 	archdata->num_utlbs = num_utlbs;
-	dev->archdata.iommu = archdata;
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -699,50 +668,27 @@ static int ipmmu_add_device(struct device *dev)
 						   SZ_1G, SZ_2G);
 		if (IS_ERR(mapping)) {
 			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
-			ret = PTR_ERR(mapping);
-			goto error;
+			return PTR_ERR(mapping);
 		}
 
 		mmu->mapping = mapping;
 	}
 
-	/* Attach the ARM VA mapping to the device. */
+	/*
+	 * Detach the device from the default ARM VA mapping and attach it to
+	 * our private mapping.
+	 */
+	arm_iommu_detach_device(dev);
 	ret = arm_iommu_attach_device(dev, mmu->mapping);
 	if (ret < 0) {
 		dev_err(dev, "Failed to attach device to VA mapping\n");
-		goto error;
+		return ret;
 	}
 
 	return 0;
-
-error:
-	arm_iommu_release_mapping(mmu->mapping);
-
-	kfree(dev->archdata.iommu);
-	kfree(utlbs);
-
-	dev->archdata.iommu = NULL;
-
-	if (!IS_ERR_OR_NULL(group))
-		iommu_group_remove_device(dev);
-
-	return ret;
 }
 
-static void ipmmu_remove_device(struct device *dev)
-{
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
-
-	arm_iommu_detach_device(dev);
-	iommu_group_remove_device(dev);
-
-	kfree(archdata->utlbs);
-	kfree(archdata);
-
-	dev->archdata.iommu = NULL;
-}
-
-static const struct iommu_ops ipmmu_ops = {
+static struct iommu_ops ipmmu_ops = {
 	.domain_alloc = ipmmu_domain_alloc,
 	.domain_free = ipmmu_domain_free,
 	.attach_dev = ipmmu_attach_device,
@@ -751,8 +697,8 @@ static const struct iommu_ops ipmmu_ops = {
 	.unmap = ipmmu_unmap,
 	.map_sg = default_iommu_map_sg,
 	.iova_to_phys = ipmmu_iova_to_phys,
-	.add_device = ipmmu_add_device,
 	.remove_device = ipmmu_remove_device,
+	.of_xlate = ipmmu_of_xlate,
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 };
 
@@ -831,10 +777,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 	 * ipmmu_init() after the probe function returns.
 	 */
 
-	spin_lock(&ipmmu_devices_lock);
-	list_add(&mmu->list, &ipmmu_devices);
-	spin_unlock(&ipmmu_devices_lock);
-
+	of_iommu_set_ops(mmu->dev->of_node, &ipmmu_ops);
 	platform_set_drvdata(pdev, mmu);
 
 	return 0;
@@ -844,10 +787,6 @@ static int ipmmu_remove(struct platform_device *pdev)
 {
 	struct ipmmu_vmsa_device *mmu = platform_get_drvdata(pdev);
 
-	spin_lock(&ipmmu_devices_lock);
-	list_del(&mmu->list);
-	spin_unlock(&ipmmu_devices_lock);
-
 	arm_iommu_release_mapping(mmu->mapping);
 
 	ipmmu_device_reset(mmu);
@@ -883,14 +822,6 @@ static int __init ipmmu_init(void)
 	return 0;
 }
 
-static void __exit ipmmu_exit(void)
-{
-	return platform_driver_unregister(&ipmmu_driver);
-}
-
 subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
 
-MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
-MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>");
-MODULE_LICENSE("GPL v2");
+IOMMU_OF_DECLARE(ipmmu_vmsa_of, "renesas,ipmmu-vmsa", NULL);
-- 
2.3.6

Re: [RFC/PATCH 0/9] IOMMU probe deferral support

[Marek Szyprowski]

Hello,

On 2015-05-15 01:00, Laurent Pinchart wrote:
> This patch series attempts to implement support for deferring probe of both
> IOMMU drivers and bus master drivers.
>
> The relationship between bus masters and IOMMUs creates a strong ordering
> during initialization of devices. As in the general case IOMMUs are hidden
> behind the DMA mapping API, IOMMU support relies on the automatic setup of DMA
> operations without any direct intervention of bus master drivers.
>
> DMA operations are set up when platform devices are added to the system. This
> requires IOMMUs to be available at that time. On systems where ordering of
> device add and probe can't be guaranteed (such as, but not limited to,
> DT-based systems) this caused incorrect DMA operation setup. This has been
> addressed by a patches series [1] that introduced a DT-based early
> registration mechanism for IOMMUs.
>
> However, that mechanism fails to address all issues. Various dependencies
> exist between IOMMU devices and other devices, in particular on clocks and on
> power domains (as mentioned by Marek in [2]). While there are mechanisms to
> handle some of them without probe deferral (for instance by using the
> OF_DECLARE macros to register clock drivers), generalizing those mechanisms
> would essentially recreate a probe ordering mechanism similar to link order
> probe ordering and couldn't really scale.
>
> Additionally, IOMMUs could also be present hot-pluggable devices and depend on
> resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case.
> For all those reasons probe deferral for IOMMUs has been considered as desired
> if it can be implemented cleanly. For more in-depth information see [3].
>
> This RFC series is a first attempt at implementing IOMMU probe deferral
> support cleanly.
>
> The core idea is to move setup of DMA operations from device add time to
> device probe time, implemented in patch 6/9. It could be possible to move
> setup of other DMA parameters (namely masks and offset) to probe time as well,
> but that change would be more intrusive and has a higher risk of introducing
> regressions. For that reason I've decided to keep DMA masks and offset setup
> at device add time and thus split DMA configuration in masks and operations
> (patch 5/9). This can be revisited if we decide that the DMA mapping API
> shouldn't require masks and offset to be set before probe time.
>
> Patch 8/9 then defers probe of bus master drivers when required IOMMUs are not
> available yet. This requires knowing when a failed IOMMU lookup should be
> considered as permanent or temporary. I've reused the OF_DECLARE_IOMMU for
> this purpose, considering that the presence of a driver compatible with the
> IOMMU DT node indicates that the failure is temporary and probing of the bus
> master device should be deferred.
>
> Note that only IOMMU drivers using the recent .of_xlate() mechanism for
> DT-based IOMMU reference can cause probe deferral of bus master devices. The
> .add_device() mechanism isn't supported in this case.
>
> As an example I've converted the ipmmu-vmsa driver to the new API in patch 9/9.
>
> At this point many enhancements are possible, but I'd like to receive feedback
> on the proposed approach before basing more patches on this series. One
> particular point I would like to address (or see being addressed) in the
> future is the use of struct iommu_ops with of_iommu_get_ops() and
> of_iommu_set_ops(). I believe we should introduct a struct iommu and register
> IOMMU instances instead of IOMMU operations. That should bring us one step
> closer to removing bus_set_iommu().

I've checked this patchset with my recent updates to Exynos IOMMU driver. It
works fine. Then I removed my exynos_iommu_of_setup() callback and 
relied only
on proper initialization from platform device probe() of all iommu 
controllers.
The driver also worked fine in such case.

The other change I had to make to get everything working was removal of some
hackery in dma-mapping internal structures in Exynos DRM driver, but this is
completely different story.

You can add the following tag for the of/iommu/dma-mapping patches:
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> [1] http://www.spinics.net/lists/arm-kernel/msg382787.html
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html
> [3] https://lkml.org/lkml/2015/2/16/345
>
> Laurent Pinchart (9):
>    arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
>    arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits
>      range
>    of: dma: Move range size workaround to of_dma_get_range()
>    of: dma: Make of_dma_deconfigure() public
>    of: dma: Split of_configure_dma() into mask and ops configuration
>    drivers: platform: Configure dma operations at probe time
>    iommu: of: Document the of_iommu_configure() function
>    iommu: of: Handle IOMMU lookup failure with deferred probing or error
>    iommu/ipmmu-vmsa: Use DT-based instantiation
>
>   arch/arm/include/asm/dma-iommu.h |   2 +-
>   arch/arm/mm/dma-mapping.c        |  21 +++--
>   drivers/base/platform.c          |   9 ++
>   drivers/iommu/ipmmu-vmsa.c       | 189 +++++++++++++--------------------------
>   drivers/iommu/of_iommu.c         |  29 +++++-
>   drivers/of/address.c             |  20 ++++-
>   drivers/of/device.c              |  77 ++++++++++------
>   drivers/of/of_pci.c              |   3 +-
>   drivers/of/platform.c            |  16 ++--
>   include/linux/of_device.h        |  14 ++-
>   10 files changed, 195 insertions(+), 185 deletions(-)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [RFC/PATCH 0/9] IOMMU probe deferral support

[Laura Abbott]

On 05/14/2015 04:00 PM, Laurent Pinchart wrote:
> Hello,
>
> This patch series attempts to implement support for deferring probe of both
> IOMMU drivers and bus master drivers.
>
> The relationship between bus masters and IOMMUs creates a strong ordering
> during initialization of devices. As in the general case IOMMUs are hidden
> behind the DMA mapping API, IOMMU support relies on the automatic setup of DMA
> operations without any direct intervention of bus master drivers.
>
> DMA operations are set up when platform devices are added to the system. This
> requires IOMMUs to be available at that time. On systems where ordering of
> device add and probe can't be guaranteed (such as, but not limited to,
> DT-based systems) this caused incorrect DMA operation setup. This has been
> addressed by a patches series [1] that introduced a DT-based early
> registration mechanism for IOMMUs.
>
> However, that mechanism fails to address all issues. Various dependencies
> exist between IOMMU devices and other devices, in particular on clocks and on
> power domains (as mentioned by Marek in [2]). While there are mechanisms to
> handle some of them without probe deferral (for instance by using the
> OF_DECLARE macros to register clock drivers), generalizing those mechanisms
> would essentially recreate a probe ordering mechanism similar to link order
> probe ordering and couldn't really scale.
>
> Additionally, IOMMUs could also be present hot-pluggable devices and depend on
> resources that are thus hot-plugged. OF_DECLARE wouldn't help in that case.
> For all those reasons probe deferral for IOMMUs has been considered as desired
> if it can be implemented cleanly. For more in-depth information see [3].
>
> This RFC series is a first attempt at implementing IOMMU probe deferral
> support cleanly.
>
> The core idea is to move setup of DMA operations from device add time to
> device probe time, implemented in patch 6/9. It could be possible to move
> setup of other DMA parameters (namely masks and offset) to probe time as well,
> but that change would be more intrusive and has a higher risk of introducing
> regressions. For that reason I've decided to keep DMA masks and offset setup
> at device add time and thus split DMA configuration in masks and operations
> (patch 5/9). This can be revisited if we decide that the DMA mapping API
> shouldn't require masks and offset to be set before probe time.
>
> Patch 8/9 then defers probe of bus master drivers when required IOMMUs are not
> available yet. This requires knowing when a failed IOMMU lookup should be
> considered as permanent or temporary. I've reused the OF_DECLARE_IOMMU for
> this purpose, considering that the presence of a driver compatible with the
> IOMMU DT node indicates that the failure is temporary and probing of the bus
> master device should be deferred.
>
> Note that only IOMMU drivers using the recent .of_xlate() mechanism for
> DT-based IOMMU reference can cause probe deferral of bus master devices. The
> .add_device() mechanism isn't supported in this case.
>
> As an example I've converted the ipmmu-vmsa driver to the new API in patch 9/9.
>
> At this point many enhancements are possible, but I'd like to receive feedback
> on the proposed approach before basing more patches on this series. One
> particular point I would like to address (or see being addressed) in the
> future is the use of struct iommu_ops with of_iommu_get_ops() and
> of_iommu_set_ops(). I believe we should introduct a struct iommu and register
> IOMMU instances instead of IOMMU operations. That should bring us one step
> closer to removing bus_set_iommu().
>
> [1] http://www.spinics.net/lists/arm-kernel/msg382787.html
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323238.html
> [3] https://lkml.org/lkml/2015/2/16/345
>
> Laurent Pinchart (9):
>    arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
>    arm: dma-mapping: Support IOMMU mappings spanning the full 32 bits
>      range
>    of: dma: Move range size workaround to of_dma_get_range()
>    of: dma: Make of_dma_deconfigure() public
>    of: dma: Split of_configure_dma() into mask and ops configuration
>    drivers: platform: Configure dma operations at probe time
>    iommu: of: Document the of_iommu_configure() function
>    iommu: of: Handle IOMMU lookup failure with deferred probing or error
>    iommu/ipmmu-vmsa: Use DT-based instantiation
>
>   arch/arm/include/asm/dma-iommu.h |   2 +-
>   arch/arm/mm/dma-mapping.c        |  21 +++--
>   drivers/base/platform.c          |   9 ++
>   drivers/iommu/ipmmu-vmsa.c       | 189 +++++++++++++--------------------------
>   drivers/iommu/of_iommu.c         |  29 +++++-
>   drivers/of/address.c             |  20 ++++-
>   drivers/of/device.c              |  77 ++++++++++------
>   drivers/of/of_pci.c              |   3 +-
>   drivers/of/platform.c            |  16 ++--
>   include/linux/of_device.h        |  14 ++-
>   10 files changed, 195 insertions(+), 185 deletions(-)
>

I no longer have hardware to test this on but the entire approach looks
reasonable to me.

Reviewed-by: Laura Abbott <labbott@redhat.com>

Re: [RFC/PATCH 0/9] IOMMU probe deferral support

[Will Deacon]

Hi Laurent,

On Fri, May 15, 2015 at 12:00:01AM +0100, Laurent Pinchart wrote:
> This patch series attempts to implement support for deferring probe of both
> IOMMU drivers and bus master drivers.

Have you had a chance to look at any of the feedback you've received on
this?

Will