linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support
@ 2014-12-15  0:13 Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 02/10] iommu/ipmmu-vmsa: Flush P[UM]D entry before freeing the child page table Laurent Pinchart
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: Simon Horman, linux-sh-u79uwXL29TY76Z2rM5mHXA

Hello,

This patch fixes several small issues with the Renesas IPMMU-VMSA driver and
then implements DT support.

Patches 01/10 to 04/10 are small bug fixes. Please see the individual patches
for more information.

Patches 05/10 and 06/10 define DT bindings and implement DT support. Patches
07/10 and 08/10 then add support for multiple micro TLBs per device and remove
platform data support. Finally patches 09/10 and 10/10 add the IPMMU DT nodes
for the R-Car H2 and M2.

Joerg, as the patches have all been sent to public mailing lists previously I
will send you a pull request for patches 01/10 to 08/10 for v3.20 when the
v3.19 merge window closes. Patches 09/10 and 10/10 can then be merged
independently through the Renesas tree.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: Simon Horman <horms@verge.net.au>

Axel Lin (1):
  iommu/ipmmu-vmsa: Return proper error if devm_request_irq fails

Laurent Pinchart (9):
  iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or
    attachment
  iommu/ipmmu-vmsa: Flush P[UM]D entry before freeing the child page
    table
  iommu/ipmmu-vmsa: Invalidate TLB after unmapping
  iommu/ipmmu-vmsa: Add device tree bindings documentation
  iommu/ipmmu-vmsa: Add device tree support
  iommu/ipmmu-vmsa: Support multiple micro TLBs per device
  iommu/ipmmu-vmsa: Remove platform data support
  ARM: shmobile: r8a7790: Add IPMMU DT nodes
  ARM: shmobile: r8a7791: Add IPMMU DT nodes

 .../bindings/iommu/renesas,ipmmu-vmsa.txt          |  37 ++++++
 arch/arm/boot/dts/r8a7790.dtsi                     |  48 ++++++++
 arch/arm/boot/dts/r8a7791.dtsi                     |  56 +++++++++
 drivers/iommu/ipmmu-vmsa.c                         | 137 +++++++++++++++------
 include/linux/platform_data/ipmmu-vmsa.h           |  24 ----
 5 files changed, 239 insertions(+), 63 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
 delete mode 100644 include/linux/platform_data/ipmmu-vmsa.h

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment
       [not found] ` <1418602429-11276-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
@ 2014-12-15  0:13   ` Laurent Pinchart
  2014-12-15  0:13   ` [PATCH 04/10] iommu/ipmmu-vmsa: Return proper error if devm_request_irq fails Laurent Pinchart
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA

The ARM IOMMU mapping needs to be released when attaching the device
fails. Add arm_iommu_release_mapping() to the error code path. This is
safe to call with a NULL mapping, so no specific check is needed.

Cleanup is also missing when failing to create a mapping. Jump to the
error code path in that case instead of returning immediately.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 7dab5cbcc775..f7036adb5634 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1090,7 +1090,8 @@ static int ipmmu_add_device(struct device *dev)
 						   SZ_1G, SZ_2G);
 		if (IS_ERR(mapping)) {
 			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
-			return PTR_ERR(mapping);
+			ret = PTR_ERR(mapping);
+			goto error;
 		}
 
 		mmu->mapping = mapping;
@@ -1106,6 +1107,7 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
+	arm_iommu_release_mapping(mmu->mapping);
 	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/10] iommu/ipmmu-vmsa: Flush P[UM]D entry before freeing the child page table
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 03/10] iommu/ipmmu-vmsa: Invalidate TLB after unmapping Laurent Pinchart
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

When clearing PUD or PMD entries the child page table (if any) is freed
and the PUD or PMD entry is then cleared. This result in a small race
condition window during which a free page table could be accessed by the
IPMMU.

Fix it by clearing and flushing the PUD or PMD entry before freeing the
child page table.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index f7036adb5634..fcb603d8b041 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -678,30 +678,33 @@ done:
 
 static void ipmmu_clear_pud(struct ipmmu_vmsa_device *mmu, pud_t *pud)
 {
-	/* Free the page table. */
 	pgtable_t table = pud_pgtable(*pud);
-	__free_page(table);
 
 	/* Clear the PUD. */
 	*pud = __pud(0);
 	ipmmu_flush_pgtable(mmu, pud, sizeof(*pud));
+
+	/* Free the page table. */
+	__free_page(table);
 }
 
 static void ipmmu_clear_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
 			    pmd_t *pmd)
 {
+	pmd_t pmdval = *pmd;
 	unsigned int i;
 
-	/* Free the page table. */
-	if (pmd_table(*pmd)) {
-		pgtable_t table = pmd_pgtable(*pmd);
-		__free_page(table);
-	}
-
 	/* Clear the PMD. */
 	*pmd = __pmd(0);
 	ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
 
+	/* Free the page table. */
+	if (pmd_table(pmdval)) {
+		pgtable_t table = pmd_pgtable(pmdval);
+
+		__free_page(table);
+	}
+
 	/* Check whether the PUD is still needed. */
 	pmd = pmd_offset(pud, 0);
 	for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) {
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/10] iommu/ipmmu-vmsa: Invalidate TLB after unmapping
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 02/10] iommu/ipmmu-vmsa: Flush P[UM]D entry before freeing the child page table Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
       [not found] ` <1418602429-11276-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

