[PATCH v3 0/8] dmaengine: rcar-dmac: add iommu support for slave transfers

Hi,

This series add iommu support to rcar-dmac. It's tested on koelsch with
CONFIG_IPMMU_VMSA and by enabling the ipmmu_ds node in r8a7791.dtsi. I
verified operation by interacting with /dev/mmcblk1  which is a device
behind the iommu.

The series depends on out of tree patch '[PATCH] dmaengine: use
phys_addr_t for slave configuration' which currently is under review.

* Changes since v2
- Drop patch to add dma_{map,unmap}_page_attrs.
- Add dma_{map,unmap}_resource to handle the mapping without involving a
  'struct page'. Thanks Laurent and Robin for pointing this out.
- Use size instead of address to keep track of if a mapping exist or not
  since addr == 0 is valid. Thanks Laurent.
- Pick up patch from Robin with Laurents ack (hope it's OK for me to
  attach the ack?) to add IOMMU_MMIO.
- Fix bug in rcar_dmac_device_config where the error check where
  inverted.
- Use DMA_BIDIRECTIONAL in rcar_dmac_device_config since we at that
  point can't be sure what direction the mapping is going to be used.

* Changes since v1
- Add and use a dma_{map,unmap}_page_attrs to be able to map the page
  using attributes DMA_ATTR_NO_KERNEL_MAPPING and
  DMA_ATTR_SKIP_CPU_SYNC. Thanks Laurent.
- Drop check if dmac is part of a iommu group or not, let the DMA
  mapping api handle it.
- Move slave configuration data around in rcar-dmac to avoid code
  duplication.
- Fix build issue reported by 'kbuild test robot' regarding phys_to_page
  not availability on some configurations.
- Add DT information for r8a7791.

* Changes since RFC
- Switch to use the dma-mapping api instead of using the iommu_map()
  directly. Turns out the dma-mapper is much smarter then me...
- Dropped the patch to expose domain->ops->pgsize_bitmap from within the
  iommu api.
- Dropped the patch showing how I tested the RFC.

Niklas S?derlund (7):
  dma-mapping: add {map,unmap}_resource to dma_map_ops
  dma-mapping: add dma_{map,unmap}_resource
  arm: dma-mapping: add {map,unmap}_resource for iommu ops
  dmaengine: rcar-dmac: group slave configuration
  dmaengine: rcar-dmac: add iommu support for slave transfers
  ARM: dts: r8a7790: add iommus to dmac0 and dmac1
  ARM: dts: r8a7791: add iommus to dmac0 and dmac1

Robin Murphy (1):
  iommu: Add MMIO mapping type

 arch/arm/boot/dts/r8a7790.dtsi | 30 +++++++++++++++
 arch/arm/boot/dts/r8a7791.dtsi | 30 +++++++++++++++
 arch/arm/mm/dma-mapping.c      | 63 +++++++++++++++++++++++++++++++
 drivers/dma/sh/rcar-dmac.c     | 86 +++++++++++++++++++++++++++++++++---------
 drivers/iommu/io-pgtable-arm.c |  4 +-
 include/linux/dma-mapping.h    | 33 ++++++++++++++++
 include/linux/iommu.h          |  1 +
 7 files changed, 229 insertions(+), 18 deletions(-)

--
2.7.1

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm/boot/dts/r8a7791.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 2a369dd..6dff061 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi

