[PATCH 0/5] Renesas VMSA-compatible IPMMU DT support

[PATCH 0/5] Renesas VMSA-compatible IPMMU DT support

[Laurent Pinchart]

Hello,

This patch set adds DT support to the Renesas VMSA-compatible IPMMU driver.
The first patch refactors the driver to remove dependencies on platform data,
the second patch adds the DT bindings documentation and the third patch
implement them in the driver.

The next two patches show real-life examples for IPMMU DT bindings usage.
Patch 4/5 adds IPMMU DT nodes to the r8a7791 SoC dtsi file, and patch 5/5
enables IOMMU support for the VSP1 on the same platform.

The patches are based on top of the IPMMU driver previously submitted to the
iommu mailing list. The last patch additionally requires to VSP1 DT support
patch series previously submitted to the linux-media mailing list and is thus
a test patch only at the moment, not meant to be merged yet.

The DT bindings are pretty simple, with only three standard properties in the
IPMMU DT node. The device to IOMMU association is represented in the bus master
device nodes using an iommus property, similarly to interrupts or clocks. This
model departs from the ARM SMMU DT bindings that represent the same information
inside the IOMMU DT node. I'm not sure what's best, so I'm open to suggestions.

Every IPMMU instance serves multiple bus master devices and implements four
independent page tables and TLBs. Each bus master can be freely assigned to one
of the page tables, lowering the risk of TLB miss when multiple bus masters
require address translation at the same time. I've decided not to express the
bus master to page table association in DT as this seems to me to be a software
configuration decision, not a hardware property. Feel free to disagree.

As a side note, there's no API at the moment to configure that association. My
plan is to assign bus masters to page tables automatically in the driver in a
round-robin way. A more configurable solution might be needed later, but I
believe that's out of scope of DT.

Cc: devicetree@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>

Laurent Pinchart (5):
  iommu/ipmmu-vmsa: Refactor micro-TLB lookup
  iommu/ipmmu-vmsa: Add device tree bindings documentation
  iommu/ipmmu-vmsa: Add device tree support
  ARM: shmobile: r8a7791: Add IPMMU DT nodes
  [TEST] ARM: shmobile: r8a7791: Enable IOMMU support for the VSP1

 .../bindings/iommu/renesas,ipmmu-vmsa.txt          |  35 +++++++
 arch/arm/boot/dts/r8a7791.dtsi                     |  54 ++++++++++
 drivers/iommu/ipmmu-vmsa.c                         | 115 +++++++++++++--------
 3 files changed, 163 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

-- 
Regards,

Laurent Pinchart

[PATCH 0/5] Renesas VMSA-compatible IPMMU DT support

[Laurent Pinchart]

Hello,

This patch set adds DT support to the Renesas VMSA-compatible IPMMU driver.
The first patch refactors the driver to remove dependencies on platform data,
the second patch adds the DT bindings documentation and the third patch
implement them in the driver.

The next two patches show real-life examples for IPMMU DT bindings usage.
Patch 4/5 adds IPMMU DT nodes to the r8a7791 SoC dtsi file, and patch 5/5
enables IOMMU support for the VSP1 on the same platform.

The patches are based on top of the IPMMU driver previously submitted to the
iommu mailing list. The last patch additionally requires to VSP1 DT support
patch series previously submitted to the linux-media mailing list and is thus
a test patch only at the moment, not meant to be merged yet.

The DT bindings are pretty simple, with only three standard properties in the
IPMMU DT node. The device to IOMMU association is represented in the bus master
device nodes using an iommus property, similarly to interrupts or clocks. This
model departs from the ARM SMMU DT bindings that represent the same information
inside the IOMMU DT node. I'm not sure what's best, so I'm open to suggestions.

Every IPMMU instance serves multiple bus master devices and implements four
independent page tables and TLBs. Each bus master can be freely assigned to one
of the page tables, lowering the risk of TLB miss when multiple bus masters
require address translation at the same time. I've decided not to express the
bus master to page table association in DT as this seems to me to be a software
configuration decision, not a hardware property. Feel free to disagree.

As a side note, there's no API at the moment to configure that association. My
plan is to assign bus masters to page tables automatically in the driver in a
round-robin way. A more configurable solution might be needed later, but I
believe that's out of scope of DT.

Cc: devicetree@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>

Laurent Pinchart (5):
  iommu/ipmmu-vmsa: Refactor micro-TLB lookup
  iommu/ipmmu-vmsa: Add device tree bindings documentation
  iommu/ipmmu-vmsa: Add device tree support
  ARM: shmobile: r8a7791: Add IPMMU DT nodes
  [TEST] ARM: shmobile: r8a7791: Enable IOMMU support for the VSP1

 .../bindings/iommu/renesas,ipmmu-vmsa.txt          |  35 +++++++
 arch/arm/boot/dts/r8a7791.dtsi                     |  54 ++++++++++
 drivers/iommu/ipmmu-vmsa.c                         | 115 +++++++++++++--------
 3 files changed, 163 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

-- 
Regards,

Laurent Pinchart

[PATCH 1/5] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Laurent Pinchart]