The TLB must be invalidated after unmapping memory to remove stale TLB
entries. this was supposed to be performed already, but a bug in the
driver prevented the TLB invalidate function from being called. Fix it.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index fcb603d8b041..9838d119045d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -785,7 +785,6 @@ static int ipmmu_clear_mapping(struct ipmmu_vmsa_domain *domain,
 	pud_t *pud;
 	pmd_t *pmd;
 	pte_t *pte;
-	int ret = 0;
 
 	if (!pgd)
 		return -EINVAL;
@@ -847,8 +846,7 @@ static int ipmmu_clear_mapping(struct ipmmu_vmsa_domain *domain,
 done:
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	if (ret)
-		ipmmu_tlb_invalidate(domain);
+	ipmmu_tlb_invalidate(domain);
 
 	return 0;
 }
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/10] iommu/ipmmu-vmsa: Return proper error if devm_request_irq fails
       [not found] ` <1418602429-11276-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
  2014-12-15  0:13   ` [PATCH 01/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment Laurent Pinchart
@ 2014-12-15  0:13   ` Laurent Pinchart
  1 sibling, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
  Cc: Axel Lin, linux-sh-u79uwXL29TY76Z2rM5mHXA

From: Axel Lin <axel.lin@ingics.com>

Signed-off-by: Axel Lin <axel.lin@ingics.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 9838d119045d..70bb5ba2aa51 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1187,7 +1187,7 @@ static int ipmmu_probe(struct platform_device *pdev)
 			       dev_name(&pdev->dev), mmu);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to request IRQ %d\n", irq);
-		return irq;
+		return ret;
 	}
 
 	ipmmu_device_reset(mmu);
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/10] iommu/ipmmu-vmsa: Add device tree bindings documentation
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (2 preceding siblings ...)
       [not found] ` <1418602429-11276-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 06/10] iommu/ipmmu-vmsa: Add device tree support Laurent Pinchart
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh, devicetree

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/iommu/renesas,ipmmu-vmsa.txt          | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt

Cc: devicetree@vger.kernel.org

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 000000000000..88b5bacc8c7f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.txt
@@ -0,0 +1,37 @@
+* Renesas VMSA-Compatible IOMMU
+
+The IPMMU is an IOMMU implementation compatible with the ARM VMSA page tables.
+It provides address translation for 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.
+
+  - #iommu-cells: Must be 1.
+
+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>;
+		#iommu-cells = <1>;
+	};
+
+	vsp1@fe928000 {
+		...
+		iommus = <&ipmmu_mx 13>;
+		...
+	};
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/10] iommu/ipmmu-vmsa: Add device tree support
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (3 preceding siblings ...)
  2014-12-15  0:13 ` [PATCH 05/10] iommu/ipmmu-vmsa: Add device tree bindings documentation Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 07/10] iommu/ipmmu-vmsa: Support multiple micro TLBs per device Laurent Pinchart
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh, devicetree

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 | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

Cc: devicetree@vger.kernel.org

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 70bb5ba2aa51..dbbdb76d5a6b 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>
@@ -1002,16 +1003,33 @@ 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_args(dev->of_node, "iommus",
+					 "#iommu-cells", 0, &args);
+	if (ret < 0)
+		return -1;
+
+	of_node_put(args.np);
+
+	if (args.np != mmu->dev->of_node || args.args_count != 1)
+		return -1;
+
+	return args.args[0];
 }
 
 static int ipmmu_add_device(struct device *dev)
@@ -1156,7 +1174,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;
 	}
@@ -1222,10 +1240,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,
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/10] iommu/ipmmu-vmsa: Support multiple micro TLBs per device
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (4 preceding siblings ...)
  2014-12-15  0:13 ` [PATCH 06/10] iommu/ipmmu-vmsa: Add device tree support Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 08/10] iommu/ipmmu-vmsa: Remove platform data support Laurent Pinchart
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