@@ -283,6 +283,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 0>,
+			 <&ipmmu_ds 1>,
+			 <&ipmmu_ds 2>,
+			 <&ipmmu_ds 3>,
+			 <&ipmmu_ds 4>,
+			 <&ipmmu_ds 5>,
+			 <&ipmmu_ds 6>,
+			 <&ipmmu_ds 7>,
+			 <&ipmmu_ds 8>,
+			 <&ipmmu_ds 9>,
+			 <&ipmmu_ds 10>,
+			 <&ipmmu_ds 11>,
+			 <&ipmmu_ds 12>,
+			 <&ipmmu_ds 13>,
+			 <&ipmmu_ds 14>;
 	};
 
 	dmac1: dma-controller at e6720000 {

@@ -314,6 +329,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 15>,
+			 <&ipmmu_ds 16>,
+			 <&ipmmu_ds 17>,
+			 <&ipmmu_ds 18>,
+			 <&ipmmu_ds 19>,
+			 <&ipmmu_ds 20>,
+			 <&ipmmu_ds 21>,
+			 <&ipmmu_ds 22>,
+			 <&ipmmu_ds 23>,
+			 <&ipmmu_ds 24>,
+			 <&ipmmu_ds 25>,
+			 <&ipmmu_ds 26>,
+			 <&ipmmu_ds 27>,
+			 <&ipmmu_ds 28>,
+			 <&ipmmu_ds 29>;
 	};
 
 	audma0: dma-controller at ec700000 {

-- 
2.7.1

Map/Unmap a device resource from a physical address. If no dma_map_ops
method is available the operation is a no-op.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-mapping.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e3aba4e..21bf986 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h

@@ -216,6 +216,33 @@ static inline void dma_unmap_page(struct device *dev, dma_addr_t addr,
 	debug_dma_unmap_page(dev, addr, size, dir, false);
 }
 
+static inline dma_addr_t dma_map_resource(struct device *dev,
+					  phys_addr_t phys_addr,
+					  size_t size,
+					  enum dma_data_direction dir,
+					  struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->map_resource)
+		return ops->map_resource(dev, phys_addr, size, dir, attrs);
+
+	return phys_addr;
+}
+
+static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
+				      size_t size, enum dma_data_direction dir,
+				      struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->unmap_resource)
+		ops->unmap_resource(dev, addr, size, dir, attrs);
+
+}
+
 static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
 					   size_t size,
 					   enum dma_data_direction dir)

-- 
2.7.1

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/iommu/io-pgtable-arm.c | 4 +++-
 include/linux/iommu.h          | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 381ca5a..3ff4f87 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,
 			pte |= ARM_LPAE_PTE_HAP_READ;
 		if (prot & IOMMU_WRITE)
 			pte |= ARM_LPAE_PTE_HAP_WRITE;
-		if (prot & IOMMU_CACHE)
+		if (prot & IOMMU_MMIO)
+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+		else if (prot & IOMMU_CACHE)
 			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
 		else
 			pte |= ARM_LPAE_PTE_MEMATTR_NC;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..34b6432 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h

@@ -30,6 +30,7 @@
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC	(1 << 3)
+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */
 
 struct iommu_ops;
 struct iommu_group;

-- 
2.7.1

Group slave address and transfer size in own structs for source and
destination. This is in preparation for hooking up the dma-mapping API
to the slave addresses.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/dma/sh/rcar-dmac.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 7820d07..743873c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c