Cache the micro-TLB number in archdata allocated in the .add_device
handler instead of looking it up when the deviced is attached and
detached. This simplifies the .attach_dev and .detach_dev operations and
prepares for DT support.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 7abadc6..ef03191 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -44,6 +44,11 @@ struct ipmmu_vmsa_domain {
 	pgd_t *pgd;
 };
 
+struct ipmmu_vmsa_archdata {
+	struct ipmmu_vmsa_device *mmu;
+	unsigned int utlb;
+};
+
 static DEFINE_SPINLOCK(ipmmu_devices_lock);
 static LIST_HEAD(ipmmu_devices);
 
@@ -265,14 +270,19 @@ static void ipmmu_tlb_invalidate(struct ipmmu_vmsa_domain *domain)
  * Enable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
-			      const struct ipmmu_vmsa_master *master)
+			      unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
+	/*
+	 * TODO: Reference-count the microTLB as several bus masters can be
+	 * connected to the same microTLB.
+	 */
+
 	/* TODO: What should we set the ASID to ? */
-	ipmmu_write(mmu, IMUASID(master->utlb), 0);
+	ipmmu_write(mmu, IMUASID(utlb), 0);
 	/* TODO: Do we need to flush the microTLB ? */
-	ipmmu_write(mmu, IMUCTR(master->utlb),
+	ipmmu_write(mmu, IMUCTR(utlb),
 		    IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
 		    IMUCTR_MMUEN);
 }
@@ -281,11 +291,11 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
  * Disable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
-			       const struct ipmmu_vmsa_master *master)
+			       unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
-	ipmmu_write(mmu, IMUCTR(master->utlb), 0);
+	ipmmu_write(mmu, IMUCTR(utlb), 0);
 }
 
 static void ipmmu_flush_pgtable(struct ipmmu_vmsa_device *mmu, void *addr,
@@ -674,21 +684,6 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
  * IOMMU Operations
  */
 
-static const struct ipmmu_vmsa_master *
-ipmmu_find_master(struct ipmmu_vmsa_device *ipmmu, struct device *dev)
-{
-	const struct ipmmu_vmsa_master *master = ipmmu->pdata->masters;
-	const char *devname = dev_name(dev);
-	unsigned int i;
-
-	for (i = 0; i < ipmmu->pdata->num_masters; ++i, ++master) {
-		if (strcmp(master->name, devname) = 0)
-			return master;
-	}
-
-	return NULL;
-}
-
 static int ipmmu_domain_init(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain;
@@ -727,9 +722,9 @@ static void ipmmu_domain_destroy(struct iommu_domain *io_domain)
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_device *mmu = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_device *mmu = archdata->mmu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 	unsigned long flags;
 	int ret = 0;
 
@@ -759,11 +754,7 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	if (ret < 0)
 		return ret;
 
-	master = ipmmu_find_master(mmu, dev);
-	if (!master)
-		return -EINVAL;
-
-	ipmmu_utlb_enable(domain, master);
+	ipmmu_utlb_enable(domain, archdata->utlb);
 
 	return 0;
 }
@@ -771,14 +762,10 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
 				struct device *dev)
 {
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 
-	master = ipmmu_find_master(domain->mmu, dev);
-	if (!master)
-		return;
-
-	ipmmu_utlb_disable(domain, master);
+	ipmmu_utlb_disable(domain, archdata->utlb);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -839,11 +826,26 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 	return __pfn_to_phys(pte_pfn(pte)) | (iova & ~PAGE_MASK);
 }
 