Devices such as the system DMA controller are connected to multiple
micro TLBs of the same IOMMU. Support this.

Selective enabling of micro TLBs based on runtime device usage isn't
possible at the moment due to lack of support in the IOMMU and DMA
mapping APIs. Support for devices connected to different IOMMUs is also
unsupported for the same reason.

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

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index dbbdb76d5a6b..5a99fad5c30f 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -47,7 +47,8 @@ struct ipmmu_vmsa_domain {
 
 struct ipmmu_vmsa_archdata {
 	struct ipmmu_vmsa_device *mmu;
-	unsigned int utlb;
+	unsigned int *utlbs;
+	unsigned int num_utlbs;
 };
 
 static DEFINE_SPINLOCK(ipmmu_devices_lock);
@@ -898,6 +899,7 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	struct ipmmu_vmsa_device *mmu = archdata->mmu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
 	unsigned long flags;
+	unsigned int i;
 	int ret = 0;
 
 	if (!mmu) {
@@ -926,7 +928,8 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	if (ret < 0)
 		return ret;
 
-	ipmmu_utlb_enable(domain, archdata->utlb);
+	for (i = 0; i < archdata->num_utlbs; ++i)
+		ipmmu_utlb_enable(domain, archdata->utlbs[i]);
 
 	return 0;
 }
@@ -936,8 +939,10 @@ static void ipmmu_detach_device(struct iommu_domain *io_domain,
 {
 	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
+	unsigned int i;
 
-	ipmmu_utlb_disable(domain, archdata->utlb);
+	for (i = 0; i < archdata->num_utlbs; ++i)
+		ipmmu_utlb_disable(domain, archdata->utlbs[i]);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -1001,10 +1006,12 @@ 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)
+static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device *dev,
+			    unsigned int **_utlbs)
 {
-	struct of_phandle_args args;
-	int ret;
+	unsigned int *utlbs;
+	unsigned int i;
+	int count;
 
 	if (mmu->pdata) {
 		const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
@@ -1012,32 +1019,64 @@ static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
 		unsigned int i;
 
 		for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
-			if (strcmp(master->name, devname) = 0)
-				return master->utlb;
+			if (strcmp(master->name, devname) = 0) {
+				utlbs = kmalloc(sizeof(*utlbs), GFP_KERNEL);
+				if (!utlbs)
+					return -ENOMEM;
+
+				utlbs[0] = master->utlb;
+
+				*_utlbs = utlbs;
+				return 1;
+			}
 		}
 
-		return -1;
+		return -EINVAL;
 	}
 
-	ret = of_parse_phandle_with_args(dev->of_node, "iommus",
-					 "#iommu-cells", 0, &args);
-	if (ret < 0)
-		return -1;
+	count = of_count_phandle_with_args(dev->of_node, "iommus",
+					   "#iommu-cells");
+	if (count < 0)
+		return -EINVAL;
+
+	utlbs = kcalloc(count, sizeof(*utlbs), GFP_KERNEL);
+	if (!utlbs)
+		return -ENOMEM;
+
+	for (i = 0; i < count; ++i) {
+		struct of_phandle_args args;
+		int ret;
+
+		ret = of_parse_phandle_with_args(dev->of_node, "iommus",
+						 "#iommu-cells", i, &args);
+		if (ret < 0)
+			goto error;
+
+		of_node_put(args.np);
+
+		if (args.np != mmu->dev->of_node || args.args_count != 1)
+			goto error;
+
+		utlbs[i] = args.args[0];
+	}
 
-	of_node_put(args.np);
+	*_utlbs = utlbs;
 
-	if (args.np != mmu->dev->of_node || args.args_count != 1)
-		return -1;
+	return count;
 
-	return args.args[0];
+error:
+	kfree(utlbs);
+	return -EINVAL;
 }
 
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
-	struct iommu_group *group;
-	int utlb = -1;
+	struct iommu_group *group = NULL;
+	unsigned int *utlbs = NULL;
+	unsigned int i;
+	int num_utlbs = 0;
 	int ret;
 
 	if (dev->archdata.iommu) {
@@ -1050,8 +1089,8 @@ static int ipmmu_add_device(struct device *dev)
 	spin_lock(&ipmmu_devices_lock);
 
 	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		utlb = ipmmu_find_utlb(mmu, dev);
-		if (utlb >= 0) {
+		num_utlbs = ipmmu_find_utlbs(mmu, dev, &utlbs);
+		if (num_utlbs) {
 			/*
 			 * TODO Take a reference to the MMU to protect
 			 * against device removal.
@@ -1062,17 +1101,22 @@ static int ipmmu_add_device(struct device *dev)
 
 	spin_unlock(&ipmmu_devices_lock);
 
-	if (utlb < 0)
+	if (num_utlbs <= 0)
 		return -ENODEV;
 
-	if (utlb >= mmu->num_utlbs)
-		return -EINVAL;
+	for (i = 0; i < num_utlbs; ++i) {
+		if (utlbs[i] >= mmu->num_utlbs) {
+			ret = -EINVAL;
+			goto error;
+		}
+	}
 
 	/* Create a device group and add the device to it. */
 	group = iommu_group_alloc();
 	if (IS_ERR(group)) {
 		dev_err(dev, "Failed to allocate IOMMU group\n");
-		return PTR_ERR(group);
+		ret = PTR_ERR(group);
+		goto error;
 	}
 
 	ret = iommu_group_add_device(group, dev);
@@ -1080,7 +1124,8 @@ static int ipmmu_add_device(struct device *dev)
 
 	if (ret < 0) {
 		dev_err(dev, "Failed to add device to IPMMU group\n");
-		return ret;
+		group = NULL;
+		goto error;
 	}
 
 	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
@@ -1090,7 +1135,8 @@ static int ipmmu_add_device(struct device *dev)
 	}
 
 	archdata->mmu = mmu;
-	archdata->utlb = utlb;
+	archdata->utlbs = utlbs;
+	archdata->num_utlbs = num_utlbs;
 	dev->archdata.iommu = archdata;
 
 	/*
@@ -1127,17 +1173,28 @@ static int ipmmu_add_device(struct device *dev)
 
 error:
 	arm_iommu_release_mapping(mmu->mapping);
+
 	kfree(dev->archdata.iommu);
+	kfree(utlbs);
+
 	dev->archdata.iommu = NULL;
-	iommu_group_remove_device(dev);
+
+	if (!IS_ERR_OR_NULL(group))
+		iommu_group_remove_device(dev);
+
 	return ret;
 }
 
 static void ipmmu_remove_device(struct device *dev)
 {
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
-	kfree(dev->archdata.iommu);
+
+	kfree(archdata->utlbs);
+	kfree(archdata);
+
 	dev->archdata.iommu = NULL;
 }
 
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/10] iommu/ipmmu-vmsa: Remove platform data support
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (5 preceding siblings ...)
  2014-12-15  0:13 ` [PATCH 07/10] iommu/ipmmu-vmsa: Support multiple micro TLBs per device Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 09/10] ARM: shmobile: r8a7790: Add IPMMU DT nodes Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 10/10] ARM: shmobile: r8a7791: " Laurent Pinchart
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

No board file instantiates the IPMMU using platform data. Now that we
have DT support, get rid of platform data.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c               | 24 ------------------------
 include/linux/platform_data/ipmmu-vmsa.h | 24 ------------------------
 2 files changed, 48 deletions(-)
 delete mode 100644 include/linux/platform_data/ipmmu-vmsa.h

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 5a99fad5c30f..aa44a93e8a17 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -17,7 +17,6 @@
 #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>
 #include <linux/slab.h>
@@ -30,7 +29,6 @@ struct ipmmu_vmsa_device {
 	void __iomem *base;
 	struct list_head list;
 
-	const struct ipmmu_vmsa_platform_data *pdata;
 	unsigned int num_utlbs;
 
 	struct dma_iommu_mapping *mapping;
@@ -1013,27 +1011,6 @@ static int ipmmu_find_utlbs(struct ipmmu_vmsa_device *mmu, struct device *dev,
 	unsigned int i;
 	int count;
 
-	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) {
-				utlbs = kmalloc(sizeof(*utlbs), GFP_KERNEL);
-				if (!utlbs)
-					return -ENOMEM;
-
-				utlbs[0] = master->utlb;
-
-				*_utlbs = utlbs;
-				return 1;
-			}
-		}
-
-		return -EINVAL;
-	}
-
 	count = of_count_phandle_with_args(dev->of_node, "iommus",
 					   "#iommu-cells");
 	if (count < 0)
@@ -1243,7 +1220,6 @@ static int ipmmu_probe(struct platform_device *pdev)
 	}
 
 	mmu->dev = &pdev->dev;
-	mmu->pdata = pdev->dev.platform_data;
 	mmu->num_utlbs = 32;
 
 	/* Map I/O memory and request IRQ. */
diff --git a/include/linux/platform_data/ipmmu-vmsa.h b/include/linux/platform_data/ipmmu-vmsa.h
deleted file mode 100644
index 5275b3ac6d37..000000000000
--- a/include/linux/platform_data/ipmmu-vmsa.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * IPMMU VMSA Platform Data
- *
- * Copyright (C) 2014 Renesas Electronics Corporation
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- */
-
-#ifndef __IPMMU_VMSA_H__
-#define __IPMMU_VMSA_H__
-
-struct ipmmu_vmsa_master {
-	const char *name;
-	unsigned int utlb;
-};
-
-struct ipmmu_vmsa_platform_data {
-	const struct ipmmu_vmsa_master *masters;
-	unsigned int num_masters;
-};
-
-#endif /* __IPMMU_VMSA_H__ */
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/10] ARM: shmobile: r8a7790: Add IPMMU DT nodes
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (6 preceding siblings ...)
  2014-12-15  0:13 ` [PATCH 08/10] iommu/ipmmu-vmsa: Remove platform data support Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15  0:13 ` [PATCH 10/10] ARM: shmobile: r8a7791: " Laurent Pinchart
  8 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

Add the six IPMMU instances found in the r8a7790 to DT with a disabled
status.

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

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index ffeff98fa16e..430bb67216fd 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -1459,4 +1459,52 @@
 			ssi9: ssi@9 { interrupts = <0 379 IRQ_TYPE_LEVEL_HIGH>; };
 		};
 	};