@@ -118,14 +118,21 @@ struct rcar_dmac_desc_page {
 	sizeof(struct rcar_dmac_xfer_chunk))
 
 /*
+ * @slave_addr: slave memory address
+ * @xfer_size: size (in bytes) of hardware transfers
+ */
+struct rcar_dmac_chan_slave {
+	dma_addr_t slave_addr;
+	unsigned int xfer_size;
+};
+
+/*
  * struct rcar_dmac_chan - R-Car Gen2 DMA Controller Channel
  * @chan: base DMA channel object
  * @iomem: channel I/O memory base
  * @index: index of this channel in the controller
- * @src_xfer_size: size (in bytes) of hardware transfers on the source side
- * @dst_xfer_size: size (in bytes) of hardware transfers on the destination side
- * @src_slave_addr: slave source memory address
- * @dst_slave_addr: slave destination memory address
+ * @src: slave memory address and size on the source side
+ * @dst: slave memory address and size on the destination side
  * @mid_rid: hardware MID/RID for the DMA client using this channel
  * @lock: protects the channel CHCR register and the desc members
  * @desc.free: list of free descriptors

@@ -142,10 +149,8 @@ struct rcar_dmac_chan {
 	void __iomem *iomem;
 	unsigned int index;
 
-	unsigned int src_xfer_size;
-	unsigned int dst_xfer_size;
-	dma_addr_t src_slave_addr;
-	dma_addr_t dst_slave_addr;
+	struct rcar_dmac_chan_slave src;
+	struct rcar_dmac_chan_slave dst;
 	int mid_rid;
 
 	spinlock_t lock;

@@ -793,13 +798,13 @@ static void rcar_dmac_chan_configure_desc(struct rcar_dmac_chan *chan,
 	case DMA_DEV_TO_MEM:
 		chcr = RCAR_DMACHCR_DM_INC | RCAR_DMACHCR_SM_FIXED
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->src_xfer_size;
+		xfer_size = chan->src.xfer_size;
 		break;
 
 	case DMA_MEM_TO_DEV:
 		chcr = RCAR_DMACHCR_DM_FIXED | RCAR_DMACHCR_SM_INC
 		     | RCAR_DMACHCR_RS_DMARS;
-		xfer_size = chan->dst_xfer_size;
+		xfer_size = chan->dst.xfer_size;
 		break;
 
 	case DMA_MEM_TO_MEM:

@@ -1038,7 +1043,7 @@ rcar_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	return rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, false);
 }

@@ -1093,7 +1098,7 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	}
 
 	dev_addr = dir == DMA_DEV_TO_MEM
-		 ? rchan->src_slave_addr : rchan->dst_slave_addr;
+		 ? rchan->src.slave_addr : rchan->dst.slave_addr;
 	desc = rcar_dmac_chan_prep_sg(rchan, sgl, sg_len, dev_addr,
 				      dir, flags, true);
 

@@ -1110,10 +1115,10 @@ static int rcar_dmac_device_config(struct dma_chan *chan,
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src_slave_addr = cfg->src_addr;
-	rchan->dst_slave_addr = cfg->dst_addr;
-	rchan->src_xfer_size = cfg->src_addr_width;
-	rchan->dst_xfer_size = cfg->dst_addr_width;
+	rchan->src.slave_addr = cfg->src_addr;
+	rchan->dst.slave_addr = cfg->dst_addr;
+	rchan->src.xfer_size = cfg->src_addr_width;
+	rchan->dst.xfer_size = cfg->dst_addr_width;
 
 	return 0;
 }

-- 
2.7.1

Add methods to map/unmap device resources addresses for dma_map_ops that
are IOMMU aware. This is needed to map a device MMIO register from a
physical address.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm/mm/dma-mapping.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 0eca381..ae2b175 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c

@@ -1814,6 +1814,63 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 	__free_iova(mapping, iova, len);
 }
 
+/**
+ * arm_iommu_map_resource - map a device resource for DMA
+ * @dev: valid struct device pointer
+ * @phys_addr: physical address of resource
+ * @size: size of resource to map
+ * @dir: DMA transfer direction
+ */
+static dma_addr_t arm_iommu_map_resource(struct device *dev,
+		phys_addr_t phys_addr, size_t size,
+		enum dma_data_direction dir, struct dma_attrs *attrs)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+	dma_addr_t dma_addr;
+	int ret, prot;
+	phys_addr_t addr = phys_addr & PAGE_MASK;
+	int offset = phys_addr & ~PAGE_MASK;
+	int len = PAGE_ALIGN(size + offset);
+
+	dma_addr = __alloc_iova(mapping, size);
+	if (dma_addr == DMA_ERROR_CODE)
+		return dma_addr;
+
+	prot = __dma_direction_to_prot(dir) | IOMMU_MMIO;
+
+	ret = iommu_map(mapping->domain, dma_addr, addr, len, prot);
+	if (ret < 0)
+		goto fail;
+
+	return dma_addr + offset;
+fail:
+	__free_iova(mapping, dma_addr, size);
+	return DMA_ERROR_CODE;
+}
+
+/**
+ * arm_iommu_unmap_resource - unmap a device DMA resource
+ * @dev: valid struct device pointer
+ * @dma_handle: DMA address to resource
+ * @size: size of resource to map
+ * @dir: DMA transfer direction
+ */
+static void arm_iommu_unmap_resource(struct device *dev, dma_addr_t dma_handle,
+		size_t size, enum dma_data_direction dir,
+		struct dma_attrs *attrs)
+{
+	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+	dma_addr_t iova = dma_handle & PAGE_MASK;
+	int offset = dma_handle & ~PAGE_MASK;
+	int len = PAGE_ALIGN(size + offset);
+
+	if (!iova)
+		return;
+
+	iommu_unmap(mapping->domain, iova, len);
+	__free_iova(mapping, iova, len);
+}
+
 static void arm_iommu_sync_single_for_cpu(struct device *dev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir)
 {

@@ -1858,6 +1915,9 @@ struct dma_map_ops iommu_ops = {
 	.sync_sg_for_cpu	= arm_iommu_sync_sg_for_cpu,
 	.sync_sg_for_device	= arm_iommu_sync_sg_for_device,
 
+	.map_resource		= arm_iommu_map_resource,
+	.unmap_resource		= arm_iommu_unmap_resource,
+
 	.set_dma_mask		= arm_dma_set_mask,
 };
 

@@ -1873,6 +1933,9 @@ struct dma_map_ops iommu_coherent_ops = {
 	.map_sg		= arm_coherent_iommu_map_sg,
 	.unmap_sg	= arm_coherent_iommu_unmap_sg,
 
+	.map_resource	= arm_iommu_map_resource,
+	.unmap_resource	= arm_iommu_unmap_resource,
+
 	.set_dma_mask	= arm_dma_set_mask,
 };
 

-- 
2.7.1

Add methods to handle mapping of device resources from a physical
address. This is needed for example to map be able to map MMIO FIFO
registers to a IOMMU.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-mapping.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 75857cd..e3aba4e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h

@@ -49,6 +49,12 @@ struct dma_map_ops {
 			 struct scatterlist *sg, int nents,
 			 enum dma_data_direction dir,
 			 struct dma_attrs *attrs);
+	dma_addr_t (*map_resource)(struct device *dev, phys_addr_t phys_addr,
+			       size_t size, enum dma_data_direction dir,
+			       struct dma_attrs *attrs);
+	void (*unmap_resource)(struct device *dev, dma_addr_t dma_handle,
+			   size_t size, enum dma_data_direction dir,
+			   struct dma_attrs *attrs);
 	void (*sync_single_for_cpu)(struct device *dev,
 				    dma_addr_t dma_handle, size_t size,
 				    enum dma_data_direction dir);

-- 
2.7.1

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 arch/arm/boot/dts/r8a7790.dtsi | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 7dfd393..048bbf8 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi

@@ -294,6 +294,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 0>,
+			 <&ipmmu_ds 1>,
+			 <&ipmmu_ds 2>,
+			 <&ipmmu_ds 3>,
+			 <&ipmmu_ds 4>,
+			 <&ipmmu_ds 5>,
+			 <&ipmmu_ds 6>,
+			 <&ipmmu_ds 7>,
+			 <&ipmmu_ds 8>,
+			 <&ipmmu_ds 9>,
+			 <&ipmmu_ds 10>,
+			 <&ipmmu_ds 11>,
+			 <&ipmmu_ds 12>,
+			 <&ipmmu_ds 13>,
+			 <&ipmmu_ds 14>;
 	};
 
 	dmac1: dma-controller at e6720000 {

@@ -325,6 +340,21 @@
 		power-domains = <&cpg_clocks>;
 		#dma-cells = <1>;
 		dma-channels = <15>;
+		iommus = <&ipmmu_ds 15>,
+			 <&ipmmu_ds 16>,
+			 <&ipmmu_ds 17>,
+			 <&ipmmu_ds 18>,
+			 <&ipmmu_ds 19>,
+			 <&ipmmu_ds 20>,
+			 <&ipmmu_ds 21>,
+			 <&ipmmu_ds 22>,
+			 <&ipmmu_ds 23>,
+			 <&ipmmu_ds 24>,
+			 <&ipmmu_ds 25>,
+			 <&ipmmu_ds 26>,
+			 <&ipmmu_ds 27>,
+			 <&ipmmu_ds 28>,
+			 <&ipmmu_ds 29>;
 	};
 
 	audma0: dma-controller at ec700000 {

-- 
2.7.1

Enable slave transfers to devices behind IPMMU:s by mapping the slave
addresses using the dma-mapping API.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/dma/sh/rcar-dmac.c | 57 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 743873c..268407c 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c

@@ -1106,21 +1106,68 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr,
 	return desc;
 }
 