+static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
+{
+	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
+	const char *devname = dev_name(dev);
+	unsigned int i;
+
+	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
+		if (strcmp(master->name, devname) = 0)
+			return master->utlb;
+	}
+
+	return -1;
+}
+
 static int ipmmu_add_device(struct device *dev)
 {
-	const struct ipmmu_vmsa_master *master = NULL;
+	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
 	struct iommu_group *group;
+	int utlb = -1;
 	int ret;
 
 	if (dev->archdata.iommu) {
@@ -856,10 +858,10 @@ static int ipmmu_add_device(struct device *dev)
 	spin_lock(&ipmmu_devices_lock);
 
 	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		master = ipmmu_find_master(mmu, dev);
-		if (master) {
+		utlb = ipmmu_find_utlb(mmu, dev);
+		if (utlb >= 0) {
 			/*
-			 * TODO Take a reference to the master to protect
+			 * TODO Take a reference to the MMU to protect
 			 * against device removal.
 			 */
 			break;
@@ -868,10 +870,10 @@ static int ipmmu_add_device(struct device *dev)
 
 	spin_unlock(&ipmmu_devices_lock);
 
-	if (!master)
+	if (utlb < 0)
 		return -ENODEV;
 
-	if (!master->utlb >= mmu->num_utlbs)
+	if (utlb >= mmu->num_utlbs)
 		return -EINVAL;
 
 	/* Create a device group and add the device to it. */
@@ -889,7 +891,15 @@ static int ipmmu_add_device(struct device *dev)
 		return ret;
 	}
 
-	dev->archdata.iommu = mmu;
+	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+	if (!archdata) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	archdata->mmu = mmu;
+	archdata->utlb = utlb;
+	dev->archdata.iommu = archdata;
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -923,6 +933,7 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
 	return ret;
@@ -932,6 +943,7 @@ static void ipmmu_remove_device(struct device *dev)
 {
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 }
 
-- 
1.8.3.2

[PATCH 1/5] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Laurent Pinchart]

Cache the micro-TLB number in archdata allocated in the .add_device
handler instead of looking it up when the deviced is attached and
detached. This simplifies the .attach_dev and .detach_dev operations and
prepares for DT support.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 drivers/iommu/ipmmu-vmsa.c | 92 ++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 7abadc6..ef03191 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -44,6 +44,11 @@ struct ipmmu_vmsa_domain {
 	pgd_t *pgd;
 };
 
+struct ipmmu_vmsa_archdata {
+	struct ipmmu_vmsa_device *mmu;
+	unsigned int utlb;
+};
+
 static DEFINE_SPINLOCK(ipmmu_devices_lock);
 static LIST_HEAD(ipmmu_devices);
 
@@ -265,14 +270,19 @@ static void ipmmu_tlb_invalidate(struct ipmmu_vmsa_domain *domain)
  * Enable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
-			      const struct ipmmu_vmsa_master *master)
+			      unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
+	/*
+	 * TODO: Reference-count the microTLB as several bus masters can be
+	 * connected to the same microTLB.
+	 */
+
 	/* TODO: What should we set the ASID to ? */
-	ipmmu_write(mmu, IMUASID(master->utlb), 0);
+	ipmmu_write(mmu, IMUASID(utlb), 0);
 	/* TODO: Do we need to flush the microTLB ? */
-	ipmmu_write(mmu, IMUCTR(master->utlb),
+	ipmmu_write(mmu, IMUCTR(utlb),
 		    IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
 		    IMUCTR_MMUEN);
 }
@@ -281,11 +291,11 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
  * Disable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
-			       const struct ipmmu_vmsa_master *master)
+			       unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
-	ipmmu_write(mmu, IMUCTR(master->utlb), 0);
+	ipmmu_write(mmu, IMUCTR(utlb), 0);
 }
 
 static void ipmmu_flush_pgtable(struct ipmmu_vmsa_device *mmu, void *addr,
@@ -674,21 +684,6 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
  * IOMMU Operations
  */
 
-static const struct ipmmu_vmsa_master *
-ipmmu_find_master(struct ipmmu_vmsa_device *ipmmu, struct device *dev)
-{
-	const struct ipmmu_vmsa_master *master = ipmmu->pdata->masters;
-	const char *devname = dev_name(dev);
-	unsigned int i;
-
-	for (i = 0; i < ipmmu->pdata->num_masters; ++i, ++master) {
-		if (strcmp(master->name, devname) == 0)
-			return master;
-	}
-
-	return NULL;
-}
-
 static int ipmmu_domain_init(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain;
@@ -727,9 +722,9 @@ static void ipmmu_domain_destroy(struct iommu_domain *io_domain)
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_device *mmu = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_device *mmu = archdata->mmu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 	unsigned long flags;
 	int ret = 0;
 
@@ -759,11 +754,7 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	if (ret < 0)
 		return ret;
 
-	master = ipmmu_find_master(mmu, dev);
-	if (!master)
-		return -EINVAL;
-
-	ipmmu_utlb_enable(domain, master);
+	ipmmu_utlb_enable(domain, archdata->utlb);
 
 	return 0;
 }
@@ -771,14 +762,10 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
 				struct device *dev)
 {
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 
-	master = ipmmu_find_master(domain->mmu, dev);
-	if (!master)
-		return;
-
-	ipmmu_utlb_disable(domain, master);
+	ipmmu_utlb_disable(domain, archdata->utlb);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -839,11 +826,26 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 	return __pfn_to_phys(pte_pfn(pte)) | (iova & ~PAGE_MASK);
 }
 