+
+	ipmmu_sy0: mmu@e6280800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6280800 0 0x800>;
+		interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_sy1: mmu@e6290800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6290800 0 0x800>;
+		interrupts = <0 225 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_ds: mmu@e6740800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6740800 0 0x800>;
+		interrupts = <0 198 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_mp: mmu@ec680800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xec680800 0 0x800>;
+		interrupts = <0 226 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_rt: mmu@ffc80800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xffc80800 0 0x800>;
+		interrupts = <0 307 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
 };
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
                   ` (7 preceding siblings ...)
  2014-12-15  0:13 ` [PATCH 09/10] ARM: shmobile: r8a7790: Add IPMMU DT nodes Laurent Pinchart
@ 2014-12-15  0:13 ` Laurent Pinchart
  2014-12-15 13:07   ` Geert Uytterhoeven
  8 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15  0:13 UTC (permalink / raw)
  To: iommu; +Cc: linux-sh

Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
status.

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

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 78d637135e77..fb4803084742 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1384,6 +1384,62 @@
 		status = "disabled";
 	};
 
+	ipmmu_sy0: mmu@e6280800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6280800 0 0x800>;
+		interrupts = <0 223 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_sy1: mmu@e6290800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6290800 0 0x800>;
+		interrupts = <0 225 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_ds: mmu@e6740800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe6740800 0 0x800>;
+		interrupts = <0 198 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_mp: mmu@ec680800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xec680800 0 0x800>;
+		interrupts = <0 226 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_mx: mmu@fe951800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xfe951800 0 0x800>;
+		interrupts = <0 222 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_rt: mmu@ffc80800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xffc80800 0 0x800>;
+		interrupts = <0 307 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
+	ipmmu_gp: mmu@e62a0800 {
+		compatible = "renesas,ipmmu-vmsa";
+		reg = <0 0xe62a0800 0 0x800>;
+		interrupts = <0 260 IRQ_TYPE_LEVEL_HIGH>;
+		#iommu-cells = <1>;
+		status = "disabled";
+	};
+
 	rcar_sound: rcar_sound@ec500000 {
 		#sound-dai-cells = <1>;
 		compatible =  "renesas,rcar_sound-r8a7791", "renesas,rcar_sound-gen2", "renesas,rcar_sound";
-- 
2.0.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15  0:13 ` [PATCH 10/10] ARM: shmobile: r8a7791: " Laurent Pinchart
@ 2014-12-15 13:07   ` Geert Uytterhoeven
       [not found]     ` <CAMuHMdWtkk=Q=Mfcyz03LBzXMuBOr8BZGNyCsQb9bwDty-4OiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-12-15 13:07 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: iommu, Linux-sh list

On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
> status.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The addresses and interrupt numbers look OK to me.
However, my comment about the "0x800" offset is still valid.
Shouldn't we have two register blocks, and let the driver use only the
second one?

If you ignore, feel free to add my
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
       [not found]     ` <CAMuHMdWtkk=Q=Mfcyz03LBzXMuBOr8BZGNyCsQb9bwDty-4OiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2014-12-15 18:44       ` Laurent Pinchart
  2014-12-15 22:44         ` Magnus Damm
  2014-12-16 10:02         ` Geert Uytterhoeven
  0 siblings, 2 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15 18:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Linux-sh list

Hi Geert,

On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
> > status.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> The addresses and interrupt numbers look OK to me.
> However, my comment about the "0x800" offset is still valid.
> Shouldn't we have two register blocks, and let the driver use only the
> second one?
> 
> If you ignore, feel free to add my
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

I don't want to ignore your comment, but I don't know what to do here :-/ The 
datasheet lacks detailed information about secure vs. non-secure mode and how 
the two register sets are supposed to interoperate and be handled by the 
operating system.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15 18:44       ` Laurent Pinchart
@ 2014-12-15 22:44         ` Magnus Damm
  2014-12-15 23:16           ` Laurent Pinchart
  2014-12-16 10:02         ` Geert Uytterhoeven
  1 sibling, 1 reply; 18+ messages in thread
From: Magnus Damm @ 2014-12-15 22:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, iommu, Linux-sh list

Hi Laurent,

On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Geert,
>
> On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
>> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
>> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
>> > status.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>>
>> The addresses and interrupt numbers look OK to me.
>> However, my comment about the "0x800" offset is still valid.
>> Shouldn't we have two register blocks, and let the driver use only the
>> second one?
>>
>> If you ignore, feel free to add my
>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> I don't want to ignore your comment, but I don't know what to do here :-/ The
> datasheet lacks detailed information about secure vs. non-secure mode and how
> the two register sets are supposed to interoperate and be handled by the
> operating system.

I don't know about that either.

But how about differences within R-Car Gen2 series so far? If there
are known differences then perhaps we should use part number in the
compatible string?

Thanks,

/ magnus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15 22:44         ` Magnus Damm
@ 2014-12-15 23:16           ` Laurent Pinchart
  2014-12-16 11:54             ` Magnus Damm
  0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-15 23:16 UTC (permalink / raw)
  To: Magnus Damm; +Cc: Geert Uytterhoeven, Laurent Pinchart, iommu, Linux-sh list

Hi Magnus,

On Tuesday 16 December 2014 07:44:32 Magnus Damm wrote:
> On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart wrote:
> > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
> >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
> >> > Add the seven IPMMU instances found in the r8a7791 to DT with a
> >> > disabled status.
> >> > 
> >> > Signed-off-by: Laurent Pinchart
> >> > <laurent.pinchart+renesas@ideasonboard.com>
> >> 
> >> The addresses and interrupt numbers look OK to me.
> >> However, my comment about the "0x800" offset is still valid.
> >> Shouldn't we have two register blocks, and let the driver use only the
> >> second one?
> >> 
> >> If you ignore, feel free to add my
> >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > I don't want to ignore your comment, but I don't know what to do here :-/
> > The datasheet lacks detailed information about secure vs. non-secure mode
> > and how the two register sets are supposed to interoperate and be handled
> > by the operating system.
> 
> I don't know about that either.
> 
> But how about differences within R-Car Gen2 series so far? If there
> are known differences then perhaps we should use part number in the
> compatible string?

I haven't noticed differences between H2 and M2. V2 and E2 are extended with 
IOMMU performance monitoring registers, so a specific compatible string is 
needed.

"renesas,ipmmu-vmsa" should be the default fallback compatible string. I was 
thinking about playing with the newer SoCs first to see how the IOMMU react 
and then most likely add SoC-specific compatible strings.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15 18:44       ` Laurent Pinchart
  2014-12-15 22:44         ` Magnus Damm
@ 2014-12-16 10:02         ` Geert Uytterhoeven
  2014-12-16 11:47           ` Laurent Pinchart
  1 sibling, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-12-16 10:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, Linux-sh list

Hi Laurent,

On Mon, Dec 15, 2014 at 7:44 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
>> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
>> > Add the seven IPMMU instances found in the r8a7791 to DT with a disabled
>> > status.
>> >
>> > Signed-off-by: Laurent Pinchart
>> > <laurent.pinchart+renesas@ideasonboard.com>
>>
>> The addresses and interrupt numbers look OK to me.
>> However, my comment about the "0x800" offset is still valid.
>> Shouldn't we have two register blocks, and let the driver use only the
>> second one?
>>
>> If you ignore, feel free to add my
>> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> I don't want to ignore your comment, but I don't know what to do here :-/ The
> datasheet lacks detailed information about secure vs. non-secure mode and how
> the two register sets are supposed to interoperate and be handled by the
> operating system.

When in doubt, the safest thing to do is "describe the hardware in DT".

The datasheet says there are two register sets of size 0x800, so IMHO we
should have both in DT. Whether the driver only uses the second bank due to
our limited understanding of the hardware is something different.
We can still fix the driver later, if needed.
Fixing the DT is, ... well... complicated ;-)