+static int rcar_dmac_set_slave_addr(struct dma_chan *chan,
+				     struct rcar_dmac_chan_slave *slave,
+				     phys_addr_t addr, size_t size)
+{
+	struct dma_attrs attrs;
+	enum dma_data_direction dir;
+
+	init_dma_attrs(&attrs);
+	dma_set_attr(DMA_ATTR_NO_KERNEL_MAPPING, &attrs);
+	dma_set_attr(DMA_ATTR_SKIP_CPU_SYNC, &attrs);
+
+	/*
+	 * We can't know the direction at this time, see documentation for
+	 * 'direction' in struct dma_slave_config.
+	 */
+	dir = DMA_BIDIRECTIONAL;
+
+	if (slave->xfer_size) {
+		dma_unmap_resource(chan->device->dev, slave->slave_addr,
+				slave->xfer_size, dir, &attrs);
+		slave->slave_addr = 0;
+		slave->xfer_size = 0;
+	}
+
+	if (size) {
+		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
+				size, dir, &attrs);
+
+		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
+			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+			dev_err(chan->device->dev,
+					"chan%u: failed to map %zx@%pap",
+					rchan->index, size, &addr);
+			return -EIO;
+		}
+
+		slave->xfer_size = size;
+	}
+
+	return 0;
+}
+
 static int rcar_dmac_device_config(struct dma_chan *chan,
 				   struct dma_slave_config *cfg)
 {
 	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+	int ret;
 
 	/*
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src.slave_addr = cfg->src_addr;
-	rchan->dst.slave_addr = cfg->dst_addr;
-	rchan->src.xfer_size = cfg->src_addr_width;
-	rchan->dst.xfer_size = cfg->dst_addr_width;
 
-	return 0;
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
+			cfg->src_addr_width);
+	if (ret)
+		return ret;
+
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
+			cfg->dst_addr_width);
+	return ret;
 }
 
 static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)

-- 
2.7.1

Hi Niklas,

Thanks for doing this, it looks good. Just a couple of minor comments on 
this and the next patch...

On 10/02/16 00:57, Niklas S?derlund wrote:

I think it would be worth also having the same inverse pfn_valid() check 
as ioremap() here, to make sure this is similarly hard to misuse.

Robin.

+	if (ops->map_resource)
+		return ops->map_resource(dev, phys_addr, size, dir, attrs);
+
+	return phys_addr;
+}
+
+static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
+				      size_t size, enum dma_data_direction dir,
+				      struct dma_attrs *attrs)
+{
+	struct dma_map_ops *ops = get_dma_ops(dev);
+
+	BUG_ON(!valid_dma_direction(dir));
+	if (ops->unmap_resource)
+		ops->unmap_resource(dev, addr, size, dir, attrs);
+
+}
+
  static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
  					   size_t size,
  					   enum dma_data_direction dir)

On 10/02/16 00:57, Niklas S?derlund wrote:

Now that we have a way to deal with MMIO addresses properly, we don't 
need these any more.

Robin.

+
+	/*
+	 * We can't know the direction at this time, see documentation for
+	 * 'direction' in struct dma_slave_config.
+	 */
+	dir = DMA_BIDIRECTIONAL;
+
+	if (slave->xfer_size) {
+		dma_unmap_resource(chan->device->dev, slave->slave_addr,
+				slave->xfer_size, dir, &attrs);
+		slave->slave_addr = 0;
+		slave->xfer_size = 0;
+	}
+
+	if (size) {
+		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
+				size, dir, &attrs);
+
+		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
+			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+			dev_err(chan->device->dev,
+					"chan%u: failed to map %zx@%pap",
+					rchan->index, size, &addr);
+			return -EIO;
+		}
+
+		slave->xfer_size = size;
+	}
+
+	return 0;
+}
+
  static int rcar_dmac_device_config(struct dma_chan *chan,
  				   struct dma_slave_config *cfg)
  {
  	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+	int ret;

  	/*
  	 * We could lock this, but you shouldn't be configuring the
  	 * channel, while using it...
  	 */
-	rchan->src.slave_addr = cfg->src_addr;
-	rchan->dst.slave_addr = cfg->dst_addr;
-	rchan->src.xfer_size = cfg->src_addr_width;
-	rchan->dst.xfer_size = cfg->dst_addr_width;

-	return 0;
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
+			cfg->src_addr_width);
+	if (ret)
+		return ret;
+
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
+			cfg->dst_addr_width);
+	return ret;
  }

  static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)