+static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
+{
+	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
+	const char *devname = dev_name(dev);
+	unsigned int i;
+
+	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
+		if (strcmp(master->name, devname) == 0)
+			return master->utlb;
+	}
+
+	return -1;
+}
+
 static int ipmmu_add_device(struct device *dev)
 {
-	const struct ipmmu_vmsa_master *master = NULL;
+	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
 	struct iommu_group *group;
+	int utlb = -1;
 	int ret;
 
 	if (dev->archdata.iommu) {
@@ -856,10 +858,10 @@ static int ipmmu_add_device(struct device *dev)
 	spin_lock(&ipmmu_devices_lock);
 
 	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		master = ipmmu_find_master(mmu, dev);
-		if (master) {
+		utlb = ipmmu_find_utlb(mmu, dev);
+		if (utlb >= 0) {
 			/*
-			 * TODO Take a reference to the master to protect
+			 * TODO Take a reference to the MMU to protect
 			 * against device removal.
 			 */
 			break;
@@ -868,10 +870,10 @@ static int ipmmu_add_device(struct device *dev)
 
 	spin_unlock(&ipmmu_devices_lock);
 
-	if (!master)
+	if (utlb < 0)
 		return -ENODEV;
 
-	if (!master->utlb >= mmu->num_utlbs)
+	if (utlb >= mmu->num_utlbs)
 		return -EINVAL;
 
 	/* Create a device group and add the device to it. */
@@ -889,7 +891,15 @@ static int ipmmu_add_device(struct device *dev)
 		return ret;
 	}
 
-	dev->archdata.iommu = mmu;
+	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+	if (!archdata) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	archdata->mmu = mmu;
+	archdata->utlb = utlb;
+	dev->archdata.iommu = archdata;
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -923,6 +933,7 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
 	return ret;
@@ -932,6 +943,7 @@ static void ipmmu_remove_device(struct device *dev)
 {
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 }
 
-- 
1.8.3.2

[PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Cc: devicetree@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
new file mode 100644
index 0000000..72e8590
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
@@ -0,0 +1,35 @@
+* Renesas VMSA-Compatible IOMMU
+
+The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
+It provides address translation bus masters outside of the CPU, each connected
+to the IPMMU through a port called micro-TLB.
+
+
+Required Properties:
+
+  - compatible: Must contain "renesas,ipmmu-vmsa".
+  - reg: Base address and size of the IPMMU registers.
+  - interrupts: Specifier for the MMU fault interrupt.
+
+
+Each bus master connected to an IPMMU must reference the IPMMU in its device
+node with the following property:
+
+  - iommus: A reference to the IPMMU in two cells. The first cell is a phandle
+    to the IPMMU and the second cell the number of the micro-TLB that the
+    device is connected to.
+
+
+Example: R8A7791 IPMMU-MX and VSP1-D0 bus master
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renasas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	vsp1@fe928000 {
+		...
+		iommus = <&ipmmu_mx 13>;
+		...
+	};
-- 
1.8.3.2

[PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Cc: devicetree@vger.kernel.org
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
new file mode 100644
index 0000000..72e8590
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
@@ -0,0 +1,35 @@
+* Renesas VMSA-Compatible IOMMU
+
+The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
+It provides address translation bus masters outside of the CPU, each connected
+to the IPMMU through a port called micro-TLB.
+
+
+Required Properties:
+
+  - compatible: Must contain "renesas,ipmmu-vmsa".
+  - reg: Base address and size of the IPMMU registers.
+  - interrupts: Specifier for the MMU fault interrupt.
+
+
+Each bus master connected to an IPMMU must reference the IPMMU in its device
+node with the following property:
+
+  - iommus: A reference to the IPMMU in two cells. The first cell is a phandle
+    to the IPMMU and the second cell the number of the micro-TLB that the
+    device is connected to.
+
+
+Example: R8A7791 IPMMU-MX and VSP1-D0 bus master
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renasas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	vsp1@fe928000 {
+		...
+		iommus = <&ipmmu_mx 13>;
+		...
+	};
-- 
1.8.3.2

[PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Laurent Pinchart]

Make platform data optional when the device is instantiated from DT and
look up the micro-TLB number in the bus master DT node.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ef03191..b56a05d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/iommu.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_data/ipmmu-vmsa.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
@@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 
 static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
 {
-	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
-	const char *devname = dev_name(dev);
-	unsigned int i;
+	struct of_phandle_args args;
+	int ret;
+
+	if (mmu->pdata) {
+		const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
+		const char *devname = dev_name(dev);
+		unsigned int i;
+
+		for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
+			if (strcmp(master->name, devname) = 0)
+				return master->utlb;
+		}
 
-	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
-		if (strcmp(master->name, devname) = 0)
-			return master->utlb;
+		return -1;
 	}
 
-	return -1;
+	ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
+					       1, 0, &args);
+	if (ret < 0)
+		return -1;
+
+	if (args.np != mmu->dev->of_node)
+		return -1;
+
+	return args.args[0];
 }
 
 static int ipmmu_add_device(struct device *dev)
@@ -980,7 +996,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 	int irq;
 	int ret;
 
-	if (!pdev->dev.platform_data) {
+	if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
 		dev_err(&pdev->dev, "missing platform data\n");
 		return -EINVAL;
 	}
@@ -1046,10 +1062,15 @@ static int ipmmu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ipmmu_of_ids[] = {
+	{ .compatible = "renesas,ipmmu-vmsa", },
+};
+
 static struct platform_driver ipmmu_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "ipmmu-vmsa",
+		.of_match_table = of_match_ptr(ipmmu_of_ids),
 	},
 	.probe = ipmmu_probe,
 	.remove	= ipmmu_remove,
-- 
1.8.3.2

[PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Laurent Pinchart]

Make platform data optional when the device is instantiated from DT and
look up the micro-TLB number in the bus master DT node.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index ef03191..b56a05d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -16,6 +16,7 @@
 #include <linux/io.h>
 #include <linux/iommu.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_data/ipmmu-vmsa.h>
 #include <linux/platform_device.h>
 #include <linux/sizes.h>
@@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 
 static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
 {
-	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
-	const char *devname = dev_name(dev);
-	unsigned int i;
+	struct of_phandle_args args;
+	int ret;
+
+	if (mmu->pdata) {
+		const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
+		const char *devname = dev_name(dev);
+		unsigned int i;
+
+		for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
+			if (strcmp(master->name, devname) == 0)
+				return master->utlb;
+		}
 
-	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
-		if (strcmp(master->name, devname) == 0)
-			return master->utlb;
+		return -1;
 	}
 
-	return -1;
+	ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
+					       1, 0, &args);
+	if (ret < 0)
+		return -1;
+
+	if (args.np != mmu->dev->of_node)
+		return -1;
+
+	return args.args[0];
 }
 
 static int ipmmu_add_device(struct device *dev)