Following the rule "describe the hardware in DT", I would also add the second
interrupt (marked "SEC") for the DS0, MX, SY0, and GP IPMMU instances.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-16 10:02         ` Geert Uytterhoeven
@ 2014-12-16 11:47           ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2014-12-16 11:47 UTC (permalink / raw)
  To: Geert Uytterhoeven, iommu; +Cc: Linux-sh list

Hi Geert,

On Tuesday 16 December 2014 11:02:03 Geert Uytterhoeven wrote:
> On Mon, Dec 15, 2014 at 7:44 PM, Laurent Pinchart wrote:
> > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
> >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
> >> > Add the seven IPMMU instances found in the r8a7791 to DT with a
> >> > disabled status.
> >> > 
> >> > Signed-off-by: Laurent Pinchart
> >> > <laurent.pinchart+renesas@ideasonboard.com>
> >> 
> >> The addresses and interrupt numbers look OK to me.
> >> However, my comment about the "0x800" offset is still valid.
> >> Shouldn't we have two register blocks, and let the driver use only the
> >> second one?
> >> 
> >> If you ignore, feel free to add my
> >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > I don't want to ignore your comment, but I don't know what to do here :-/
> > The datasheet lacks detailed information about secure vs. non-secure mode
> > and how the two register sets are supposed to interoperate and be handled
> > by the operating system.
> 
> When in doubt, the safest thing to do is "describe the hardware in DT".
> 
> The datasheet says there are two register sets of size 0x800, so IMHO we
> should have both in DT. Whether the driver only uses the second bank due to
> our limited understanding of the hardware is something different. We can
> still fix the driver later, if needed. Fixing the DT is, ... well...
> complicated ;-)

According to the datasheet there are two register sets sharing the same base 
address, banked through secure mode. There's additionally an alias at a 
different base address for non-secure registers access when running in secure 
mode. The hardware description thus depends on whether we're running in secure 
or non-secure mode, which doesn't play nicely with DT.

Additionally, I've tried to disable secure mode at boot time to test non-
secure mode at the base address, but it didn't seem to make any difference. I 
might have done something wrong though.

For these reasons I don't really like the idea of having two resources for 
secure and non-secure mode. Extending the single resource to cover the base 
address and the aliases should be fine though, I'll give it a go.

> Following the rule "describe the hardware in DT", I would also add the
> second interrupt (marked "SEC") for the DS0, MX, SY0, and GP IPMMU
> instances.

Agreed, I'll do that.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 10/10] ARM: shmobile: r8a7791: Add IPMMU DT nodes
  2014-12-15 23:16           ` Laurent Pinchart