Hello.

On 2/10/2016 3:57 AM, Niklas S?derlund wrote:

Add methods to handle mapping of device resources from a physical
address. This is needed for example to map be able to map MMIO FIFO

                                          ^^^ not needed

registers to a IOMMU.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

[...]

MBR, Sergei

Hi Niklas,

I am deferring accepting this and the similar patch for the r8a7791 pending
acceptance of the driver changes earlier in this series. Please let me know
if you prefer a different course of action.

I notice that the devel branch of there renesas tree there are
dmac nodes for the r8a7793, r8a7794 and r8a7795. Is this change,
also suitable for those SoCs? If so, do you plan to update them?
If not I'll add it to my todo list.

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:51 Niklas S?derlund wrote:

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Answering the question from the cover letter, yes, it's totally fine to pick 
the ack, that's actually expected.

---
 drivers/iommu/io-pgtable-arm.c | 4 +++-
 include/linux/iommu.h          | 1 +

You might be asked to split this patch in two.

-- 
Regards,

Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:52 Niklas S?derlund wrote:

Add methods to handle mapping of device resources from a physical
address. This is needed for example to map be able to map MMIO FIFO
registers to a IOMMU.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

Apart from the typo in the commit message that Sergei already pointed out,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:54 Niklas S?derlund wrote:

Add methods to map/unmap device resources addresses for dma_map_ops that
are IOMMU aware. This is needed to map a device MMIO register from a
physical address.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:55 Niklas S?derlund wrote:

Group slave address and transfer size in own structs for source and
destination. This is in preparation for hooking up the dma-mapping API
to the slave addresses.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:56 Niklas S?derlund wrote:

Nitpicking, you can align slave with chan on the previous line.

+		slave->slave_addr = 0;
+		slave->xfer_size = 0;
+	}
+
+	if (size) {
+		slave->slave_addr = dma_map_resource(chan->device->dev, addr,
+				size, dir, &attrs);
+
+		if (dma_mapping_error(chan->device->dev, slave->slave_addr)) {
+			struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+
+			dev_err(chan->device->dev,
+					"chan%u: failed to map %zx@%pap",
+					rchan->index, size, &addr);

Indentation looks weird to me.

+			return -EIO;
+		}
+
+		slave->xfer_size = size;
+	}
+
+	return 0;
+}
+
 static int rcar_dmac_device_config(struct dma_chan *chan,
 				   struct dma_slave_config *cfg)
 {
 	struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
+	int ret;

 	/*
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...
 	 */
-	rchan->src.slave_addr = cfg->src_addr;
-	rchan->dst.slave_addr = cfg->dst_addr;
-	rchan->src.xfer_size = cfg->src_addr_width;
-	rchan->dst.xfer_size = cfg->dst_addr_width;

-	return 0;
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->src, cfg->src_addr,
+			cfg->src_addr_width);
+	if (ret)
+		return ret;
+
+	ret = rcar_dmac_set_slave_addr(chan, &rchan->dst, cfg->dst_addr,
+			cfg->dst_addr_width);

You could align cfg with chan on the previous line (twice).

With this fixed and the attributes removed as explained by Robin,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

+	return ret;
 }

 static int rcar_dmac_chan_terminate_all(struct dma_chan *chan)

-- 
Regards,

Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:57 Niklas S?derlund wrote:

No commit message ? I'd at least mention that as a side effect of this patch 
channel 0 and 15 are disabled, reducing the effective number of channels to 14 
per DMAC.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Same comment and ack for patch 8/8.

Note that we should still try to find a way to selectively enable the IOMMU in 
a per-device fashion, as system integrators might want it to be disabled for 
some devices. There's no urgency though.

-- 
Regards,

Laurent Pinchart

Hi Simon,

* Simon Horman [off-list ref] [2016-02-10 18:55:59 +0100]:

Hi Niklas,

I am deferring accepting this and the similar patch for the r8a7791 pending
acceptance of the driver changes earlier in this series. Please let me know
if you prefer a different course of action.

That sounds good, thanks.