@@ -980,7 +996,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 	int irq;
 	int ret;
 
-	if (!pdev->dev.platform_data) {
+	if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
 		dev_err(&pdev->dev, "missing platform data\n");
 		return -EINVAL;
 	}
@@ -1046,10 +1062,15 @@ static int ipmmu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ipmmu_of_ids[] = {
+	{ .compatible = "renesas,ipmmu-vmsa", },
+};
+
 static struct platform_driver ipmmu_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "ipmmu-vmsa",
+		.of_match_table = of_match_ptr(ipmmu_of_ids),
 	},
 	.probe = ipmmu_probe,
 	.remove	= ipmmu_remove,
-- 
1.8.3.2

[PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 2e22cb2..bbadf7a 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -909,4 +909,53 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	ipmmu_sy0: mmu@e6280800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6280800 0 0x800>;
+		interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_sy1: mmu@e6290800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6290800 0 0x800>;
+		interrupts = <0 225 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_ds: mmu@e6740800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6740800 0 0x800>;
+		interrupts = <0 198 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_mp: mmu@ec680800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xec680800 0 0x800>;
+		interrupts = <0 226 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_rt: mmu@ffc80800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xffc80800 0 0x800>;
+		interrupts = <0 307 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_gp: mmu@e62a0800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe62a0800 0 0x800>;
+		interrupts = <0 260 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
 };
-- 
1.8.3.2

[PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 49 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 2e22cb2..bbadf7a 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -909,4 +909,53 @@
 		#size-cells = <0>;
 		status = "disabled";
 	};
+
+	ipmmu_sy0: mmu@e6280800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6280800 0 0x800>;
+		interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_sy1: mmu@e6290800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6290800 0 0x800>;
+		interrupts = <0 225 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_ds: mmu@e6740800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6740800 0 0x800>;
+		interrupts = <0 198 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_mp: mmu@ec680800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xec680800 0 0x800>;
+		interrupts = <0 226 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_rt: mmu@ffc80800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xffc80800 0 0x800>;
+		interrupts = <0 307 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
+
+	ipmmu_gp: mmu@e62a0800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe62a0800 0 0x800>;
+		interrupts = <0 260 IRQ_TYPE_LEVEL_HIGH>;
+		status = "disabled";
+	};
 };
-- 
1.8.3.2

[PATCH 5/5] [TEST] ARM: shmobile: r8a7791: Enable IOMMU support for the VSP1

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index bbadf7a..26edcbe 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -508,6 +508,8 @@
 		interrupts = <0 267 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_SY>;
 
+		iommus = <&ipmmu_mx 10>;
+
 		renesas,has-lut;
 		renesas,has-sru;
 		renesas,#rpf = <5>;
@@ -521,6 +523,8 @@
 		interrupts = <0 246 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_DU0>;
 
+		iommus = <&ipmmu_mx 13>;
+
 		renesas,has-lif;
 		renesas,has-lut;
 		renesas,#rpf = <4>;
@@ -534,6 +538,8 @@
 		interrupts = <0 247 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_DU1>;
 
+		iommus = <&ipmmu_mx 14>;
+
 		renesas,has-lif;
 		renesas,has-lut;
 		renesas,#rpf = <4>;
@@ -942,7 +948,6 @@
 		compatible = "renesas,ipmmu-vmsa";
 		reg = <0 0xfe951800 0 0x800>;
 		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
-		status = "disabled";
 	};
 
 	ipmmu_rt: mmu@ffc80800 {
-- 
1.8.3.2

[PATCH 5/5] [TEST] ARM: shmobile: r8a7791: Enable IOMMU support for the VSP1

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm/boot/dts/r8a7791.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index bbadf7a..26edcbe 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -508,6 +508,8 @@
 		interrupts = <0 267 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_SY>;
 
+		iommus = <&ipmmu_mx 10>;
+
 		renesas,has-lut;
 		renesas,has-sru;
 		renesas,#rpf = <5>;
@@ -521,6 +523,8 @@
 		interrupts = <0 246 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_DU0>;
 
+		iommus = <&ipmmu_mx 13>;
+
 		renesas,has-lif;
 		renesas,has-lut;
 		renesas,#rpf = <4>;
@@ -534,6 +538,8 @@
 		interrupts = <0 247 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp1_clks R8A7791_CLK_VSP1_DU1>;
 
+		iommus = <&ipmmu_mx 14>;
+
 		renesas,has-lif;
 		renesas,has-lut;
 		renesas,#rpf = <4>;
@@ -942,7 +948,6 @@
 		compatible = "renesas,ipmmu-vmsa";
 		reg = <0 0xfe951800 0 0x800>;
 		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
-		status = "disabled";
 	};
 
 	ipmmu_rt: mmu@ffc80800 {
-- 
1.8.3.2

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Sergei Shtylyov]

Hello.

On 03/29/2014 02:36 AM, Laurent Pinchart wrote:

> Cc: devicetree@vger.kernel.org
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 ++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

> diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> new file mode 100644
> index 0000000..72e8590
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> @@ -0,0 +1,35 @@
> +* Renesas VMSA-Compatible IOMMU
> +
> +The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
> +It provides address translation bus masters outside of the CPU, each connected
                                   ^ "for" missing?

[...]

WBR, Sergei

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Sergei Shtylyov]

Hello.

On 03/29/2014 02:36 AM, Laurent Pinchart wrote:

> Cc: devicetree@vger.kernel.org
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>   .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 ++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

> diff --git a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> new file mode 100644
> index 0000000..72e8590
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> @@ -0,0 +1,35 @@
> +* Renesas VMSA-Compatible IOMMU
> +
> +The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
> +It provides address translation bus masters outside of the CPU, each connected
                                   ^ "for" missing?

[...]

WBR, Sergei

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> +Required Properties:
> +
> +  - compatible: Must contain "renesas,ipmmu-vmsa".
> +  - reg: Base address and size of the IPMMU registers.
> +  - interrupts: Specifier for the MMU fault interrupt.

Does it make sense to have a property for the number of micro-TLBs,
or is this handled transparently?

E.g. ipmmu_mx has 24 micro-TLBs, but ipmmu_mp has 30.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> +Required Properties:
> +
> +  - compatible: Must contain "renesas,ipmmu-vmsa".
> +  - reg: Base address and size of the IPMMU registers.
> +  - interrupts: Specifier for the MMU fault interrupt.

Does it make sense to have a property for the number of micro-TLBs,
or is this handled transparently?

E.g. ipmmu_mx has 24 micro-TLBs, but ipmmu_mp has 30.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> @@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
>
>  static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
>  {

> +       ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
> +                                              1, 0, &args);

drivers/of/base.c:of_parse_phandle_with_fixed_args():

"Caller is responsible to call of_node_put() on the returned out_args->node
 pointer."

> +       if (ret < 0)
> +               return -1;
> +
> +       if (args.np != mmu->dev->of_node)
> +               return -1;
> +
> +       return args.args[0];
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> @@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
>
>  static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
>  {

> +       ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
> +                                              1, 0, &args);

drivers/of/base.c:of_parse_phandle_with_fixed_args():

"Caller is responsible to call of_node_put() on the returned out_args->node
 pointer."

> +       if (ret < 0)
> +               return -1;
> +
> +       if (args.np != mmu->dev->of_node)
> +               return -1;
> +
> +       return args.args[0];
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> +       ipmmu_sy0: mmu@e6280800 {
> +               compatible = "renesas,ipmmu-vmsa";
> +               reg = <0 0xe6280800 0 0x800>;

Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
banks?

Is there any specific reason you're using the second bank of registers?
These may read as zero, depending on the SoC mode.

> +               interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
> +               status = "disabled";
> +       };

Same comment for the other nodes.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart
<laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> +       ipmmu_sy0: mmu@e6280800 {
> +               compatible = "renesas,ipmmu-vmsa";
> +               reg = <0 0xe6280800 0 0x800>;

Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
banks?

Is there any specific reason you're using the second bank of registers?
These may read as zero, depending on the SoC mode.

> +               interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
> +               status = "disabled";
> +       };

Same comment for the other nodes.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:39:48 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > +Required Properties:
> > +
> > +  - compatible: Must contain "renesas,ipmmu-vmsa".
> > +  - reg: Base address and size of the IPMMU registers.
> > +  - interrupts: Specifier for the MMU fault interrupt.
> 
> Does it make sense to have a property for the number of micro-TLBs,
> or is this handled transparently?
> 
> E.g. ipmmu_mx has 24 micro-TLBs, but ipmmu_mp has 30.

The IPMMU driver doesn't need to know the number of micro-TLBs, so we should 
be fine without specifying it in DT. The micro-TLBs are configured when a bus 
master device is attached or detached, and at that point the device provides 
its micro-TLB number.

The only reason I can foresee why the number of micro-TLBs would be useful is 
to iterate over micro-TLBs when the driver probes the device to disable them 
all. A mask would probably be better than a number in that case, and I think 
we can always add that later if we find a need for it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:39:48 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > +Required Properties:
> > +
> > +  - compatible: Must contain "renesas,ipmmu-vmsa".
> > +  - reg: Base address and size of the IPMMU registers.
> > +  - interrupts: Specifier for the MMU fault interrupt.
> 
> Does it make sense to have a property for the number of micro-TLBs,
> or is this handled transparently?
> 
> E.g. ipmmu_mx has 24 micro-TLBs, but ipmmu_mp has 30.

The IPMMU driver doesn't need to know the number of micro-TLBs, so we should 
be fine without specifying it in DT. The micro-TLBs are configured when a bus 
master device is attached or detached, and at that point the device provides 
its micro-TLB number.