@ 2014-12-16 11:54             ` Magnus Damm
  0 siblings, 0 replies; 18+ messages in thread
From: Magnus Damm @ 2014-12-16 11:54 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Laurent Pinchart, iommu, Linux-sh list

Hi Laurent,

On Tue, Dec 16, 2014 at 8:16 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Tuesday 16 December 2014 07:44:32 Magnus Damm wrote:
>> On Tue, Dec 16, 2014 at 3:44 AM, Laurent Pinchart wrote:
>> > On Monday 15 December 2014 14:07:52 Geert Uytterhoeven wrote:
>> >> On Mon, Dec 15, 2014 at 1:13 AM, Laurent Pinchart wrote:
>> >> > Add the seven IPMMU instances found in the r8a7791 to DT with a
>> >> > disabled status.
>> >> >
>> >> > Signed-off-by: Laurent Pinchart
>> >> > <laurent.pinchart+renesas@ideasonboard.com>
>> >>
>> >> The addresses and interrupt numbers look OK to me.
>> >> However, my comment about the "0x800" offset is still valid.
>> >> Shouldn't we have two register blocks, and let the driver use only the
>> >> second one?
>> >>
>> >> If you ignore, feel free to add my
>> >> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> >
>> > I don't want to ignore your comment, but I don't know what to do here :-/
>> > The datasheet lacks detailed information about secure vs. non-secure mode
>> > and how the two register sets are supposed to interoperate and be handled
>> > by the operating system.
>>
>> I don't know about that either.
>>
>> But how about differences within R-Car Gen2 series so far? If there
>> are known differences then perhaps we should use part number in the
>> compatible string?
>
> I haven't noticed differences between H2 and M2. V2 and E2 are extended with
> IOMMU performance monitoring registers, so a specific compatible string is
> needed.

Ok!

> "renesas,ipmmu-vmsa" should be the default fallback compatible string. I was
> thinking about playing with the newer SoCs first to see how the IOMMU react
> and then most likely add SoC-specific compatible strings.

Sounds good, thanks!

/ magnus

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-12-16 11:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-15  0:13 [PATCH 00/10] Renesas IPMMU-VMSA fixes and DT support Laurent Pinchart
2014-12-15  0:13 ` [PATCH 02/10] iommu/ipmmu-vmsa: Flush P[UM]D entry before freeing the child page table Laurent Pinchart
2014-12-15  0:13 ` [PATCH 03/10] iommu/ipmmu-vmsa: Invalidate TLB after unmapping Laurent Pinchart
     [not found] ` <1418602429-11276-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2014-12-15  0:13   ` [PATCH 01/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment Laurent Pinchart
2014-12-15  0:13   ` [PATCH 04/10] iommu/ipmmu-vmsa: Return proper error if devm_request_irq fails Laurent Pinchart
2014-12-15  0:13 ` [PATCH 05/10] iommu/ipmmu-vmsa: Add device tree bindings documentation Laurent Pinchart
2014-12-15  0:13 ` [PATCH 06/10] iommu/ipmmu-vmsa: Add device tree support Laurent Pinchart
2014-12-15  0:13 ` [PATCH 07/10] iommu/ipmmu-vmsa: Support multiple micro TLBs per device Laurent Pinchart
2014-12-15  0:13 ` [PATCH 08/10] iommu/ipmmu-vmsa: Remove platform data support Laurent Pinchart
2014-12-15  0:13 ` [PATCH 09/10] ARM: shmobile: r8a7790: Add IPMMU DT nodes Laurent Pinchart
2014-12-15  0:13 ` [PATCH 10/10] ARM: shmobile: r8a7791: " Laurent Pinchart
2014-12-15 13:07   ` Geert Uytterhoeven
     [not found]     ` <CAMuHMdWtkk=Q=Mfcyz03LBzXMuBOr8BZGNyCsQb9bwDty-4OiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-12-15 18:44       ` Laurent Pinchart
2014-12-15 22:44         ` Magnus Damm
2014-12-15 23:16           ` Laurent Pinchart
2014-12-16 11:54             ` Magnus Damm
2014-12-16 10:02         ` Geert Uytterhoeven
2014-12-16 11:47           ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).