I notice that the devel branch of there renesas tree there are
dmac nodes for the r8a7793, r8a7794 and r8a7795. Is this change,
also suitable for those SoCs? If so, do you plan to update them?
If not I'll add it to my todo list.

I planed to update all effected SoCs once the dependencies for this
series where accepted. But if you want to keep track of this I'm happy.

On 11/02/16 00:02, Laurent Pinchart wrote:

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:51 Niklas S?derlund wrote:

quoted

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Answering the question from the cover letter, yes, it's totally fine to pick
the ack, that's actually expected.

quoted

---
  drivers/iommu/io-pgtable-arm.c | 4 +++-
  include/linux/iommu.h          | 1 +

You might be asked to split this patch in two.

Worse than that, you might also be asked to fix it up when the silly 
author remembers that he did this on a stage-2-only ARM SMMU, and the 
attributes for the stage 1 tables that the IPMMU uses are in a different 
code path:

--->8---

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 5b5c299..7622c6e 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct 

arm_lpae_io_pgtable *data,
                 if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
                         pte |= ARM_LPAE_PTE_AP_RDONLY;

-               if (prot & IOMMU_CACHE)
+               if (prot & IOMMU_MMIO)
+                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
+                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
+               else if (prot & IOMMU_CACHE)
                         pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
                                 << ARM_LPAE_PTE_ATTRINDX_SHIFT);
         } else {
--->8---

Sorry for the bother,
Robin.

quoted

  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 381ca5a..3ff4f87 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct

arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
  		if (prot & IOMMU_WRITE)
  			pte |= ARM_LPAE_PTE_HAP_WRITE;
-		if (prot & IOMMU_CACHE)
+		if (prot & IOMMU_MMIO)
+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+		else if (prot & IOMMU_CACHE)
  			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
  		else
  			pte |= ARM_LPAE_PTE_MEMATTR_NC;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..34b6432 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h

@@ -30,6 +30,7 @@
  #define IOMMU_WRITE	(1 << 1)
  #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
  #define IOMMU_NOEXEC	(1 << 3)
+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */

  struct iommu_ops;
  struct iommu_group;

Hi Robin,

Thanks for your update patch I will include it in my next version. But
I'm sorry I do not understand, is your modification an addition or a
substitution to your original patch?

* Robin Murphy [off-list ref] [2016-02-11 15:57:26 +0000]:

On 16/02/16 12:06, Niklas S?derlund wrote:

Hi Robin,

Thanks for your update patch I will include it in my next version. But
I'm sorry I do not understand, is your modification an addition or a
substitution to your original patch?

Apologies for being confusing - that was a diff on top of the existing 
patch, to be folded in. My original patch was only handling IOMMU_MMIO 
for stage 2 PTEs, so we also need the extra code to handle the different 
way of setting the appropriate memory type in stage 1 PTEs.

Robin.

* Robin Murphy [off-list ref] [2016-02-11 15:57:26 +0000]:

quoted

On 11/02/16 00:02, Laurent Pinchart wrote:

quoted

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:51 Niklas S?derlund wrote:

quoted

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Answering the question from the cover letter, yes, it's totally fine to pick
the ack, that's actually expected.

quoted

---
  drivers/iommu/io-pgtable-arm.c | 4 +++-
  include/linux/iommu.h          | 1 +

You might be asked to split this patch in two.

Worse than that, you might also be asked to fix it up when the silly author
remembers that he did this on a stage-2-only ARM SMMU, and the attributes
for the stage 1 tables that the IPMMU uses are in a different code path:

--->8---

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 5b5c299..7622c6e 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct

arm_lpae_io_pgtable *data,
                 if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
                         pte |= ARM_LPAE_PTE_AP_RDONLY;

-               if (prot & IOMMU_CACHE)
+               if (prot & IOMMU_MMIO)
+                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
+                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
+               else if (prot & IOMMU_CACHE)
                         pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
                                 << ARM_LPAE_PTE_ATTRINDX_SHIFT);
         } else {
--->8---

Sorry for the bother,
Robin.

quoted

quoted

  2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 381ca5a..3ff4f87 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct

arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
  		if (prot & IOMMU_WRITE)
  			pte |= ARM_LPAE_PTE_HAP_WRITE;
-		if (prot & IOMMU_CACHE)
+		if (prot & IOMMU_MMIO)
+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+		else if (prot & IOMMU_CACHE)
  			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
  		else
  			pte |= ARM_LPAE_PTE_MEMATTR_NC;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..34b6432 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h

@@ -30,6 +30,7 @@
  #define IOMMU_WRITE	(1 << 1)
  #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
  #define IOMMU_NOEXEC	(1 << 3)
+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */

  struct iommu_ops;
  struct iommu_group;

* Robin Murphy [off-list ref] [2016-02-16 12:43:40 +0000]:

On 16/02/16 12:06, Niklas S?derlund wrote:

quoted

Hi Robin,

Thanks for your update patch I will include it in my next version. But
I'm sorry I do not understand, is your modification an addition or a
substitution to your original patch?

Apologies for being confusing - that was a diff on top of the existing
patch, to be folded in. My original patch was only handling IOMMU_MMIO for
stage 2 PTEs, so we also need the extra code to handle the different way of
setting the appropriate memory type in stage 1 PTEs.

That's what I though but wanted to be clear, thanks for clarifying. I
will fold the diff into your patch and keep your SoB line and send it
out with my series, hope that's a OK way for me to handle it.

Once more thanks for your patch and feedback.

Robin.

quoted

* Robin Murphy [off-list ref] [2016-02-11 15:57:26 +0000]:

quoted

On 11/02/16 00:02, Laurent Pinchart wrote:

quoted

Hi Niklas,

Thank you for the patch.

On Wednesday 10 February 2016 01:57:51 Niklas S?derlund wrote:

quoted

From: Robin Murphy <robin.murphy@arm.com>

On some platforms, MMIO regions might need slightly different treatment
compared to mapping regular memory; add the notion of MMIO mappings to
the IOMMU API's memory type flags, so that callers can let the IOMMU
drivers know to do the right thing.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Answering the question from the cover letter, yes, it's totally fine to pick
the ack, that's actually expected.

quoted

---
 drivers/iommu/io-pgtable-arm.c | 4 +++-
 include/linux/iommu.h          | 1 +

You might be asked to split this patch in two.

Worse than that, you might also be asked to fix it up when the silly author
remembers that he did this on a stage-2-only ARM SMMU, and the attributes
for the stage 1 tables that the IPMMU uses are in a different code path:

--->8---

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 5b5c299..7622c6e 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -354,7 +354,10 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct

arm_lpae_io_pgtable *data,
                if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
                        pte |= ARM_LPAE_PTE_AP_RDONLY;

-               if (prot & IOMMU_CACHE)
+               if (prot & IOMMU_MMIO)
+                       pte |= (ARM_LPAE_MAIR_ATTR_IDX_DEV
+                               << ARM_LPAE_PTE_ATTRINDX_SHIFT);
+               else if (prot & IOMMU_CACHE)
                        pte |= (ARM_LPAE_MAIR_ATTR_IDX_CACHE
                                << ARM_LPAE_PTE_ATTRINDX_SHIFT);
        } else {
--->8---

Sorry for the bother,
Robin.

quoted

quoted

 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 381ca5a..3ff4f87 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c

@@ -364,7 +364,9 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct

arm_lpae_io_pgtable *data, pte |= ARM_LPAE_PTE_HAP_READ;
 		if (prot & IOMMU_WRITE)
 			pte |= ARM_LPAE_PTE_HAP_WRITE;
-		if (prot & IOMMU_CACHE)
+		if (prot & IOMMU_MMIO)
+			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
+		else if (prot & IOMMU_CACHE)
 			pte |= ARM_LPAE_PTE_MEMATTR_OIWB;
 		else
 			pte |= ARM_LPAE_PTE_MEMATTR_NC;

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a5c539f..34b6432 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h

@@ -30,6 +30,7 @@
 #define IOMMU_WRITE	(1 << 1)
 #define IOMMU_CACHE	(1 << 2) /* DMA cache coherency */
 #define IOMMU_NOEXEC	(1 << 3)
+#define IOMMU_MMIO	(1 << 4) /* e.g. things like MSI doorbells */

 struct iommu_ops;
 struct iommu_group;