The only reason I can foresee why the number of micro-TLBs would be useful is 
to iterate over micro-TLBs when the driver probes the device to disable them 
all. A mask would probably be better than a number in that case, and I think 
we can always add that later if we find a need for it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > +       ipmmu_sy0: mmu@e6280800 {
> > +               compatible = "renesas,ipmmu-vmsa";
> > +               reg = <0 0xe6280800 0 0x800>;
> 
> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
> banks?
> 
> Is there any specific reason you're using the second bank of registers?
> These may read as zero, depending on the SoC mode.

That's a very good question, and I have no clear answer. According to the 
datasheet the second bank of registers is an alias for the non-secure IPMMU 
registers. It looks like we're running in secure mode (that's what I assume 
the "CPU: All CPU(s) started in SVC mode." kernel log message means), and the 
secure IPMMU didn't seem to be functional when I've tested it.

This requires more investigation, but I'm not familiar with secure mode, and 
the IPMMU documentation is really sparse in that area.

> > +               interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
> > +               status = "disabled";
> > +       };
> 
> Same comment for the other nodes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > +       ipmmu_sy0: mmu@e6280800 {
> > +               compatible = "renesas,ipmmu-vmsa";
> > +               reg = <0 0xe6280800 0 0x800>;
> 
> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
> banks?
> 
> Is there any specific reason you're using the second bank of registers?
> These may read as zero, depending on the SoC mode.

That's a very good question, and I have no clear answer. According to the 
datasheet the second bank of registers is an alias for the non-secure IPMMU 
registers. It looks like we're running in secure mode (that's what I assume 
the "CPU: All CPU(s) started in SVC mode." kernel log message means), and the 
secure IPMMU didn't seem to be functional when I've tested it.

This requires more investigation, but I'm not familiar with secure mode, and 
the IPMMU documentation is really sparse in that area.

> > +               interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
> > +               status = "disabled";
> > +       };
> 
> Same comment for the other nodes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:45:00 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > @@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct
> > iommu_domain *io_domain,> 
> >  static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
> >  *dev) {
> > 
> > +       ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
> > +                                              1, 0, &args);
> 
> drivers/of/base.c:of_parse_phandle_with_fixed_args():
> 
> "Caller is responsible to call of_node_put() on the returned out_args->node
>  pointer."

Thank you for the review. I'll fix that in v2.

> > +       if (ret < 0)
> > +               return -1;
> > +
> > +       if (args.np != mmu->dev->of_node)
> > +               return -1;
> > +
> > +       return args.args[0];
> >  }

- 
Regards,

Laurent Pinchart

Re: [PATCH 3/5] iommu/ipmmu-vmsa: Add device tree support

[Laurent Pinchart]

Hi Geert,

On Monday 31 March 2014 10:45:00 Geert Uytterhoeven wrote:
> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> > @@ -828,16 +829,31 @@ static phys_addr_t ipmmu_iova_to_phys(struct
> > iommu_domain *io_domain,> 
> >  static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
> >  *dev) {
> > 
> > +       ret = of_parse_phandle_with_fixed_args(dev->of_node, "iommus",
> > +                                              1, 0, &args);
> 
> drivers/of/base.c:of_parse_phandle_with_fixed_args():
> 
> "Caller is responsible to call of_node_put() on the returned out_args->node
>  pointer."

Thank you for the review. I'll fix that in v2.

> > +       if (ret < 0)
> > +               return -1;
> > +
> > +       if (args.np != mmu->dev->of_node)
> > +               return -1;
> > +
> > +       return args.args[0];
> >  }

- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Ben Dooks]

On 31/03/14 15:01, Laurent Pinchart wrote:
> Hi Geert,
>
> On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
>> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
>>> +       ipmmu_sy0: mmu@e6280800 {
>>> +               compatible = "renesas,ipmmu-vmsa";
>>> +               reg = <0 0xe6280800 0 0x800>;
>>
>> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
>> banks?
>>
>> Is there any specific reason you're using the second bank of registers?
>> These may read as zero, depending on the SoC mode.
>
> That's a very good question, and I have no clear answer. According to the
> datasheet the second bank of registers is an alias for the non-secure IPMMU
> registers. It looks like we're running in secure mode (that's what I assume
> the "CPU: All CPU(s) started in SVC mode." kernel log message means), and the
> secure IPMMU didn't seem to be functional when I've tested it.
>
> This requires more investigation, but I'm not familiar with secure mode, and
> the IPMMU documentation is really sparse in that area.

The default for the R8A7790 is to start in secure-svc mode.

I would test it in non-secure SVC but the security framework we
are using blocks access to the IPMMU blocks :/


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Ben Dooks]

On 31/03/14 15:01, Laurent Pinchart wrote:
> Hi Geert,
>
> On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
>> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
>>> +       ipmmu_sy0: mmu@e6280800 {
>>> +               compatible = "renesas,ipmmu-vmsa";
>>> +               reg = <0 0xe6280800 0 0x800>;
>>
>> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
>> banks?
>>
>> Is there any specific reason you're using the second bank of registers?
>> These may read as zero, depending on the SoC mode.
>
> That's a very good question, and I have no clear answer. According to the
> datasheet the second bank of registers is an alias for the non-secure IPMMU
> registers. It looks like we're running in secure mode (that's what I assume
> the "CPU: All CPU(s) started in SVC mode." kernel log message means), and the
> secure IPMMU didn't seem to be functional when I've tested it.
>
> This requires more investigation, but I'm not familiar with secure mode, and
> the IPMMU documentation is really sparse in that area.

The default for the R8A7790 is to start in secure-svc mode.

I would test it in non-secure SVC but the security framework we
are using blocks access to the IPMMU blocks :/


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Ben,

On Monday 31 March 2014 15:30:00 Ben Dooks wrote:
> On 31/03/14 15:01, Laurent Pinchart wrote:
> > On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
> >> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> >>> +       ipmmu_sy0: mmu@e6280800 {
> >>> +               compatible = "renesas,ipmmu-vmsa";
> >>> +               reg = <0 0xe6280800 0 0x800>;
> >> 
> >> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
> >> banks?
> >> 
> >> Is there any specific reason you're using the second bank of registers?
> >> These may read as zero, depending on the SoC mode.
> > 
> > That's a very good question, and I have no clear answer. According to the
> > datasheet the second bank of registers is an alias for the non-secure
> > IPMMU registers. It looks like we're running in secure mode (that's what I
> > assume the "CPU: All CPU(s) started in SVC mode." kernel log message
> > means), and the secure IPMMU didn't seem to be functional when I've tested
> > it.
> > 
> > This requires more investigation, but I'm not familiar with secure mode,
> > and the IPMMU documentation is really sparse in that area.
> 
> The default for the R8A7790 is to start in secure-svc mode.

I've tried to boot in non-secure mode by modifying MD5 (DIP switch SW10), but 
it doesn't seem to have any influence.

> I would test it in non-secure SVC but the security framework we are using
> blocks access to the IPMMU blocks :/

I assume that the reason why I have to program the IPMMU non-secure page table 
is that the memory requests coming from the DU are considered as non-secure. 
That's just a guess though. Without a detailed description of how the hardware 
is supposed to work I can't improve the code and DT bindings.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/5] ARM: shmobile: r8a7791: Add IPMMU DT nodes

[Laurent Pinchart]

Hi Ben,

On Monday 31 March 2014 15:30:00 Ben Dooks wrote:
> On 31/03/14 15:01, Laurent Pinchart wrote:
> > On Monday 31 March 2014 10:52:28 Geert Uytterhoeven wrote:
> >> On Sat, Mar 29, 2014 at 12:36 AM, Laurent Pinchart wrote:
> >>> +       ipmmu_sy0: mmu@e6280800 {
> >>> +               compatible = "renesas,ipmmu-vmsa";
> >>> +               reg = <0 0xe6280800 0 0x800>;
> >> 
> >> Shouldn't this be "reg = <0 0xe6280000 0 0x1000>", i.e. expose both
> >> banks?
> >> 
> >> Is there any specific reason you're using the second bank of registers?
> >> These may read as zero, depending on the SoC mode.
> > 
> > That's a very good question, and I have no clear answer. According to the
> > datasheet the second bank of registers is an alias for the non-secure
> > IPMMU registers. It looks like we're running in secure mode (that's what I
> > assume the "CPU: All CPU(s) started in SVC mode." kernel log message
> > means), and the secure IPMMU didn't seem to be functional when I've tested
> > it.
> > 
> > This requires more investigation, but I'm not familiar with secure mode,
> > and the IPMMU documentation is really sparse in that area.
> 
> The default for the R8A7790 is to start in secure-svc mode.

I've tried to boot in non-secure mode by modifying MD5 (DIP switch SW10), but 
it doesn't seem to have any influence.

> I would test it in non-secure SVC but the security framework we are using
> blocks access to the IPMMU blocks :/

I assume that the reason why I have to program the IPMMU non-secure page table 
is that the memory requests coming from the DU are considered as non-secure. 
That's just a guess though. Without a detailed description of how the hardware 
is supposed to work I can't improve the code and DT bindings.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Hi Sergei,

Thank you for the review.

On Saturday 29 March 2014 18:50:16 Sergei Shtylyov wrote:
> On 03/29/2014 02:36 AM, Laurent Pinchart wrote:
> > Cc: devicetree@vger.kernel.org
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 +++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> > b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt new file
> > mode 100644
> > index 0000000..72e8590
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> > @@ -0,0 +1,35 @@
> > +* Renesas VMSA-Compatible IOMMU
> > +
> > +The IPMMU is an IOMMU implementation compatible with the ARM VMSA page
> > tables.
> > +It provides address translation bus masters outside of the CPU, each
> > connected
>                                    ^ "for" missing?

Of course. I'll fix that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/5] iommu/ipmmu-vmsa: Add device tree bindings documentation

[Laurent Pinchart]

Hi Sergei,

Thank you for the review.

On Saturday 29 March 2014 18:50:16 Sergei Shtylyov wrote:
> On 03/29/2014 02:36 AM, Laurent Pinchart wrote:
> > Cc: devicetree@vger.kernel.org
> > Cc: Will Deacon <will.deacon@arm.com>
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 35 +++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> > b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt new file
> > mode 100644
> > index 0000000..72e8590
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
> > @@ -0,0 +1,35 @@
> > +* Renesas VMSA-Compatible IOMMU
> > +
> > +The IPMMU is an IOMMU implementation compatible with the ARM VMSA page
> > tables.
> > +It provides address translation bus masters outside of the CPU, each
> > connected
>                                    ^ "for" missing?

Of course. I'll fix that.

-- 
Regards,

Laurent Pinchart